mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH master 1/4] of: only replace a device tree that is actually tentative
@ 2026-08-24 11:59 Ahmad Fatoum
  2026-08-24 11:59 ` [PATCH master 2/4] tlsf: unpoison whole block in malloc_usable_size() Ahmad Fatoum
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2026-08-24 11:59 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

barebox_register_of() is called a second time when the EFI payload
registers a device tree read from the EFI system partition on top of the
tentative empty device tree registered earlier at core_initcall.

The check that is meant to allow this looks at the children of the newly
passed device tree instead of those of the already registered one, so it
rejects exactly the case it was added for: registering a populated
device tree over the empty placeholder returns -EBUSY, while an empty
device tree may silently replace a fully populated one.

Look at the registered root node instead.

Fixes: 10922345fbcc ("efi: payload: register dummy device tree")
Assisted-by: Claude:opus-5
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/base.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 62d452907a57..2a2edc167cc6 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2151,7 +2151,11 @@ of_populate_initcall(barebox_of_populate);
 int barebox_register_of(struct device_node *root)
 {
 	if (root_node) {
-		if (!list_empty(&root->children))
+		/*
+		 * Only a tentative device tree, i.e. one without any nodes,
+		 * may be replaced by a real one.
+		 */
+		if (!list_empty(&root_node->children))
 			return -EBUSY;
 		/* Not necessarily safe to free, so leak it.. */
 		root_node = NULL;
-- 
2.47.3




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

* [PATCH master 2/4] tlsf: unpoison whole block in malloc_usable_size()
  2026-08-24 11:59 [PATCH master 1/4] of: only replace a device tree that is actually tentative Ahmad Fatoum
@ 2026-08-24 11:59 ` Ahmad Fatoum
  2026-08-24 11:59 ` [PATCH master 3/4] partitions: dos: bound extended partition chain Ahmad Fatoum
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2026-08-24 11:59 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

malloc_usable_size() tells the caller how many bytes beyond the
originally requested size may be accessed, but TLSF only unpoisons the
requested size, leaving the padding up to the block size poisoned.
free_sensitive() zeroes the whole usable size, so with CONFIG_KASAN
enabled, freeing sensitive memory whose size is not a multiple of the
poisoning granule falsely reports a use-after-poison in
memzero_explicit().

Unpoison the whole block when its usable size is queried, so callers
can do what the API promises.

Fixes: 0af97b298266 ("malloc: implement free_sensitive()")
Assisted-by: Claude:fable-5
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/tlsf_malloc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/common/tlsf_malloc.c b/common/tlsf_malloc.c
index 36fdc307cc26..8315073105cc 100644
--- a/common/tlsf_malloc.c
+++ b/common/tlsf_malloc.c
@@ -52,7 +52,16 @@ EXPORT_SYMBOL(free);
 
 size_t malloc_usable_size(void *mem)
 {
-	return tlsf_block_size(mem);
+	size_t size = tlsf_block_size(mem);
+
+	/*
+	 * Callers like free_sensitive() may access the whole usable
+	 * size, so unpoison the padding beyond the requested size.
+	 */
+	if (size)
+		kasan_unpoison_shadow(mem, size);
+
+	return size;
 }
 EXPORT_SYMBOL(malloc_usable_size);
 
-- 
2.47.3




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

* [PATCH master 3/4] partitions: dos: bound extended partition chain
  2026-08-24 11:59 [PATCH master 1/4] of: only replace a device tree that is actually tentative Ahmad Fatoum
  2026-08-24 11:59 ` [PATCH master 2/4] tlsf: unpoison whole block in malloc_usable_size() Ahmad Fatoum
@ 2026-08-24 11:59 ` Ahmad Fatoum
  2026-08-24 11:59 ` [PATCH master 4/4] of: fdt: bound node nesting depth in __of_unflatten_dtb Ahmad Fatoum
  2026-08-24 12:58 ` [PATCH master 1/4] of: only replace a device tree that is actually tentative Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2026-08-24 11:59 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

dos_extended_partition() follows the EBR chain by reading each logical
partition's link entry (the second entry in the EBR) to locate the next
EBR. The loop is an unbounded while (1) and thus could keep running
until barebox runs out of memory.

Bound it at MAX_PARTITION to fix this.

Fixes: 57392a862d40 ("partition: allocate struct partition in parser")
Assisted-by: Claude:fable-5
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/partitions/dos.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/common/partitions/dos.c b/common/partitions/dos.c
index e5286cb6eb1f..3d9e6e67c676 100644
--- a/common/partitions/dos.c
+++ b/common/partitions/dos.c
@@ -129,11 +129,11 @@ static void dos_extended_partition(struct block_device *blk, struct dos_partitio
 	uint8_t *buf = xmalloc(BLOCKSIZE(blk));
 	uint32_t ebr_sector = partition->first_sec;
 	struct partition_entry *table = (struct partition_entry *)&buf[0x1be];
-	unsigned partno = 4;
+	unsigned partno;
 	struct dos_partition *dpart;
 	struct partition *pentry;
 
-	while (1) {
+	for (partno = 4; partno < MAX_PARTITION; partno++) {
 		int rc, i;
 
 		dev_dbg(blk->dev, "expect EBR in sector 0x%x\n", ebr_sector);
@@ -176,8 +176,6 @@ static void dos_extended_partition(struct block_device *blk, struct dos_partitio
 
 		list_add_tail(&pentry->list, &dpd->pd.partitions);
 
-		partno++;
-
 		/* the second entry defines the start of the next ebr if != 0 */
 		if (get_unaligned_le32(&table[1].partition_start))
 			ebr_sector = partition->first_sec +
@@ -186,6 +184,12 @@ static void dos_extended_partition(struct block_device *blk, struct dos_partitio
 			break;
 	}
 
+	/* bound the EBR chain: a cyclic link would loop forever */
+	if (partno == MAX_PARTITION) {
+		dev_err(blk->dev, "too many logical partitions\n");
+		goto out;
+	}
+
 out:
 	free(buf);
 	return;
-- 
2.47.3




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

* [PATCH master 4/4] of: fdt: bound node nesting depth in __of_unflatten_dtb
  2026-08-24 11:59 [PATCH master 1/4] of: only replace a device tree that is actually tentative Ahmad Fatoum
  2026-08-24 11:59 ` [PATCH master 2/4] tlsf: unpoison whole block in malloc_usable_size() Ahmad Fatoum
  2026-08-24 11:59 ` [PATCH master 3/4] partitions: dos: bound extended partition chain Ahmad Fatoum
@ 2026-08-24 11:59 ` Ahmad Fatoum
  2026-08-24 12:58 ` [PATCH master 1/4] of: only replace a device tree that is actually tentative Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Ahmad Fatoum @ 2026-08-24 11:59 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

of_new_node() builds each node's full_name by concatenating the parent's
full path, so unflattening a chain of N nested nodes costs O(N^2) time and
memory. A crafted FIT/DTB with hundreds of thousands of nested nodes (e.g.
the BRLY-2026-042 U-Boot PoC, 500k deep) therefore drives barebox into
multi-gigabyte allocations and minutes of CPU before failing, a denial of
service, even though the iterative walk here never overflows the stack.

Reject blobs nested deeper than FDT_MAX_DEPTH (64, as Linux's own
drivers/of/fdt.c uses) by tracking depth across FDT_BEGIN_NODE/FDT_END_NODE.
Real device trees are only a handful of levels deep, so the limit is
generous for legitimate input while cutting the pathological case off early.

Assisted-by: Claude:fable-5
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/of/fdt.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 1648f4c2d945..b5b64cd06b8d 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -173,6 +173,12 @@ static int fdt_parse_header(const struct fdt_header *fdt, size_t fdt_size,
 	return 0;
 }
 
+/*
+ * Maximum node nesting depth we are willing to unflatten.
+ * Matches the limit Linux uses in its own drivers/of/fdt.c.
+ */
+#define FDT_MAX_DEPTH	64
+
 /**
  * of_unflatten_dtb - unflatten a dtb binary blob
  * @infdt - the fdt blob to unflatten
@@ -196,6 +202,7 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
 	struct fdt_header f;
 	int ret;
 	int maxlen;
+	unsigned int depth = 0;
 	const struct fdt_header *fdt = infdt;
 
 	ret = fdt_parse_header(infdt, size, &f);
@@ -247,6 +254,12 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
 				goto err;
 			}
 
+			if (++depth > FDT_MAX_DEPTH) {
+				pr_err("unflatten: node nesting too deep\n");
+				ret = -EINVAL;
+				goto err;
+			}
+
 			if (!node) {
 				/* The root node must have an empty name */
 				if (*pathp) {
@@ -272,6 +285,7 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
 				goto err;
 			}
 
+			depth--;
 			node = node->parent;
 
 			dt_struct = dt_struct_advance(&f, dt_struct, FDT_TAGSIZE, 0);
-- 
2.47.3




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

* Re: [PATCH master 1/4] of: only replace a device tree that is actually tentative
  2026-08-24 11:59 [PATCH master 1/4] of: only replace a device tree that is actually tentative Ahmad Fatoum
                   ` (2 preceding siblings ...)
  2026-08-24 11:59 ` [PATCH master 4/4] of: fdt: bound node nesting depth in __of_unflatten_dtb Ahmad Fatoum
@ 2026-08-24 12:58 ` Sascha Hauer
  3 siblings, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2026-08-24 12:58 UTC (permalink / raw)
  To: barebox, Ahmad Fatoum


On Mon, 24 Aug 2026 13:59:55 +0200, Ahmad Fatoum wrote:
> barebox_register_of() is called a second time when the EFI payload
> registers a device tree read from the EFI system partition on top of the
> tentative empty device tree registered earlier at core_initcall.
> 
> The check that is meant to allow this looks at the children of the newly
> passed device tree instead of those of the already registered one, so it
> rejects exactly the case it was added for: registering a populated
> device tree over the empty placeholder returns -EBUSY, while an empty
> device tree may silently replace a fully populated one.
> 
> [...]

Applied, thanks!

[1/4] of: only replace a device tree that is actually tentative
      https://git.pengutronix.de/cgit/barebox/commit/?id=1408da582445 (link may not be stable)
[2/4] tlsf: unpoison whole block in malloc_usable_size()
      https://git.pengutronix.de/cgit/barebox/commit/?id=89489cc0f45d (link may not be stable)
[3/4] partitions: dos: bound extended partition chain
      https://git.pengutronix.de/cgit/barebox/commit/?id=ddbe5e709988 (link may not be stable)
[4/4] of: fdt: bound node nesting depth in __of_unflatten_dtb
      https://git.pengutronix.de/cgit/barebox/commit/?id=05b6373a0fda (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>




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

end of thread, other threads:[~2026-08-24 13:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-24 11:59 [PATCH master 1/4] of: only replace a device tree that is actually tentative Ahmad Fatoum
2026-08-24 11:59 ` [PATCH master 2/4] tlsf: unpoison whole block in malloc_usable_size() Ahmad Fatoum
2026-08-24 11:59 ` [PATCH master 3/4] partitions: dos: bound extended partition chain Ahmad Fatoum
2026-08-24 11:59 ` [PATCH master 4/4] of: fdt: bound node nesting depth in __of_unflatten_dtb Ahmad Fatoum
2026-08-24 12:58 ` [PATCH master 1/4] of: only replace a device tree that is actually tentative Sascha Hauer

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