Devicetree
 help / color / mirror / Atom feed
* [PATCH 0/2] of: reserved_mem: fix stack overflow and make dynamic reserved region configurable
@ 2026-06-04  7:36 Wandun Chen
  2026-06-04  7:36 ` [PATCH 1/2] of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem() Wandun Chen
  2026-06-04  7:36 ` [PATCH 2/2] of: reserved_mem: add config to extend dynamic reserved memory regions Wandun Chen
  0 siblings, 2 replies; 6+ messages in thread
From: Wandun Chen @ 2026-06-04  7:36 UTC (permalink / raw)
  To: devicetree, linux-kernel; +Cc: robh, saravanak

From: Wandun Chen <chenwandun@lixiang.com>

Patch 1 fixes a stack buffer overflow in fdt_scan_reserved_mem() when the
DTS contains more than 64 reserved-memory regions.

Patch 2 makes the limit configurable via Kconfig
(OF_RESERVED_MEM_DYNAMIC_REGIONS) so platforms with many such regions
can modify it without patching the source.

Wandun Chen (2):
  of: reserved_mem: avoid stack buffer overflow in
    fdt_scan_reserved_mem()
  of: reserved_mem: add config to extend dynamic reserved memory regions

 drivers/of/Kconfig           | 11 +++++++++++
 drivers/of/of_private.h      |  2 +-
 drivers/of/of_reserved_mem.c |  6 ++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem()
  2026-06-04  7:36 [PATCH 0/2] of: reserved_mem: fix stack overflow and make dynamic reserved region configurable Wandun Chen
@ 2026-06-04  7:36 ` Wandun Chen
  2026-06-04  7:45   ` sashiko-bot
  2026-06-04  7:36 ` [PATCH 2/2] of: reserved_mem: add config to extend dynamic reserved memory regions Wandun Chen
  1 sibling, 1 reply; 6+ messages in thread
From: Wandun Chen @ 2026-06-04  7:36 UTC (permalink / raw)
  To: devicetree, linux-kernel; +Cc: robh, saravanak

From: Wandun Chen <chenwandun@lixiang.com>

Sashiko found a potential stack buffer overflow in fdt_scan_reserved_mem()
due to missing bounds checking on dynamic_nodes_cnt [1].

Fix this by adding bounds check.

Link: https://sashiko.dev/#/patchset/20260604015332.3669384-1-chenwandun1%40gmail.com?part=1 [1]
Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
---
 drivers/of/of_reserved_mem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index ce1d5530ec0f..27dc98aa9bf9 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -346,6 +346,12 @@ int __init fdt_scan_reserved_mem(void)
 		err = __reserved_mem_reserve_reg(child, uname);
 		if (!err)
 			count++;
+
+		if (dynamic_nodes_cnt >= MAX_RESERVED_REGIONS) {
+			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d)\n",
+				    MAX_RESERVED_REGIONS);
+			continue;
+		}
 		/*
 		 * Save the nodes for the dynamically-placed regions
 		 * into an array which will be used for allocation right
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] of: reserved_mem: add config to extend dynamic reserved memory regions
  2026-06-04  7:36 [PATCH 0/2] of: reserved_mem: fix stack overflow and make dynamic reserved region configurable Wandun Chen
  2026-06-04  7:36 ` [PATCH 1/2] of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem() Wandun Chen
@ 2026-06-04  7:36 ` Wandun Chen
  2026-06-04  7:44   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Wandun Chen @ 2026-06-04  7:36 UTC (permalink / raw)
  To: devicetree, linux-kernel; +Cc: robh, saravanak

From: Wandun Chen <chenwandun@lixiang.com>

Nowadays, the dynamic reserved memory regions is 64 by default, If
the count of reserved memory regions defined in DTS bigger than 64,
only 64 reserved memory can be handled properly.

So add a config to configure the actual dynamic reserved memory
regions count instead of modify the code.

Signed-off-by: Wandun Chen <chenwandun@lixiang.com>
---
 drivers/of/Kconfig           | 11 +++++++++++
 drivers/of/of_private.h      |  2 +-
 drivers/of/of_reserved_mem.c |  2 +-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 50697cc3b07e..d6496ec3765c 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -99,6 +99,17 @@ config OF_IRQ
 config OF_RESERVED_MEM
 	def_bool OF_EARLY_FLATTREE
 
+config OF_RESERVED_MEM_DYNAMIC_REGIONS
+	int "Maximum count of the dynamic reserved memory regions"
+	depends on OF_RESERVED_MEM
+	default 64
+	range 1 256
+	help
+	  Allows to define proper dynamic reserved memory regions number
+	  according to DTS configuration.
+
+	  If unsure, leave the default value "64".
+
 config OF_RESOLVE
 	bool
 
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 0ae16da066e2..6ad00798f39d 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -9,7 +9,7 @@
  */
 
 #define FDT_ALIGN_SIZE 8
-#define MAX_RESERVED_REGIONS    64
+#define MAX_RESERVED_REGIONS	CONFIG_OF_RESERVED_MEM_DYNAMIC_REGIONS
 
 /**
  * struct alias_prop - Alias property in 'aliases' node
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 27dc98aa9bf9..d1680fc4fb38 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -348,7 +348,7 @@ int __init fdt_scan_reserved_mem(void)
 			count++;
 
 		if (dynamic_nodes_cnt >= MAX_RESERVED_REGIONS) {
-			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d)\n",
+			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d), try expanding CONFIG_OF_RESERVED_MEM_DYNAMIC_REGIONS.\n",
 				    MAX_RESERVED_REGIONS);
 			continue;
 		}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] of: reserved_mem: add config to extend dynamic reserved memory regions
  2026-06-04  7:36 ` [PATCH 2/2] of: reserved_mem: add config to extend dynamic reserved memory regions Wandun Chen
@ 2026-06-04  7:44   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-04  7:44 UTC (permalink / raw)
  To: Wandun Chen; +Cc: devicetree, conor+dt, robh

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

Pre-existing issues:
- [High] Out-of-bounds write on the static `reserved_mem_array` if dynamic allocation of the reserved memory array fails.
--

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

of: reserved_mem: add config to extend dynamic reserved memory regions
    
Nowadays, the dynamic reserved memory regions is 64 by default, If
the count of reserved memory regions defined in DTS bigger than 64,
only 64 reserved memory can be handled properly.

So add a config to configure the actual dynamic reserved memory
regions count instead of modify the code.

> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 056c4179f549c..73d7d7865518e 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -343,7 +343,7 @@ int __init fdt_scan_reserved_mem(void)
>  			count++;
>  
>  		if (dynamic_nodes_cnt >= MAX_RESERVED_REGIONS) {
> -			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d)\n",
> +			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d), try expanding CONFIG_OF_RESERVED_MEM_DYNAMIC_REGIONS.\n",
>  				    MAX_RESERVED_REGIONS);
>  			continue;
>  		}

[Severity: High]
This is a pre-existing issue, but there appears to be an out-of-bounds
write on the static reserved_mem_array if dynamic allocation fails.

In fdt_scan_reserved_mem(), the total count of reserved memory regions (and
thus total_reserved_mem_cnt) can exceed MAX_RESERVED_REGIONS because static
regions are not capped by the dynamic_nodes_cnt >= MAX_RESERVED_REGIONS check.

Later, alloc_reserved_mem_array() allocates a dynamically sized array of
total_reserved_mem_cnt elements. If memblock_alloc() fails, it returns early
without resetting total_reserved_mem_cnt to MAX_RESERVED_REGIONS:

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;
	}
...
}

When fdt_scan_reserved_mem_late() subsequently initializes the static regions,
it calls fdt_init_reserved_mem_node() for each. The bounds check relies on
the uncapped total_reserved_mem_cnt:

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

Since total_reserved_mem_cnt wasn't reset on failure, the check allows
reserved_mem_count to exceed MAX_RESERVED_REGIONS. Does this result in
out-of-bounds writes to the static reserved_mem_array?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem()
  2026-06-04  7:36 ` [PATCH 1/2] of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem() Wandun Chen
@ 2026-06-04  7:45   ` sashiko-bot
  2026-06-04  9:18     ` Wandun
  0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-06-04  7:45 UTC (permalink / raw)
  To: Wandun Chen; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added bounds check for `dynamic_nodes_cnt` is placed outside the block that evaluates whether a node is actually a dynamically-placed region, leading to spurious error logs.
--

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

of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem()

This commit adds bounds checking on dynamic_nodes_cnt to prevent a
potential stack buffer overflow in fdt_scan_reserved_mem().

Link: https://sashiko.dev/#/patchset/20260604015332.3669384-1-chenwandun1%40gmail.com?part=1 [1]

> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c

[ ... ]

> @@ -341,6 +341,12 @@ int __init fdt_scan_reserved_mem(void)
>  		err = __reserved_mem_reserve_reg(child, uname);
>  		if (!err)
>  			count++;
> +
> +		if (dynamic_nodes_cnt >= MAX_RESERVED_REGIONS) {
> +			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d)\n",
> +				    MAX_RESERVED_REGIONS);
> +			continue;
> +		}

[Severity: Medium]
Will this bounds check trigger spurious error logs for statically-placed
regions?

Because the check is placed outside the block that evaluates whether a node
is actually a dynamically-placed region, it evaluates for all subsequent nodes
once the limit is reached.

If the number of dynamically-placed regions exactly reaches
MAX_RESERVED_REGIONS, and a statically-placed region is processed
afterwards, won't it incorrectly trigger the warning claiming the dynamic
nodes limit was exceeded?

>  		/*
>  		 * Save the nodes for the dynamically-placed regions
>  		 * into an array which will be used for allocation right
>  		 * after all the statically-placed regions are reserved
>  		 * 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++;
>  		}

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem()
  2026-06-04  7:45   ` sashiko-bot
@ 2026-06-04  9:18     ` Wandun
  0 siblings, 0 replies; 6+ messages in thread
From: Wandun @ 2026-06-04  9:18 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: devicetree, robh, conor+dt



On 6/4/26 15:45, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The newly added bounds check for `dynamic_nodes_cnt` is placed outside the block that evaluates whether a node is actually a dynamically-placed region, leading to spurious error logs.
> --
>
> commit 6a79f526de5e1185b7c79fb469545f4094fb402f
> Author: Wandun Chen <chenwandun@lixiang.com>
>
> of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem()
>
> This commit adds bounds checking on dynamic_nodes_cnt to prevent a
> potential stack buffer overflow in fdt_scan_reserved_mem().
>
> Link: https://sashiko.dev/#/patchset/20260604015332.3669384-1-chenwandun1%40gmail.com?part=1 [1]
>
>> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
>> --- a/drivers/of/of_reserved_mem.c
>> +++ b/drivers/of/of_reserved_mem.c
> [ ... ]
>
>> @@ -341,6 +341,12 @@ int __init fdt_scan_reserved_mem(void)
>>   		err = __reserved_mem_reserve_reg(child, uname);
>>   		if (!err)
>>   			count++;
>> +
>> +		if (dynamic_nodes_cnt >= MAX_RESERVED_REGIONS) {
>> +			pr_err_once("Reserved memory: reached MAX_RESERVED_REGIONS(%d)\n",
>> +				    MAX_RESERVED_REGIONS);
>> +			continue;
>> +		}
> [Severity: Medium]
> Will this bounds check trigger spurious error logs for statically-placed
> regions?
Yes, it indeed, will fix in v2.
>
> Because the check is placed outside the block that evaluates whether a node
> is actually a dynamically-placed region, it evaluates for all subsequent nodes
> once the limit is reached.
>
> If the number of dynamically-placed regions exactly reaches
> MAX_RESERVED_REGIONS, and a statically-placed region is processed
> afterwards, won't it incorrectly trigger the warning claiming the dynamic
> nodes limit was exceeded?
>
>>   		/*
>>   		 * Save the nodes for the dynamically-placed regions
>>   		 * into an array which will be used for allocation right
>>   		 * after all the statically-placed regions are reserved
>>   		 * 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++;
>>   		}


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-04  9:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04  7:36 [PATCH 0/2] of: reserved_mem: fix stack overflow and make dynamic reserved region configurable Wandun Chen
2026-06-04  7:36 ` [PATCH 1/2] of: reserved_mem: avoid stack buffer overflow in fdt_scan_reserved_mem() Wandun Chen
2026-06-04  7:45   ` sashiko-bot
2026-06-04  9:18     ` Wandun
2026-06-04  7:36 ` [PATCH 2/2] of: reserved_mem: add config to extend dynamic reserved memory regions Wandun Chen
2026-06-04  7:44   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox