Re: [PATCH v3 5/5] of: reserved_mem: reject static mapped regions overlapping existing reservations

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] of/reserved_mem: valid overlapping reservations are broken by unconditional rejection
--

commit f3ecc5d72c441ce7fd915e30544e07c69215f6c4
Author: Wandun Chen <chenwandun@lixiang.com>

of: reserved_mem: reject static mapped regions overlapping existing reservations

This commit modifies early_init_dt_reserve_memory() to reject static
reserved-memory regions that overlap with existing memblock reservations.
It aims to prevent memory corruption when initialization of a statically
placed region fails and memblock_phys_free() returns the overlap to the
buddy allocator.

Link: https://sashiko.dev/#/message/20260806100605.2C2C01F000E9%40smtp.kernel.org [1]

> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 8d2057f2ac12e..2d1e01c0f8867 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -141,6 +141,10 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base,
>  		return ret;
>  	}
>  
> +	if (memblock_is_region_reserved(base, size) ||
> +	    memblock_overlaps_nomap_region(base, size))
> +		return -EBUSY;
> +

[Severity: High]
Does this code unconditionally reject reserved-memory nodes that perfectly
describe memory already reserved by the architecture or EFI setup?

If a bootloader (like U-Boot or EFI) reserves a memory region (e.g.
EFI_RESERVED_TYPE) and also passes a /reserved-memory DT node describing the
same region to ensure compatibility, it looks like
early_init_dt_reserve_memory() will now find the region already reserved in
memblock and return -EBUSY. This bypasses memblock_mark_rsrv_rmem().

Later in the boot process, fdt_scan_reserved_mem_late() checks for the
MEMBLOCK_RSRV_RMEM flag on the region:

drivers/of/of_reserved_mem.c:fdt_scan_reserved_mem_late() {
    ...
    if (!memblock_is_region_rsrv_rmem(base, size, nomap))
        continue;
    ...
}

If the flag is missing, it skips calling fdt_init_reserved_mem_node(). Could
this cause valid overlapping DT nodes to be silently discarded, causing
devices relying on these nodes (like DRM display drivers using an EFI
framebuffer) to fail to probe because of_reserved_mem_lookup() will return
NULL?

Additionally, does the existing check in the if (nomap) block earlier in
early_init_dt_reserve_memory() have the same effect? It appears to reject
no-map DT nodes if the region was already marked nomap by EFI, effectively
destroying those DT nodes as well.

>  	ret = memblock_reserve(base, size);
>  	if (!ret)
>  		memblock_mark_rsrv_rmem(base, size);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920092852.614973-1-chenwandun1@gmail.com?part=5