[PATCH v2] lib: fdtdec: validate bloblist FDT before consuming libfdt size
Tom Rini
trini at konsulko.com
Wed May 6 18:51:43 CEST 2026
On Wed, May 06, 2026 at 11:00:43AM -0400, Raymond Mao wrote:
> Hi Tom,
>
> On Tue, May 5, 2026 at 10:32 AM Tom Rini <trini at konsulko.com> wrote:
> >
> > On Tue, May 05, 2026 at 01:49:53PM +0000, Sverdlin, Alexander wrote:
> > > Hi Raymond,
> > >
> > > On Tue, 2026-05-05 at 09:47 -0400, Raymond Mao wrote:
> > > > > > Coverity Scan defects are observed in fdtdec_apply_bloblist_dtos(),
> > > > > > since the live FDT taken from the bloblist is passed to libfdt helpers
> > > > > > which consume header size/offset fields:
> > > > > > - fdt_open_into()
> > > > > > - fdt_pack()
> > > > > > - bloblist_resize(..., fdt_totalsize(...))
> > > > > >
> > > > > > Add a small helper to validate the FDT header and confirm that the
> > > > > > advertised totalsize fits within the currently allocated bloblist
> > > > > > record. Use the sanitized size before calling fdt_open_into(), again
> > > > > > after overlays are applied before calling fdt_pack(), and once more
> > > > > > after packing before shrinking the bloblist record.
> > > > > >
> > > > > > This keeps the existing flow unchanged while making the size consumers
> > > > > > operate on validated FDT metadata.
> > > > > >
> > > > > > Fixes: b70cbbfbf94f ("fdtdec: apply DT overlays from bloblist")
> > > > > > Addresses-Coverity-ID: CID 645837: (TAINTED_SCALAR)
> > > > > > Signed-off-by: Raymond Mao <raymond.mao at riscstar.com>
> > > > > > ---
> > > > > > changes in v2:
> > > > > > - Rebased on master.
> > > > > >
> > > > > > lib/fdtdec.c | 43 +++++++++++++++++++++++++++++++++++++++----
> > > > > > 1 file changed, 39 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c
> > > > > > index 2d66860f6ed..601f418db6e 100644
> > > > > > --- a/lib/fdtdec.c
> > > > > > +++ b/lib/fdtdec.c
> > > > > > @@ -1744,9 +1744,31 @@ static int fdtdec_apply_dto_blob(void **blob, __maybe_unused int size)
> > > > > > return fdt_overlay_apply_verbose((void *)gd->fdt_blob, *blob);
> > > > > > }
> > > > > >
> > > > > > +static int fdtdec_get_valid_fdt_size(const void *fdt, int alloc_size,
> > > > > > + int *fdt_sizep)
> > > > > > +{
> > > > > > + int ret, fdt_size;
> > > > > > +
> > > > > > + /*
> > > > > > + * Validate the header before libfdt trusts any header offsets/sizes.
> > > > > > + * Also make sure the advertised totalsize fits in the bloblist record.
> > > > > > + */
> > > > > > + ret = fdt_check_header(fdt);
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > +
> > > > > > + fdt_size = fdt_totalsize(fdt);
> > > > > > + if (fdt_size > alloc_size)
> > > > > > + return -FDT_ERR_TRUNCATED;
> > > > >
> > > > > here you return an error from the libfdt error space, but...
> > > > >
> > > > > > +
> > > > > > + *fdt_sizep = fdt_size;
> > > > > > +
> > > > > > + return 0;
> > > > > > +}
> > > > > > +
> > > > > > static int fdtdec_apply_bloblist_dtos(void)
> > > > > > {
> > > > > > - int ret;
> > > > > > + int ret, live_fdt_size;
> > > > > > struct fdt_header *live_fdt;
> > > > > > int blob_size;
> > > > > > size_t padded_size, max_size;
> > > > > > @@ -1760,8 +1782,12 @@ static int fdtdec_apply_bloblist_dtos(void)
> > > > > > if (live_fdt != gd->fdt_blob)
> > > > > > return -ENOENT;
> > > > >
> > > > > ... the caller returns the codes from the standard errno.h
> > > > >
> > > > > >
> > > > > > + ret = fdtdec_get_valid_fdt_size(live_fdt, blob_size, &live_fdt_size);
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > +
> > > > > > /* Calculate the allowed padded size */
> > > > > > - padded_size = fdt_totalsize(live_fdt) + CONFIG_SYS_FDT_PAD;
> > > > > > + padded_size = live_fdt_size + CONFIG_SYS_FDT_PAD;
> > > > > > max_size = bloblist_get_total_size() - bloblist_get_size() + blob_size;
> > > > > > if (padded_size > max_size)
> > > > > > padded_size = max_size;
> > > > > > @@ -1772,6 +1798,7 @@ static int fdtdec_apply_bloblist_dtos(void)
> > > > > > if (ret)
> > > > > > return ret;
> > > > > >
> > > > > > + blob_size = padded_size;
> > > > > > ret = fdt_open_into(live_fdt, live_fdt, padded_size);
> > > > > > if (ret)
> > > > > > return ret;
> > > > > > @@ -1781,12 +1808,20 @@ static int fdtdec_apply_bloblist_dtos(void)
> > > > > > if (ret)
> > > > > > return ret;
> > > > > >
> > > > > > - /* Shrink the blob to the actual FDT size */
> > > > > > + ret = fdtdec_get_valid_fdt_size(live_fdt, blob_size, &live_fdt_size);
> > > > > > + if (ret)
> > > > > > + return ret;
> > > > > > +
> > > > > > ret = fdt_pack(live_fdt);
> > > > > > if (ret)
> > > > > > return ret;
> > > > > >
> > > > > > - return bloblist_resize(BLOBLISTT_CONTROL_FDT, fdt_totalsize(live_fdt));
> > > > > > + ret = fdtdec_get_valid_fdt_size(live_fdt, blob_size, &live_fdt_size);
> > > > >
> > > > > doesn't the fdt_pack() already guarantee a valid header? Can the packed size
> > > > > exceed the blob_size if packing only shrinks?
> > > > >
> > > >
> > > > This might not happen, but just in order to solve the complaint from
> > > > the coverity tool, otherwise we need an exemption.
> > >
> > > Is it some king of new U-Boot policy that Coverity false-positives justify
> > > a dead or cargo-cult code? Shall it maybe fixed in Coverity itself?
> >
> > If this is a false-positive we can just mark it as such in Coverity.
> > That would be the correct path.
> >
>
> v3 is posted at:
> https://lore.kernel.org/u-boot/20260506145535.1679611-1-raymondmaoca@gmail.com/T/#u
> leaving the report (as below) between fdt_pack() and bloblist_resize()
> as a false-positive.
>
> 1784 /* Shink the blob to the actual FDT size */
> 1785 fdt_pack(live_fdt);
> >>> CID 645837: (TAINTED_SCALAR)
> >>> Passing tainted expression "fdt32_ld(&((struct fdt_header const *)live_fdt)->totalsize)" to "bloblist_resize", which uses it as an offset.
> 1786 return bloblist_resize(BLOBLISTT_CONTROL_FDT,
> fdt_totalsize(live_fdt));
Thanks, I've updated the issue.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20260506/277e8e8b/attachment.sig>
More information about the U-Boot
mailing list