mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <a.fatoum@pengutronix.de>
To: barebox@lists.infradead.org
Cc: Ahmad Fatoum <a.fatoum@pengutronix.de>
Subject: [PATCH v2] of: fdt: fix overflows when parsing sizes
Date: Wed, 24 Jul 2024 11:25:52 +0200	[thread overview]
Message-ID: <20240724092552.303668-1-a.fatoum@pengutronix.de> (raw)

The function dt_struct_advance() is used to advance a pointer to the next
offset within the structure block, while checking that the result is in
bounds.

Unfortunately, the function used a signed size argument. This had the
effect that a too-large size in the FDT wrapped around and caused the
pointer to move backwards.

This issue was found by libfuzzer which generated an FDT that
always triggered an out-of-memory condition: One struct indicated a size
that caused the pointer to move backwards.

The resulting loop allocated memory on every iteration and eventually
ran out.

Fix this by using unsigned sizes and treating wrap around as an
error case.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - remove unneeded (!dt && size) check (Sascha)

@Sascha, I didn't know (or had forgotten) that the FIT image parser is
separate. It lacks a number of checks that were added to harden the FDT
parser. I will submit this separately.
---
 drivers/of/fdt.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 8dca41990c87..4cd33cb04947 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -32,11 +32,12 @@ static inline bool __dt_ptr_ok(const struct fdt_header *fdt, const void *p,
 }
 #define dt_ptr_ok(fdt, p) __dt_ptr_ok(fdt, p, sizeof(*(p)), __alignof__(*(p)))
 
-static inline uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, int size)
+static inline uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, uint32_t size)
 {
-	dt += size;
-	dt = ALIGN(dt, 4);
+	if (check_add_overflow(dt, size, &dt))
+		return 0;
 
+	dt = ALIGN(dt, 4);
 	if (dt > f->off_dt_struct + f->size_dt_struct)
 		return 0;
 
@@ -165,7 +166,7 @@ static struct device_node *__of_unflatten_dtb(const void *infdt, int size,
 {
 	const void *nodep;	/* property node pointer */
 	uint32_t tag;		/* tag */
-	int  len;		/* length of the property */
+	uint32_t len;		/* length of the property */
 	const struct fdt_property *fdt_prop;
 	const char *pathp, *name;
 	struct device_node *root, *node = NULL;
-- 
2.39.2




             reply	other threads:[~2024-07-24  9:31 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-24  9:25 Ahmad Fatoum [this message]
2024-07-30  8:16 ` Sascha Hauer
2024-07-30  9:32 ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240724092552.303668-1-a.fatoum@pengutronix.de \
    --to=a.fatoum@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox