On Wed, Jun 3, 2026 at 1:44 AM Wandun <[email protected]> wrote: > > > > 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 <[email protected]> > >> > >> 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 <[email protected]> > >> --- > >> 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.
I would go with this option. AIUI, this series isn't going to land in 7.2, so ultimately you will rebase on v7.2-rc1 which will have the fix. > > b) Keep patch 03 in the v4 respin of the full series, reordered to the front > per your earlier comment. Rob
