On Mon, Jan 26, 2026 at 05:03:36PM -0500, Sean Anderson wrote:
> On 1/26/26 16:50, Marek Vasut wrote:
> > On 1/26/26 6:38 PM, Sean Anderson wrote:
> >> On 1/23/26 05:51, Marek Vasut wrote:
> >>> On 1/23/26 11:40 AM, Anshul Dalal wrote:
> >>>> Hi Marek,
> >>>>
> >>>> On Fri Jan 23, 2026 at 3:57 PM IST, Marek Vasut wrote:
> >>>>> On 1/23/26 4:32 AM, Beleswar Prasad Padhi wrote:
> >>>>>>
> >>>>>> On 23/01/26 00:33, Marek Vasut wrote:
> >>>>>>> On 1/22/26 6:45 PM, Tom Rini wrote:
> >>>>>>>> On Thu, Jan 22, 2026 at 10:52:47PM +0530, Beleswar Padhi wrote:
> >>>>>>>>
> >>>>>>>>> When CONFIG_SPL_MULTI_DTB_FIT is enabled, multiple device trees are
> >>>>>>>>> packed inside the multidtb.fit FIT image. While the individual DTBs
> >>>>>>>>> and the FIT image start address are 8-byte aligned, the DTBs
> >>>>>>>>> embedded
> >>>>>>>>> within the FIT image are not guaranteed to maintain 8-byte
> >>>>>>>>> alignment.
> >>>>>>>>>
> >>>>>>>>> This misalignment causes -FDT_ERR_ALIGNMENT failure in
> >>>>>>>>> setup_multi_dtb_fit() when locating the next available DTB within
> >>>>>>>>> the
> >>>>>>>>> FIT blob and setting gd->fdt_blob, because of the recent libfdt
> >>>>>>>>> hardening since commit 0535e46d55d7 ("scripts/dtc: Update to
> >>>>>>>>> upstream
> >>>>>>>>> version v1.7.2-35-g52f07dcca47c")
> >>>>>>>>>
> >>>>>>>>> The mkimage tool already supports enforcing this alignment via the
> >>>>>>>>> -B
> >>>>>>>>> option, but users would have to specify it explicitly. This change
> >>>>>>>>> makes 8-byte alignment the default when using -b.
> >>>>>>>>>
> >>>>>>>>> Reported-by: Anshul Dalal <[email protected]>
> >>>>>>>>> Closes:
> >>>>>>>>> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2fu%2dboot%2fDFJ950O0QM0D.380U0N16ZO19E%40ti.com&umid=003810bd-b0e0-4a71-9348-75ffbe5cbae5&rct=1769167692&auth=d807158c60b7d2502abde8a2fc01f40662980862-3859cf23dac8d65722a0a9ed4c40c98c98a297de
> >>>>>>>>> Fixes: 0535e46d55d7 ("scripts/dtc: Update to upstream version
> >>>>>>>>> v1.7.2-35-g52f07dcca47c")
> >>>>>>>>> Signed-off-by: Beleswar Padhi <[email protected]>
> >>>>>>>>
> >>>>>>>> Tested-by: Tom Rini <[email protected]>
> >>>>>>> Doesn't this enable 8-byte alignment for everything in case a DT is
> >>>>>>> specified on mkimage command line ? That doesn't seem right ?
> >>>>>>>
> >>>>>>> If I do mkimage -f auto -b ...dtb ... , then I only want the DTB to
> >>>>>>> be 8-byte aligned, all the other blobs can be 4 byte aligned just
> >>>>>>> fine .
> >>>>>>
> >>>>>>
> >>>>>> All the mkimage flags are parsed at once and populated in the params
> >>>>>> data structure in one shot in the code. There is no separation of code
> >>>>>> specifically for generating a FIT for a DTB vs for anything else.
> >>>>>
> >>>>> That does not seem to be true ?
> >>>>>
> >>>>> $ git grep -p '\<bl_len\>' tools/
> >>>>> tools/fit_image.c=static int fit_extract_data(struct image_tool_params
> >>>>> *params, const char *fname)
> >>>>> tools/fit_image.c: align_size = params->bl_len ? params->bl_len :
> >>>>> 4;
> >>>>> ...
> >>>>>
> >>>>> So something like that may work ?
> >>>>>
> >>>>> "
> >>>>> diff --git a/tools/fit_image.c b/tools/fit_image.c
> >>>>> index 0306333141e..0c606ba4cc3 100644
> >>>>> --- a/tools/fit_image.c
> >>>>> +++ b/tools/fit_image.c
> >>>>> @@ -642,9 +642,15 @@ static int fit_extract_data(struct
> >>>>> image_tool_params *params, const char *fname)
> >>>>> for (node = fdt_first_subnode(fdt, images);
> >>>>> node >= 0;
> >>>>> node = fdt_next_subnode(fdt, node)) {
> >>>>> - const char *data;
> >>>>> + const char *data, *type;
> >>>>> int len;
> >>>>>
> >>>>> + if (align_size < 8) {
> >>>>> + type = fdt_getprop(fdt, node, FIT_TYPE_PROP,
> >>>>> &len);
> >>>>> + if (type && !strcmp(type, "flat_dt"))
> >>>>> + align_size = 8;
> >>>>> + }
> >>>>> +
> >>>>> data = fdt_getprop(fdt, node, FIT_DATA_PROP, &len);
> >>>>> if (!data)
> >>>>> continue;
> >>>>> "
> >>>>>
> >>>>
> >>>> I just ran a quick test with this diff and it seems to fix the issue for
> >>>> us. If it's okay can we take the change as is.
> >>> I think the strcmp needs to be some strncmp and check the 'len' too.
> >>>
> >>> Let's see what others think.
> >>
> >> strcmp is fine when one of the strings is a constant
> >>
> >> If type is shorter than "flat_dt" then there is no risk of overflow.
> >
> > What exact data would you be comparing here , the constant and something
> > read from possibly past the user supplied "type" buffer ?
> >
> > Some validation of "len" has to be done before it is fed into strcmp() here.
>
> Well if you are concerned about lacking a nul-terminator then we will
> read at most 8 bytes beyond the string.
>
> The other thing that can go wrong is if we get something like
> "flat_dt\0extra data" we will match.
>
> Surprisingly the closest we have to this construct is fdt_string_eq_. I
> suppose this is because most strings in U-Boot are guaranteed to be
> nul-terminated.And gets back to my original point. Either we're fine here, because libfdt is doing enough input validation (type and len come from fdt_getprop(...)) and if that passes enough validation to happen but also allow malicious input this is not the most interesting place to wreck havoc. Or we're fine, actually. Which is what I was saying on IRC. -- Tom
signature.asc
Description: PGP signature
