On Fri, May 22, 2026 at 01:30:20PM +0100, Anton Ivanov wrote: > From: Binarly Vulnerability Research <[email protected]> > > 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 <[email protected]> > --- > 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
signature.asc
Description: PGP signature
