[PATCH v3] mkimage: Default to 8-byte alignment for DTBs added via -b argument

[Marek Vasut]

On 1/27/26 8:26 PM, Sean Anderson wrote:
> On 1/27/26 14:13, Marek Vasut wrote:
>> On 1/27/26 8:04 PM, Sean Anderson wrote:
>>> On 1/27/26 13:52, Marek Vasut wrote:
>>>> On 1/27/26 5:38 PM, Tom Rini wrote:
>>>>
>>>>>> No, the check in fdt_getprop is only for the name, not the value (which
>>>>>> could be arbitrary binary data including many nul bytes).
>>>>>>
>>>>>> I think the assumption in most of U-Boot is that the provided FDT is
>>>>>> trusted (or at least not maliciously ill-formed).  functions like
>>>>>> image_get_checksum_algo read up to 6 bytes beyond full_name if full_name
>>>>>> == "". It's possible to read off the end of the DTB if this is the very
>>>>>> last string in the strings block and there is no trailing padding. If
>>>>>> we're really concerned about this (e.g. to avoid false-positives with
>>>>>> ASAN), then we should just add (say) 16 bytes to the end of every DTB
>>>>>> when we malloc it.
>>>>>
>>>>> Yes, we rely on the sanity checking in libfdt, which I think the kernel
>>>>> also does.
>>>>>
>>>>> And no, I'm not sure if we care enough about all of these corner cases,
>>>>> but if we do then I'm not sure this right here and now is where to
>>>>> start. It should start with upstream libfdt to see whatever cases aren't
>>>>> handled, and then what cases fall on the callers to deal with wrt
>>>>> security implications.
>>>>>
>>>>> Because I really, really, really, do not want to block fixing booting on
>>>>> some large number of boards because now we're going to depend first on a
>>>>> security audit here.
>>>> So back to my and Sean suggestion:
>>>>
>>>> fdt_string_eq_() does effectively memcmp():
>>>>
>>>> return p && (slen == len) && (memcmp(p, s, len) == 0);
>>>>
>>>> So do this and be done with it ?
>>>>
>>>> type = fdt_getprop(fdt, node, FIT_TYPE_PROP, &len);
>>>> if (type && len == strlen(FIT_TYPE_PROP) && !memcmp(type, "flat_dt", len))
>>>
>>> this is wrong it needs to be
>>>
>>> type && len == sizeof("flat_dt") && !memcmp(type, "flat_dt", len)
>>
>> ... because ... ?
> 
> Because the check you wrote is equivalent to
> 
> if (type && sizeof("flat_dt") == strlen("type") && ...)
> 
> which will always fail.
Oh, that ... you're right.