[PATCH 3/3] libfdt: Check fdt_offset_ptr() return value unconditionally

[Simon Glass]

Hi Marke,

On Thu, 13 Nov 2025 at 14:57, Marek Vasut <marek.vasut at mailbox.org> wrote:
>
> On 11/13/25 8:33 PM, Simon Glass wrote:
>
> Hello Simon,
>
> >> The fdt_offset_ptr() may return NULL pointer, check the return
> >> value from fdt_offset_ptr() for NULL unconditionally to prevent
> >> sandbox crashes.
> >>
> >> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> >> ---
> >> Cc: Adriano Cordova <adrianox at gmail.com>
> >> Cc: Andrew Goodbody <andrew.goodbody at linaro.org>
> >> Cc: Christian Marangi <ansuelsmth at gmail.com>
> >> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> >> Cc: Patrice Chotard <patrice.chotard at foss.st.com>
> >> Cc: Sam Edwards <cfsworks at gmail.com>
> >> Cc: Simon Glass <sjg at chromium.org>
> >> Cc: Tom Rini <trini at konsulko.com>
> >> Cc: u-boot at lists.denx.de
> >> ---
> >>   scripts/dtc/libfdt/fdt.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/scripts/dtc/libfdt/fdt.c b/scripts/dtc/libfdt/fdt.c
> >> index 95f644c31f9..d16cc3d4730 100644
> >> --- a/scripts/dtc/libfdt/fdt.c
> >> +++ b/scripts/dtc/libfdt/fdt.c
> >> @@ -168,7 +168,7 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
> >>
> >>          *nextoffset = -FDT_ERR_TRUNCATED;
> >>          tagp = fdt_offset_ptr(fdt, offset, FDT_TAGSIZE);
> >> -       if (!can_assume(VALID_DTB) && !tagp)
> >> +       if (!tagp)
> >>                  return FDT_END; /* premature end */
> >>          tag = fdt32_to_cpu(*tagp);
> >>          offset += FDT_TAGSIZE;
> >> @@ -186,7 +186,7 @@ uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
> >>
> >>          case FDT_PROP:
> >>                  lenp = fdt_offset_ptr(fdt, offset, sizeof(*lenp));
> >> -               if (!can_assume(VALID_DTB) && !lenp)
> >> +               if (!lenp)
> >>                          return FDT_END; /* premature end */
> >>
> >>                  len = fdt32_to_cpu(*lenp);
> >> --
> >> 2.51.0
> >>
> >
> > The can_assume() is there to be respected and helps to reduce code
> > size. What problem are you actually seeing?
> fdt_offset_ptr() can return NULL, and it does in test_spl on sandbox if
> I recall it right, because the U-Boot code passes in offsets which can
> not be resolved properly and that makes fdt_offset_ptr() return NULL and
> trigger SIGSEGV when it is dereferenced here.

That sounds like the FDT is corrupt? Here we are parsing through tags,
so we should really figure out why it is corrupt.

But if note, then we should not be passing bad offsets...so what is
the root cause of that?

Regards,
Simon