[PATCH v2 0/2] of: reserved_mem: fix OOB write and name skipped nodes

[PATCH v2 0/2] of: reserved_mem: fix OOB write and name skipped nodes

[Sang-Heon Jeon]

Patch 1 fixes an out-of-bounds write in fdt_scan_reserved_mem(). When
more than MAX_RESERVED_REGIONS dynamically-placed regions are defined,
it writes past the end of the local array.

Patch 2 names the skipped node in fdt_init_reserved_mem_node(), the other
place that drops regions.

Changes from v1 [1]
- fix bounds check to cover the missed case in v1, based on sashiko
  review
- print skipped no name error message in fdt_scan_reserved_mem() and
  fdt_init_reserved_mem_node() both.

[1] https://lore.kernel.org/all/20260603152709.941788-1-ekffu200098@gmail.com/

Sang-Heon Jeon (2):
  of: reserved_mem: prevent OOB when too many dynamic regions are
    defined
  of: reserved_mem: print skipped node name when too many regions are
    defined

 drivers/of/of_reserved_mem.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] of: reserved_mem: prevent OOB when too many dynamic regions are defined

[Sang-Heon Jeon]

On boot, fdt_scan_reserved_mem() saves each dynamically-placed
/reserved-memory subnode into a local array of size
MAX_RESERVED_REGIONS.

If the device tree defines more than MAX_RESERVED_REGIONS
dynamically-placed regions, fdt_scan_reserved_mem() writes past the
end of the local array.

Add a bounds check that logs an error and skips the excess regions,
restoring the original behavior.

Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved memory regions are processed")
Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
 drivers/of/of_reserved_mem.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 82222bd45ac6..42e3e2d8a2b8 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -359,6 +359,7 @@ int __init fdt_scan_reserved_mem(void)
 		err = __reserved_mem_reserve_reg(child, uname);
 		if (!err)
 			count++;
+
 		/*
 		 * Save the nodes for the dynamically-placed regions
 		 * into an array which will be used for allocation right
@@ -366,10 +367,17 @@ int __init fdt_scan_reserved_mem(void)
 		 * or marked as no-map. This is done to avoid dynamically
 		 * allocating from one of the statically-placed regions.
 		 */
-		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) {
-			dynamic_nodes[dynamic_nodes_cnt] = child;
-			dynamic_nodes_cnt++;
+		if (err != -ENOENT || !of_get_flat_dt_prop(child, "size", NULL))
+			continue;
+
+		if (dynamic_nodes_cnt == MAX_RESERVED_REGIONS) {
+			pr_err("too many defined dynamic regions, skip '%s'\n",
+			       uname);
+			continue;
 		}
+
+		dynamic_nodes[dynamic_nodes_cnt] = child;
+		dynamic_nodes_cnt++;
 	}
 	for (int i = 0; i < dynamic_nodes_cnt; i++) {
 		const char *uname;
-- 
2.43.0

[PATCH v2 2/2] of: reserved_mem: print skipped node name when too many regions are defined

[Sang-Heon Jeon]

When too many reserved memory regions are defined,
fdt_init_reserved_mem_node() logs an error and skips the region without
naming the node.

Print the node name so the skipped node can be identified, matching
fdt_scan_reserved_mem().

Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
 drivers/of/of_reserved_mem.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 42e3e2d8a2b8..7b1616b9c6c2 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -641,7 +641,8 @@ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *un
 	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
 
 	if (reserved_mem_count == total_reserved_mem_cnt) {
-		pr_err("not enough space for all defined regions.\n");
+		pr_err("not enough space for all defined regions, skip '%s'\n",
+		       uname);
 		return;
 	}
 
-- 
2.43.0

Re: [PATCH v2 1/2] of: reserved_mem: prevent OOB when too many dynamic regions are defined

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Global buffer overflow on static reserved_mem_array when dynamic allocation fails
--

commit ce2daed7217e851888336023e70ee6c49e1f66cd
Author: Sang-Heon Jeon <ekffu200098@gmail.com>

of: reserved_mem: prevent OOB when too many dynamic regions are defined

This commit adds bounds checking for dynamically-placed /reserved-memory
subnodes to prevent out-of-bounds writes to the local dynamic_nodes array
in fdt_scan_reserved_mem().

> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 8d5777cb5d1b3..499b769e98096 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
[ ... ]
> @@ -348,10 +349,17 @@ int __init fdt_scan_reserved_mem(void)
>  		 * or marked as no-map. This is done to avoid dynamically
>  		 * allocating from one of the statically-placed regions.
>  		 */
> -		if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) {
> -			dynamic_nodes[dynamic_nodes_cnt] = child;
> -			dynamic_nodes_cnt++;
> +		if (err != -ENOENT || !of_get_flat_dt_prop(child, "size", NULL))
> +			continue;
> +
> +		if (dynamic_nodes_cnt == MAX_RESERVED_REGIONS) {
> +			pr_err("too many defined dynamic regions, skip '%s'\n",
> +			       uname);
> +			continue;
>  		}
> +
> +		dynamic_nodes[dynamic_nodes_cnt] = child;
> +		dynamic_nodes_cnt++;
>  	}
>  	for (int i = 0; i < dynamic_nodes_cnt; i++) {
>  		const char *uname;

[Severity: Critical]
This is a pre-existing issue, but could the initialization sequence still
leave the kernel vulnerable to a global buffer overflow later on?

In fdt_scan_reserved_mem(), total_reserved_mem_cnt is updated to the
total count of regions, which could exceed MAX_RESERVED_REGIONS due to
statically-placed regions.

Later, fdt_scan_reserved_mem_late() calls alloc_reserved_mem_array() to
dynamically allocate a larger array:

drivers/of/of_reserved_mem.c:alloc_reserved_mem_array() {
    ...
    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;
    }
    ...
}

If memblock_alloc() fails, it returns early but does not reset
total_reserved_mem_cnt back to MAX_RESERVED_REGIONS.

When iterating over the regions, fdt_init_reserved_mem_node() relies on
total_reserved_mem_cnt for its bounds checking:

drivers/of/of_reserved_mem.c:fdt_init_reserved_mem_node() {
    ...
    if (reserved_mem_count == total_reserved_mem_cnt) {
        pr_err("not enough space for all defined regions, skip '%s'\n",
               uname);
        return;
    }
    ...
}

Since total_reserved_mem_cnt remains inflated, this check will fail to protect
the statically-sized reserved_mem_array, leading to an out-of-bounds write.

Should alloc_reserved_mem_array() reset total_reserved_mem_cnt to
MAX_RESERVED_REGIONS on the error path to avoid overflowing the static
reserved_mem_array?

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

Re: [PATCH v2 2/2] of: reserved_mem: print skipped node name when too many regions are defined

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds write to the static `reserved_mem_array` if `memblock_alloc` fails during dynamic array reallocation.
- [High] Ignored memory reservation failures allow unreserved memory to be successfully registered and handed out as exclusive reserved memory.
- [Medium] Dynamically allocated `no-map` reserved memory regions are leaked from `memblock.reserved` if their initialization fails.
--

commit f95986ab58870e3d79cb320c6dc73ef80982357e
Author: Sang-Heon Jeon <ekffu200098@gmail.com>

of: reserved_mem: print skipped node name when too many regions are defined

When too many reserved memory regions are defined,
fdt_init_reserved_mem_node() logs an error and skips the region without
naming the node.

Print the node name so the skipped node can be identified, matching
fdt_scan_reserved_mem().

> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 499b769e98096..df3d90cb8dbe1 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -623,7 +623,8 @@ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *un
>  	struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
>  
>  	if (reserved_mem_count == total_reserved_mem_cnt) {
> -		pr_err("not enough space for all defined regions.\n");
> +		pr_err("not enough space for all defined regions, skip '%s'\n",
> +		       uname);
>  		return;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does alloc_reserved_mem_array()
fail to reset total_reserved_mem_cnt on error?

If memblock_alloc() fails during dynamic array reallocation, it returns early
without resetting total_reserved_mem_cnt back to its original value. When the
bounds check in fdt_init_reserved_mem_node() is reached later,
reserved_mem_count can increment beyond the statically-sized
reserved_mem_array limit, potentially causing an out-of-bounds write:

drivers/of/of_reserved_mem.c:alloc_reserved_mem_array() {
    ...
	new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
	if (!new_array) {
		pr_err("Failed to allocate memory for reserved_mem array...");
		return;
	}
    ...
}

[Severity: High]
This is a pre-existing issue, but does __reserved_mem_reserve_reg()
unconditionally return 0 even if early_init_dt_reserve_memory() fails?

If memory reservation fails, it logs an error but returns success, allowing
unreserved memory to be successfully registered and later handed out as
exclusive reserved memory:

drivers/of/of_reserved_mem.c:__reserved_mem_reserve_reg() {
    ...
		if (early_init_dt_reserve_memory(base, size, nomap)) {
			pr_err("Reserved memory: failed to reserve...");
		}
	}
	return 0;
}

[Severity: Medium]
This is a pre-existing issue, but does fdt_init_reserved_mem_node() leak
dynamically allocated no-map regions if initialization fails?

If __reserved_mem_init_node() returns an error, the cleanup path for no-map
regions clears the MEMBLOCK_NOMAP flag but skips calling memblock_phys_free(),
which permanently leaks the physical memory from memblock.reserved:

drivers/of/of_reserved_mem.c:fdt_init_reserved_mem_node() {
    ...
	err = __reserved_mem_init_node(rmem, node);
	if (err != 0 && err != -ENODEV) {
		pr_info("node %s compatible matching fail\n", rmem->name);
		rmem->name = NULL;

		if (nomap)
			memblock_clear_nomap(rmem->base, rmem->size);
		else
			memblock_phys_free(rmem->base, rmem->size);
		return;
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260614133807.2165124-1-ekffu200098@gmail.com?part=2

Re: [PATCH v2 2/2] of: reserved_mem: print skipped node name when too many regions are defined

[Sang-Heon Jeon]

On Sun, Jun 14, 2026 at 10:50 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Out-of-bounds write to the static `reserved_mem_array` if `memblock_alloc` fails during dynamic array reallocation.
> - [High] Ignored memory reservation failures allow unreserved memory to be successfully registered and handed out as exclusive reserved memory.
> - [Medium] Dynamically allocated `no-map` reserved memory regions are leaked from `memblock.reserved` if their initialization fails.
> --
>
> commit f95986ab58870e3d79cb320c6dc73ef80982357e
> Author: Sang-Heon Jeon <ekffu200098@gmail.com>
>
> of: reserved_mem: print skipped node name when too many regions are defined
>
> When too many reserved memory regions are defined,
> fdt_init_reserved_mem_node() logs an error and skips the region without
> naming the node.
>
> Print the node name so the skipped node can be identified, matching
> fdt_scan_reserved_mem().
>
> > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > index 499b769e98096..df3d90cb8dbe1 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -623,7 +623,8 @@ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *un
> >       struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> >
> >       if (reserved_mem_count == total_reserved_mem_cnt) {
> > -             pr_err("not enough space for all defined regions.\n");
> > +             pr_err("not enough space for all defined regions, skip '%s'\n",
> > +                    uname);
> >               return;
> >       }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does alloc_reserved_mem_array()
> fail to reset total_reserved_mem_cnt on error?
>
> If memblock_alloc() fails during dynamic array reallocation, it returns early
> without resetting total_reserved_mem_cnt back to its original value. When the
> bounds check in fdt_init_reserved_mem_node() is reached later,
> reserved_mem_count can increment beyond the statically-sized
> reserved_mem_array limit, potentially causing an out-of-bounds write:
>
> drivers/of/of_reserved_mem.c:alloc_reserved_mem_array() {
>     ...
>         new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
>         if (!new_array) {
>                 pr_err("Failed to allocate memory for reserved_mem array...");
>                 return;
>         }
>     ...
> }
>
> [Severity: High]
> This is a pre-existing issue, but does __reserved_mem_reserve_reg()
> unconditionally return 0 even if early_init_dt_reserve_memory() fails?
>
> If memory reservation fails, it logs an error but returns success, allowing
> unreserved memory to be successfully registered and later handed out as
> exclusive reserved memory:
>
> drivers/of/of_reserved_mem.c:__reserved_mem_reserve_reg() {
>     ...
>                 if (early_init_dt_reserve_memory(base, size, nomap)) {
>                         pr_err("Reserved memory: failed to reserve...");
>                 }
>         }
>         return 0;
> }

These areas are affected by the ongoing series [1], so it might be
better not to touch them until it is merged.

[1] https://lore.kernel.org/all/20260527032917.3385849-1-chenwandun1@gmail.com/

> [Severity: Medium]
> This is a pre-existing issue, but does fdt_init_reserved_mem_node() leak
> dynamically allocated no-map regions if initialization fails?
>
> If __reserved_mem_init_node() returns an error, the cleanup path for no-map
> regions clears the MEMBLOCK_NOMAP flag but skips calling memblock_phys_free(),
> which permanently leaks the physical memory from memblock.reserved:
>
> drivers/of/of_reserved_mem.c:fdt_init_reserved_mem_node() {
>     ...
>         err = __reserved_mem_init_node(rmem, node);
>         if (err != 0 && err != -ENODEV) {
>                 pr_info("node %s compatible matching fail\n", rmem->name);
>                 rmem->name = NULL;
>
>                 if (nomap)
>                         memblock_clear_nomap(rmem->base, rmem->size);
>                 else
>                         memblock_phys_free(rmem->base, rmem->size);
>                 return;
>     ...
> }

It should be rare, but looks correct to me. BTW, it is not directly
related to this patchset, so I think it is better to fix it in a
separate patch.

> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260614133807.2165124-1-ekffu200098@gmail.com?part=2

Best Regards,
Sang-Heon Jeon