[PATCH] efi_loader: Assure fitImage from capsule is used from 8-byte aligned address

[Heinrich Schuchardt]

On 11/14/25 08:03, Ilias Apalodimas wrote:
> Hi Marek,
> 
> 
> On Thu, 13 Nov 2025 at 13:56, Marek Vasut
> <marek.vasut+renesas at mailbox.org> wrote:
>>
>> The fitImage may be stored in EFI update capsule at address that
>> is not aligned to 8 bytes. Since fitImage is a DT, new version of
>> libfdt 1.7.2 rejects such an unaligned DT. Patch the code and copy
>> the fitImage into aligned buffer in case it is not aligned. This
>> does increase overhead for unaligned fitImages in EFI capsules, but
>> tries to keep the overhead low for aligned ones.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> 
> At some point I'd prefer getting rid of FIT images for capsules
> overall. It was implemented as an alternative to authenticated capsule
> updates, but since then proper capsule support has been added. The
> patch is ok though for now,
> 
>> ---
>> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> Cc: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>> Cc: Jonathan Humphreys <j-humphreys at ti.com>
>> Cc: Mattijs Korpershoek <mkorpershoek at kernel.org>
>> Cc: Tom Rini <trini at konsulko.com>
>> Cc: Wadim Egorov <w.egorov at phytec.de>
>> Cc: u-boot at lists.denx.de
>> ---
>>   lib/efi_loader/efi_firmware.c | 16 +++++++++++++++-
>>   1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
>> index 216df83de67..953a310190b 100644
>> --- a/lib/efi_loader/efi_firmware.c
>> +++ b/lib/efi_loader/efi_firmware.c
>> @@ -651,6 +651,7 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
>>          efi_status_t status;
>>          struct fmp_state state = { 0 };
>>          char *orig_dfu_env;
>> +       void *img;
>>
>>          EFI_ENTRY("%p %d %p %zu %p %p %p\n", this, image_index, image,
>>                    image_size, vendor_code, progress, abort_reason);
>> @@ -677,7 +678,20 @@ efi_status_t EFIAPI efi_firmware_fit_set_image(
>>                  return EFI_EXIT(EFI_DEVICE_ERROR);
>>          }
>>
>> -       ret = fit_update(image);
>> +       /* Make sure the update fitImage is properly aligned to 8-bytes */
>> +       if ((uintptr_t)image & 7) {
> 
> Can we replace this with IS_ALGINED?

I will fix that in my pull-request:

      if (!IS_ALIGNED((uintptr_t)image, 8)) {


> 
>> +               img = memalign(8, image_size);
>> +               if (!img)
>> +                       return EFI_EXIT(EFI_BAD_BUFFER_SIZE);
>> +               memcpy(img, image, image_size);
>> +       } else {
>> +               img = (void *)image;
>> +       }
>> +
>> +       ret = fit_update(img);
>> +
>> +       if ((uintptr_t)image & 7)

     if (!IS_ALIGNED((uintptr_t)image, 8))

Best regards

Heinrich

>> +               free(img);
> 
> Initialize img to NULL and make the check a bit simpler to read (or
> get rid of it overall)
> 
> Other than that LGTM
> 
> Cheers
> /Ilias
>>
>>          if (env_set("dfu_alt_info", orig_dfu_env))
>>                  log_warning("Unable to restore env variable \"dfu_alt_info\".  Further DFU operations may fail!\n");
>> --
>> 2.51.0
>>