Re: [PATCH v2 5/5] of: reserved_mem: retain static no-map memory on init failure

On 8/18/26 17:34, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] In `__reserved_mem_alloc_size`, an allocation error when marking a region as `NOMAP` is ignored, leading to a use-after-free of memblock memory.
> --
> 
> --- Patch [5]: [PATCH v2 5/5] of: reserved_mem: retain static no-map memory on init failure ---
> commit 35ad8d6e4b86dc63d754a8415950fc7ab54dfce4
> Author: Wandun Chen <[email protected]>
> 
> of: reserved_mem: retain static no-map memory on init failure
> 
> Clear MEMBLOCK_NOMAP only for dynamically allocated regions on init failure.
> Static regions are initialized after paging_init() and should retain their
> memory states.
> 
> Link: https://sashiko.dev/#/message/20260814090305.4C8741F00A3D%40smtp.kernel.org [1]
> 
>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>> index a0df075a9902e..0c5163ce601ea 100644
>> --- a/drivers/of/of_reserved_mem.c
>> +++ b/drivers/of/of_reserved_mem.c
>> @@ -708,7 +708,7 @@ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *un
>>  		pr_info("node %s compatible matching fail\n", rmem->name);
>>  		rmem->name = NULL;
>>  
>> -		if (nomap)
>> +		if (dynamic && nomap)
>>  			memblock_clear_nomap(rmem->base, rmem->size);
>>  
>>  		if (dynamic || !nomap)
> 
> [Severity: High]
> This is a pre-existing issue, but while looking at reserved memory error
> handling in this file, I noticed a potential memory corruption when dynamic
> allocation fails.
> 
> In early_init_dt_alloc_reserved_memory_arch(), the base address is assigned
> before the nomap check:
> 
> base = memblock_phys_alloc_range(size, align, start, end);
> ...
> *res_base = base;
> if (nomap) {
>         err = memblock_mark_nomap(base, size);
>         if (err)
>                 memblock_phys_free(base, size);
> }
> 
> If memblock_mark_nomap() fails, the memory is freed, but *res_base still
> holds the freed address instead of being cleared.
> 
> Then in __reserved_mem_alloc_size(), the code checks the base address instead
> of the return value ret:
> 
> ret = early_init_dt_alloc_reserved_memory_arch(size, align,
>                                                0, 0, nomap, &base);
> if (ret == 0)
>         pr_debug("...");
> 
> if (base == 0) {
>         pr_err("failed to allocate memory...");
>         return -ENOMEM;
> }

Repeated questions;

The reason of memblock_mark_nomap fails only can be out of memory, and this would
trigger panic because memblock is not allowd resize before paging_init.

> 
> Since base contains the freed address instead of 0, does this allow the
> freed memory to be registered and later handed out by the buddy allocator
> while still being incorrectly used as reserved memory?
>