[PATCH v3 01/15] libfdt: overlay: Fix out-of-bounds write on bad fixup offset
From: Douglas Anderson
Date: Sun Nov 01 2026 - 17:03:46 EST
A malformed overlay with a negative or very large fixup offset in
/__fixups__ (such as "/fragment@0:target:-4" or
"/fragment@0:target:4294967292") can cause an out-of-bounds write during
overlay application:
1. overlay_fixup_phandle() parses the offset with strtoul(), which
accepts negative numbers and values larger than UINT32_MAX, and
truncates the result into a 32-bit integer.
2. fdt_setprop_inplace_namelen_partial() checks bounds with
`(unsigned)proplen < (len + idx)`, where `len + idx` wraps around to
a small number when `idx` is near UINT32_MAX, bypassing the check and
passing the huge offset to memcpy().
Tighten the fixup offset parsing in overlay_fixup_phandle(), fix the
bounds check in fdt_setprop_inplace_namelen_partial() to avoid unsigned
wrap-around, and add test cases covering both negative and overflowing
offsets.
Assisted-by: Gemini:gemini-pro-prerelease
Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
---
Changes in v3:
- Split the fixup offset overflow fix and fdt_wip.c bounds check out of
"libfdt: overlay: Prepare fixup and fragment path helpers for reuse"
into a standalone bugfix patch with dedicated tests (Simon Glass)
- Add a comment explaining why an initial ASCII digit check is needed
before strtoul() (David Gibson)
libfdt/fdt_overlay.c | 34 +++++++++++++++++-----
libfdt/fdt_wip.c | 7 ++++-
tests/overlay_bad_fixup_negative_index.dts | 9 ++++++
tests/overlay_bad_fixup_overflow_index.dts | 9 ++++++
tests/run_tests.sh | 2 ++
tests/setprop_inplace.c | 8 +++++
6 files changed, 60 insertions(+), 9 deletions(-)
create mode 100644 tests/overlay_bad_fixup_negative_index.dts
create mode 100644 tests/overlay_bad_fixup_overflow_index.dts
diff --git a/libfdt/fdt_overlay.c b/libfdt/fdt_overlay.c
index 51a3859620a4..c6dc4774cff3 100644
--- a/libfdt/fdt_overlay.c
+++ b/libfdt/fdt_overlay.c
@@ -330,10 +330,10 @@ static int overlay_update_local_references(void *fdto, uint32_t delta)
static int overlay_fixup_one_phandle(void *fdto, int symbols_off,
const char *path, uint32_t path_len,
const char *name, uint32_t name_len,
- int poffset, uint32_t phandle)
+ uint32_t poffset, uint32_t phandle)
{
fdt32_t phandle_prop;
- int fixup_off;
+ int fixup_off, ret;
if (symbols_off < 0)
return symbols_off;
@@ -345,10 +345,20 @@ static int overlay_fixup_one_phandle(void *fdto, int symbols_off,
return fixup_off;
phandle_prop = cpu_to_fdt32(phandle);
- return fdt_setprop_inplace_namelen_partial(fdto, fixup_off,
- name, name_len, poffset,
- &phandle_prop,
- sizeof(phandle_prop));
+ ret = fdt_setprop_inplace_namelen_partial(fdto, fixup_off,
+ name, name_len, poffset,
+ &phandle_prop,
+ sizeof(phandle_prop));
+ /*
+ * fdt_setprop_inplace_namelen_partial() only returns -FDT_ERR_NOSPACE
+ * when poffset + sizeof(phandle_prop) exceeds the existing property's
+ * length in the overlay, which means the overlay is malformed (not
+ * that the base DTB buffer ran out of space).
+ */
+ if (ret == -FDT_ERR_NOSPACE)
+ return -FDT_ERR_BADOVERLAY;
+
+ return ret;
}
/**
@@ -409,7 +419,8 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
uint32_t fixup_len;
const char *sep;
char *endptr;
- int poffset, ret;
+ unsigned long poffset;
+ int ret;
fixup_end = memchr(value, '\0', len);
if (!fixup_end)
@@ -438,8 +449,15 @@ static int overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
if (!name_len)
return -FDT_ERR_BADOVERLAY;
+ /*
+ * Require an initial ASCII digit because strtoul() accepts
+ * leading whitespace and '+'/'-' signs (wrapping negative
+ * values to unsigned long).
+ */
+ if ((*(sep + 1) < '0') || (*(sep + 1) > '9'))
+ return -FDT_ERR_BADOVERLAY;
poffset = strtoul(sep + 1, &endptr, 10);
- if ((*endptr != '\0') || (endptr <= (sep + 1)))
+ if ((*endptr != '\0') || (poffset > UINT32_MAX))
return -FDT_ERR_BADOVERLAY;
ret = overlay_fixup_one_phandle(fdto, symbols_off,
diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c
index c2d7566a67dc..dd163d91498c 100644
--- a/libfdt/fdt_wip.c
+++ b/libfdt/fdt_wip.c
@@ -23,7 +23,12 @@ int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
if (!propval)
return proplen;
- if ((unsigned)proplen < (len + idx))
+ /*
+ * Check that idx + len fits within proplen using subtraction rather
+ * than addition so that a large idx (or negative len) cannot wrap
+ * around 32-bit unsigned arithmetic.
+ */
+ if (idx > (unsigned)proplen || (unsigned)len > (unsigned)proplen - idx)
return -FDT_ERR_NOSPACE;
memcpy((char *)propval + idx, val, len);
diff --git a/tests/overlay_bad_fixup_negative_index.dts b/tests/overlay_bad_fixup_negative_index.dts
new file mode 100644
index 000000000000..02ccee905e85
--- /dev/null
+++ b/tests/overlay_bad_fixup_negative_index.dts
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+ __fixups__ {
+ test = "/fragment@0:target:-4";
+ };
+};
diff --git a/tests/overlay_bad_fixup_overflow_index.dts b/tests/overlay_bad_fixup_overflow_index.dts
new file mode 100644
index 000000000000..d7d05267d7b3
--- /dev/null
+++ b/tests/overlay_bad_fixup_overflow_index.dts
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+/include/ "overlay_bad_fixup_base.dtsi"
+
+/ {
+ __fixups__ {
+ test = "/fragment@0:target:4294967292";
+ };
+};
diff --git a/tests/run_tests.sh b/tests/run_tests.sh
index 5bc7817dd12a..0d815e307909 100755
--- a/tests/run_tests.sh
+++ b/tests/run_tests.sh
@@ -274,6 +274,8 @@ BAD_FIXUP_TREES="bad_index \
empty \
empty_index \
index_trailing \
+ negative_index \
+ overflow_index \
path_empty_prop \
path_only \
path_only_sep \
diff --git a/tests/setprop_inplace.c b/tests/setprop_inplace.c
index 61a0da16fce8..48182169fbf6 100644
--- a/tests/setprop_inplace.c
+++ b/tests/setprop_inplace.c
@@ -80,5 +80,13 @@ int main(int argc, char *argv[])
check_getprop(fdt, 0, "compatible", strlen(TEST_STRING_4_RESULT) + 1,
TEST_STRING_4_RESULT);
+ err = fdt_setprop_inplace_namelen_partial(fdt, 0, "compatible",
+ strlen("compatible"),
+ 0xfffffffcU,
+ TEST_STRING_4_PARTIAL, 4);
+ if (err != -FDT_ERR_NOSPACE)
+ FAIL("fdt_setprop_inplace_namelen_partial() with wrapping idx returned %d instead of -FDT_ERR_NOSPACE\n",
+ err);
+
PASS();
}
--
2.56.0.317.g454f8302e1-goog