From: Wandun Chen __reserved_mem_reserve_reg() discards the error from early_init_dt_reserve_memory() and returns 0 unconditionally, so the caller counts the node in total_reserved_mem_cnt and the late scan initializes it without checking whether the early reservation actually succeeded. A region whose reservation failed is then handed to a device assuming the memory is protected. Propagate the error so failed reservations are no longer counted, and record the failed nodes so fdt_scan_reserved_mem_late() can skip them. Recording the failed nodes explicitly is necessary because fdt_scan_reserved_mem_late() rescans the DT independently. It cannot tell from memblock whether early reservation succeeded. The failed-node array is bounded by MAX_RESERVED_REGIONS, the number of static regions is not bounded by it, so on overflow the extra nodes fall back to being initialized, which is the current behavior. Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved memory regions are processed") Signed-off-by: Wandun Chen --- drivers/of/of_reserved_mem.c | 65 ++++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 8c9d6395d6a3..c6e73d710ee1 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -32,6 +32,30 @@ static struct reserved_mem *reserved_mem __refdata = reserved_mem_array; static int total_reserved_mem_cnt = MAX_RESERVED_REGIONS; static int reserved_mem_count; +static int reserve_failed_nodes[MAX_RESERVED_REGIONS] __initdata; +static int reserve_failed_nodes_cnt __initdata; + +static bool __init reserved_mem_node_reserve_failed(int node) +{ + int i; + + for (i = 0; i < reserve_failed_nodes_cnt; i++) + if (reserve_failed_nodes[i] == node) + return true; + return false; +} + +static bool __init record_reserve_failed_node(int node, const char *uname) +{ + if (reserve_failed_nodes_cnt == MAX_RESERVED_REGIONS) { + pr_err("too many failed regions, '%s' reservation failed\n", uname); + return false; + } + + reserve_failed_nodes[reserve_failed_nodes_cnt++] = node; + return true; +} + static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size, phys_addr_t align, phys_addr_t start, phys_addr_t end, bool nomap, phys_addr_t *res_base) @@ -141,7 +165,8 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base, * first entry in 'reg' property */ static int __init __reserved_mem_reserve_reg(unsigned long node, - const char *uname) + const char *uname, + bool *should_record_failed_node) { phys_addr_t base, size; int len, err; @@ -149,6 +174,8 @@ static int __init __reserved_mem_reserve_reg(unsigned long node, bool nomap; u64 b, s; + *should_record_failed_node = false; + prop = of_flat_dt_get_addr_size_prop(node, "reg", &len); if (!prop || !len) return -ENOENT; @@ -167,14 +194,20 @@ static int __init __reserved_mem_reserve_reg(unsigned long node, base = b; size = s; - if (size && early_init_dt_reserve_memory(base, size, nomap) == 0) { - fdt_fixup_reserved_mem_node(node, base, size); - pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n", - uname, &base, (unsigned long)(size / SZ_1M)); - } else { + if (!size) + return -EINVAL; + + err = early_init_dt_reserve_memory(base, size, nomap); + if (err) { + *should_record_failed_node = true; pr_err("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n", uname, &base, (unsigned long)(size / SZ_1M)); + return err; } + + fdt_fixup_reserved_mem_node(node, base, size); + pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n", + uname, &base, (unsigned long)(size / SZ_1M)); return 0; } @@ -306,10 +339,14 @@ void __init fdt_scan_reserved_mem_late(void) base = b; size = s; - if (size) { - uname = fdt_get_name(fdt, child, NULL); - fdt_init_reserved_mem_node(child, uname, base, size); - } + if (!size) + continue; + + if (reserved_mem_node_reserve_failed(child)) + continue; + + uname = fdt_get_name(fdt, child, NULL); + fdt_init_reserved_mem_node(child, uname, base, size); } /* check for overlapping reserved regions */ @@ -349,6 +386,7 @@ int __init fdt_scan_reserved_mem(void) fdt_for_each_subnode(child, fdt, node) { const char *uname; + bool should_record_failed_node; int err; if (!of_fdt_device_is_available(fdt, child)) @@ -356,9 +394,14 @@ int __init fdt_scan_reserved_mem(void) uname = fdt_get_name(fdt, child, NULL); - err = __reserved_mem_reserve_reg(child, uname); + err = __reserved_mem_reserve_reg(child, uname, + &should_record_failed_node); if (!err) count++; + else if (should_record_failed_node && + !record_reserve_failed_node(child, uname)) + /* Keep a slot for the untracked node's late initialization. */ + count++; /* * Save the nodes for the dynamically-placed regions -- 2.43.0 From: Wandun Chen Static no-map reserved-memory regions are marked in memblock.memory rather than memblock.reserved. So the reservation overlap check does not reject a static region that overlaps existing no-map memory. Both regions can then be initialized and hand the same physical memory to different reserved-memory drivers. So reject a static region that overlaps existing no-map memory. Sashiko found this issue in [1]. Fixes: 86588296acbf ("fdt: Properly handle "no-map" field in the memory region") Signed-off-by: Wandun Chen Link: https://sashiko.dev/#/message/20260814084718.29C341F000E9%40smtp.kernel.org [1] --- drivers/of/of_reserved_mem.c | 3 ++- include/linux/memblock.h | 1 + mm/memblock.c | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index c6e73d710ee1..9fb2e4c29443 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -152,7 +152,8 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base, * if the region isn't memory as it won't be mapped. */ if (memblock_overlaps_region(&memblock.memory, base, size) && - memblock_is_region_reserved(base, size)) + (memblock_is_region_reserved(base, size) || + memblock_overlaps_nomap(base, size))) return -EBUSY; return memblock_mark_nomap(base, size); diff --git a/include/linux/memblock.h b/include/linux/memblock.h index d62db9e776cf..27d68fbb3157 100644 --- a/include/linux/memblock.h +++ b/include/linux/memblock.h @@ -148,6 +148,7 @@ int memblock_physmem_add(phys_addr_t base, phys_addr_t size); void memblock_trim_memory(phys_addr_t align); unsigned long memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1, phys_addr_t base2, phys_addr_t size2); +bool memblock_overlaps_nomap(phys_addr_t base, phys_addr_t size); bool memblock_overlaps_region(struct memblock_type *type, phys_addr_t base, phys_addr_t size); bool memblock_validate_numa_coverage(unsigned long threshold_bytes); diff --git a/mm/memblock.c b/mm/memblock.c index 9ce86349a29f..4cabfe365ac4 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -202,6 +202,20 @@ memblock_addrs_overlap(phys_addr_t base1, phys_addr_t size1, phys_addr_t base2, return ((base1 < (base2 + size2)) && (base2 < (base1 + size1))); } +bool __init memblock_overlaps_nomap(phys_addr_t base, phys_addr_t size) +{ + struct memblock_region *region; + + memblock_cap_size(base, &size); + for_each_mem_region(region) { + if (memblock_is_nomap(region) && + memblock_addrs_overlap(base, size, region->base, region->size)) + return true; + } + + return false; +} + bool __init_memblock memblock_overlaps_region(struct memblock_type *type, phys_addr_t base, phys_addr_t size) { -- 2.43.0 From: Wandun Chen memblock_reserve() permits overlapping reservations, so a statically placed region whose 'reg' overlaps an existing one is accepted. When initialization of a statically placed reserved-memory region overlapping an existing reservation fails, memblock_phys_free() returns the overlap to the buddy allocator, corrupting that memory. Reject the overlap up front. Dynamically allocated regions are unaffected, as they are allocated from free memory and so cannot overlap an existing reservation. Sashiko found this issue in [1]. Fixes: d0b8ed47e83a ("of: reserved_mem: fix reserve memory leak") Signed-off-by: Wandun Chen Link: https://sashiko.dev/#/message/20260806100605.2C2C01F000E9%40smtp.kernel.org [1] --- drivers/of/of_reserved_mem.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 9fb2e4c29443..767ef702392e 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -158,6 +158,11 @@ static int __init early_init_dt_reserve_memory(phys_addr_t base, return memblock_mark_nomap(base, size); } + + if (memblock_is_region_reserved(base, size) || + memblock_overlaps_nomap(base, size)) + return -EBUSY; + return memblock_reserve(base, size); } -- 2.43.0 From: Wandun Chen Dynamically reserved-memory regions are added to memblock.reserved by memblock_phys_alloc_range() during __reserved_mem_alloc_size(). When a reserved-memory region's driver initialization fails, fdt_init_reserved_mem_node() cleans up the reservation. For no-map regions it only calls memblock_clear_nomap(), leaving the range in memblock.reserved and unavailable for normal memory use. Fix it by freeing the region on init failure when it was dynamically allocated. Sashiko found this issue in [1]. Fixes: 7b25995f5319 ("of: of_reserved_mem: mark nomap memory instead of removing") Signed-off-by: Wandun Chen Link: https://sashiko.dev/#/message/20260806100605.2C2C01F000E9%40smtp.kernel.org [1] --- drivers/of/of_reserved_mem.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index 767ef702392e..ac5db19dcc93 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -136,7 +136,8 @@ static int __init alloc_reserved_mem_array(void) } static void fdt_init_reserved_mem_node(unsigned long node, const char *uname, - phys_addr_t base, phys_addr_t size); + phys_addr_t base, phys_addr_t size, + bool dynamic); static int fdt_validate_reserved_mem_node(unsigned long node, phys_addr_t *align); static int fdt_fixup_reserved_mem_node(unsigned long node, @@ -352,7 +353,7 @@ void __init fdt_scan_reserved_mem_late(void) continue; uname = fdt_get_name(fdt, child, NULL); - fdt_init_reserved_mem_node(child, uname, base, size); + fdt_init_reserved_mem_node(child, uname, base, size, false); } /* check for overlapping reserved regions */ @@ -567,7 +568,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node, const char *unam } fdt_fixup_reserved_mem_node(node, base, size); - fdt_init_reserved_mem_node(node, uname, base, size); + fdt_init_reserved_mem_node(node, uname, base, size, true); return 0; } @@ -676,13 +677,15 @@ static int __init __reserved_mem_init_node(struct reserved_mem *rmem, * @uname: name of the reserved memory node * @base: base address of the reserved memory region * @size: size of the reserved memory region + * @dynamic: whether the region was dynamically allocated * * This function calls the region-specific initialization function for a * reserved memory region and saves all region-specific data to the * reserved_mem array to allow of_reserved_mem_lookup() to find it. */ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *uname, - phys_addr_t base, phys_addr_t size) + phys_addr_t base, phys_addr_t size, + bool dynamic) { int err = 0; bool nomap; @@ -708,7 +711,8 @@ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *un if (nomap) memblock_clear_nomap(rmem->base, rmem->size); - else + + if (dynamic || !nomap) memblock_phys_free(rmem->base, rmem->size); return; } else { -- 2.43.0 From: Wandun Chen Static no-map reserved-memory regions are initialized after paging_init(). If initialization fails, MEMBLOCK_NOMAP would be cleared, and the memory is available to the buddy allocator even though it was excluded from the kernel linear mapping. Clear MEMBLOCK_NOMAP only for dynamically allocated regions, which are initialized before paging_init() and can safely be made available to the kernel linear mapping. Sashiko found this issue in [1]. Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved memory regions are processed") Signed-off-by: Wandun Chen Link: https://sashiko.dev/#/message/20260814090305.4C8741F00A3D%40smtp.kernel.org [1] --- drivers/of/of_reserved_mem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index ac5db19dcc93..0090fa86dd10 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -709,7 +709,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) -- 2.43.0