Re: [PATCH v3 03/11] of: reserved_mem: avoid post-init UAF when alloc_reserved_mem_array() fails

[Wandun]

On 6/3/26 00:24, Rob Herring wrote:
> On Wed, May 27, 2026 at 11:29:09AM +0800, Wandun Chen wrote:
> > From: Wandun Chen <chenwandun@xxxxxxxxxxx>
> >
> > The global pointer 'reserved_mem' continues to reference the
> > reserved_mem_array which lives in __initdata if
> > alloc_reserved_mem_array() fails. of_reserved_mem_lookup() is
> > exported for post-init use, that would dereference freed memory
> > and trigger a use-after-free.
> >
> > So reset reserved_mem_count to 0 when alloc_reserved_mem_array()
> > fails.
> >
> > Fixes: 00c9a452a235 ("of: reserved_mem: Add code to dynamically allocate reserved_mem array")
> Fixes should come first in a series.
Understood, will do in future submissions.
> > Signed-off-by: Wandun Chen <chenwandun@xxxxxxxxxxx>
> > ---
> >   drivers/of/of_reserved_mem.c | 20 ++++++++++++++------
> >   1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > index 313cbc57aa45..6d479381ff1f 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -69,29 +69,31 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> >    * the initial static array is copied over to this new array and
> >    * the new array is used from this point on.
> >    */
> > -static void __init alloc_reserved_mem_array(void)
> > +static bool __init alloc_reserved_mem_array(void)
> >   {
> >   	struct reserved_mem *new_array;
> >   	size_t alloc_size, copy_size, memset_size;
> >   
> > +	if (!total_reserved_mem_cnt)
> > +		return true;
> > +
> >   	alloc_size = array_size(total_reserved_mem_cnt, sizeof(*new_array));
> >   	if (alloc_size == SIZE_MAX) {
> >   		pr_err("Failed to allocate memory for reserved_mem array with err: %d", -EOVERFLOW);
> > -		return;
> > +		goto fail;
> >   	}
> >   
> >   	new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
> >   	if (!new_array) {
> >   		pr_err("Failed to allocate memory for reserved_mem array with err: %d", -ENOMEM);
> > -		return;
> > +		goto fail;
> >   	}
> >   
> >   	copy_size = array_size(reserved_mem_count, sizeof(*new_array));
> >   	if (copy_size == SIZE_MAX) {
> >   		memblock_free(new_array, alloc_size);
> > -		total_reserved_mem_cnt = MAX_RESERVED_REGIONS;
> >   		pr_err("Failed to allocate memory for reserved_mem array with err: %d", -EOVERFLOW);
> These prints could be moved to 'fail'. Perhaps instead of just printing
> an error value, you can return the error value instead of boolean.
Will do, consolidating pr_err() under 'fail' and changing the return type
to int.
> If you respin just this patch, I can pick it up for 7.2.
Before I respin, I'd like to flag a dependency:
patch 05/07 in this series build on the signature change introduced by this
patch ("the void -> bool return type change of alloc_reserved_mem_array()")

Could you let me know which of the following you'd prefer:
a) Take patch 03 alone via your tree as you suggested, after it lands, I'll
   respin the remaining patches of this series.

b) Keep patch 03 in the v4 respin of the full series, reordered to the front
   per your earlier comment.

Best regards,
Wandun
> Rob