Re: [PATCH v2 05/10] libfdt: overlay: Prepare fixup and fragment path helpers for reuse

[Doug Anderson]

Hi David,

On Thu, Oct 29, 2026 at 8:55 PM David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 28, 2026 at 02:55:55PM -0700, Douglas Anderson wrote:
> > Tighten `overlay_parse_fixup_entry()` and `overlay_split_fragment_path()`
> > before reusing them to update the base tree's `/__local_fixups__`:
> >
> > - In `overlay_parse_fixup_entry()`, store the `strtoul()` result in an
> >   `unsigned long` before range-checking against `INT32_MAX -
> >   sizeof(uint32_t)`, and require the offset field to begin with an ASCII
> >   digit (`'0'`..`'9'`) since `strtoul()` accepts leading whitespace and
> >   `+`/`-` signs (wrapping negative values to `unsigned long`).
>
> Huh, I'd assumed that strtoul() wouldn't accept negative strings, but
> according to BUGS in strtoul(1) that might not be the case (at least
> for Linux+glibc).  Good catch.
>
> This may need some minor rebasing for the suggestions on the previous
> patch, plus some additional minor refinements suggested below.
>
> > @@ -404,9 +405,10 @@ static int overlay_parse_fixup_entry(const char **valuep, int *lenp,
> >       if (!name_len)
> >               return -FDT_ERR_BADOVERLAY;
> >
> > +     if ((*(sep + 1) < '0') || (*(sep + 1) > '9'))
> > +             return -FDT_ERR_BADOVERLAY;
>
> Putting the rationale for this in a comment here would be more useful
> than just in the commit message.

Done. Added a comment right above this check explaining why we require an
ASCII digit before calling `strtoul()`. Also, as Simon suggested, I split
this out-of-bounds write fix (along with fixing the unsigned overflow check
in `fdt_setprop_inplace_namelen_partial()`) into a standalone bugfix patch
at the start of the series.


-Doug