On Thu, Jun 15, 2023 at 04:51:44PM +0200, Stephan Gerhold wrote: > On Thu, Jun 15, 2023 at 11:44:06AM +0100, Caleb Connolly wrote: > > On 6/14/23 17:31, Stephan Gerhold wrote: > > > If Linux fails to allocate the dynamic reserved memory specified in the > > > device tree, the size of the reserved_mem will be 0. Add a check for > > > this to avoid using an invalid reservation. > > > > > > Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx> > > > > Other uses of of_reserved_mem_lookup() also have unchecked uses of rmem [1], > > or check different things [2]. > > > > Does it make sense to put this check in the function itself? > > > > I can't think of any obvious scenarios where it makes sense to differentiate > > between rmem being NULL vs having a size of zero at the time where a driver > > is fetching it. > > > > As Bjorn described in the rmtfs patch, the memory allocation is essentially > > ignored, wouldn't it be better to print an error and invalidate the rmem in > > [3]? > > > > "Invalidating" isn't that easy because the reserved_mem is currently > stored in a simple array. Removing an entry would require shifting all > following values. But I suppose it would be easy to add the rmem->size > != 0 check in of_reserved_mem_lookup() so it doesn't have to be checked > on all usages. > > Given that no one seems to check for this at the moment I'm inclined to > agree with you that it would be better to handle this directly in > of_reserved_mem. Bjorn, what do you think? > I sent a v3 with the additional checks reverted. I'll work on a separate patch series to improve this independently of this one for all users (directly in of_reserved_mem). Thanks, Stephan
