[PATCH] fdt: Check return value and len of fdt_get_name() calls
Anton Ivanov
anton at binarly.io
Tue May 26 12:57:40 CEST 2026
> So scripts/dtc/libfdt/fdt_ro.c comes from https://github.com/dgibson/dtc
> and should be sent upstream there too (and all of scripts/dtc/ is, for
> reference for the other patches), so that we don't drop this part when
> re-syncing later via the linux kernel.
The patch for fdt_check_full() was added in the upstream dtc code back in
2022:
https://github.com/dgibson/dtc/commit/fda71da26e7fa00966a58afeb2114269b5512e59
It was never ported to U-Boot, most likely because the function was moved
from fdt_ro.c to the fdt_check.c.
Linux kernel does not use fdt_check_full(), and thus it is not affected.
We also updated the BRLY-2026-037_BRLY-2026-038.patch to align with the
current dtc logic:
https://github.com/dgibson/dtc/commit/50454658f2b5c8968ccd7856d94020c893a4be46
> Next, please rewrite the commit message to be concise and follow best
> practices and examples found throughout the codebase. This applies to
> all of the patches you posted, thanks.
We sent the new version of patches with the updated commit messages.
Thank you,
Anton
On Fri, May 22, 2026 at 7:32 PM Tom Rini <trini at konsulko.com> wrote:
> On Fri, May 22, 2026 at 01:30:20PM +0100, Anton Ivanov wrote:
>
> > From: Binarly Vulnerability Research <vr at binarly.io>
> >
> > fdt_get_name() in scripts/dtc/libfdt/fdt_ro.c can return a null
> > pointer. This happens when the fdt_ro_probe_() or
> > fdt_check_node_offset_() check fails, as well as when the
> > '!can_assume(LATEST) && fdt_version(fdt) < 0x10' condition is true
> > and the FDT node name doesn't contain the '/' character:
> >
> > const char *fdt_get_name(const void *fdt, int nodeoffset, int *len)
> > {
> > const struct fdt_node_header *nh = fdt_offset_ptr_(fdt,
> nodeoffset);
> > const char *nameptr;
> > int err;
> >
> > if (((err = fdt_ro_probe_(fdt)) < 0)
> > || ((err = fdt_check_node_offset_(fdt,
> nodeoffset)) < 0))
> > goto fail;
> >
> > nameptr = nh->name;
> >
> > if (!can_assume(LATEST) && fdt_version(fdt) < 0x10) {
> > /*
> > * For old FDT versions, match the naming
> conventions of V16:
> > * give only the leaf name (after all /). The
> actual tree
> > * contents are loosely checked.
> > */
> > const char *leaf;
> > leaf = strrchr(nameptr, '/');
> > if (leaf == NULL) {
> > err = -FDT_ERR_BADSTRUCTURE;
> > goto fail;
> > }
> > nameptr = leaf+1;
> > }
> >
> > if (len)
> > *len = strlen(nameptr);
> >
> > return nameptr;
> >
> > fail:
> > if (len)
> > *len = err;
> > return NULL;
> > }
> >
> > The fdt_get_name() function is used in fdt_find_regions() in
> > boot/fdt_region.c:
> >
> > int fdt_find_regions(const void *fdt, char * const inc[], int
> inc_count,
> > char * const exc_prop[], int
> exc_prop_count,
> > struct fdt_region region[], int
> max_regions,
> > char *path, int path_len, int
> add_string_tab)
> > {
> > int stack[FDT_MAX_DEPTH] = { 0 };
> > char *end;
> > int nextoffset = 0;
> > uint32_t tag;
> > int count = 0;
> > int start = -1;
> > int depth = -1;
> > int want = 0;
> > int base = fdt_off_dt_struct(fdt);
> > bool expect_end = false;
> >
> > end = path;
> > *end = '\0';
> > do {
> > const struct fdt_property *prop;
> > const char *name;
> > const char *str;
> > int include = 0;
> > int stop_at = 0;
> > int offset;
> > int len;
> >
> > offset = nextoffset;
> > tag = fdt_next_tag(fdt, offset, &nextoffset);
> > stop_at = nextoffset;
> >
> > /* If we see two root nodes, something is wrong */
> > if (expect_end && tag != FDT_END)
> > return -FDT_ERR_BADLAYOUT;
> >
> > switch (tag) {
> > case FDT_PROP:
> > include = want >= 2;
> > stop_at = offset;
> > prop = fdt_get_property_by_offset(fdt,
> offset, NULL);
> > str = fdt_string(fdt,
> fdt32_to_cpu(prop->nameoff));
> > if (!str)
> > return -FDT_ERR_BADSTRUCTURE;
> > if (str_in_list(str, exc_prop,
> exc_prop_count))
> > include = 0;
> > break;
> >
> > case FDT_NOP:
> > include = want >= 2;
> > stop_at = offset;
> > break;
> >
> > case FDT_BEGIN_NODE:
> > depth++;
> > if (depth == FDT_MAX_DEPTH)
> > return -FDT_ERR_BADSTRUCTURE;
> > name = fdt_get_name(fdt, offset, &len);
> >
> > /* The root node must have an empty name */
> > if (!depth && *name)
> > return -FDT_ERR_BADLAYOUT;
> > if (end - path + 2 + len >= path_len)
> > return -FDT_ERR_NOSPACE;
> > if (end != path + 1)
> > *end++ = '/';
> > strcpy(end, name);
> > end += len;
> > stack[depth] = want;
> > if (want == 1)
> > stop_at = offset;
> > if (str_in_list(path, inc, inc_count))
> > want = 2;
> > else if (want)
> > want--;
> > else
> > stop_at = offset;
> > include = want;
> > break;
> > ...
> >
> > There is no check that the value returned by fdt_get_name() is not
> > null, which leads to a null pointer dereference (at *name and
> > strcpy(end, name)) and a potential stack overflow of the 'end'
> > buffer. However, if the strcpy call succeeds, the subsequent
> > 'end += len' decreases the 'end' pointer, since 'len' is set to the
> > negative error code returned by fdt_get_name(). As a result, the next
> > iteration of the loop writes data to a stack location before the
> > start of the 'path' buffer, which can be used to overwrite the
> > return address of fdt_find_regions() and achieve code execution in
> > the context of the bootloader. This attack is possible when the page
> > at address 0x0 is mapped, which is common for embedded devices, so
> > the strcpy() call doesn't fail.
> >
> > The same lack of validation exists in fdt_next_region() in
> > boot/fdt_region.c, fdt_check_full() in scripts/dtc/libfdt/fdt_ro.c,
> > and display_fdt_by_regions() in tools/fdtgrep.c.
> >
> > Fix: Validate both the returned pointer and the length before use.
> >
> > Signed-off-by: Binarly Vulnerability Research <vr at binarly.io>
> > ---
> > boot/fdt_region.c | 4 ++++
> > scripts/dtc/libfdt/fdt_ro.c | 2 ++
> > tools/fdtgrep.c | 2 ++
> > 3 files changed, 8 insertions(+)
>
> So scripts/dtc/libfdt/fdt_ro.c comes from https://github.com/dgibson/dtc
> and should be sent upstream there too (and all of scripts/dtc/ is, for
> reference for the other patches), so that we don't drop this part when
> re-syncing later via the linux kernel.
>
> Next, please rewrite the commit message to be concise and follow best
> practices and examples found throughout the codebase. This applies to
> all of the patches you posted, thanks.
>
> --
> Tom
>
More information about the U-Boot
mailing list