[PATCH 1/1] efi_loader: validate PE-COFF relocation data
Heinrich Schuchardt
xypron.glpk at gmx.de
Mon May 18 10:30:05 CEST 2026
Am 18. Mai 2026 10:03:14 MESZ schrieb Anas Cherni <anas at calif.io>:
>Hello,
>
>Could you please change the reporter section to:
>Anthropic Research and Claude, in collaboration with calif.io
>
>Best regards,
>Anas
Reported-by: needs a name and an email address and you are the person who reported the issue to the U-Boot project.
Best regards
Heinrich
>
>
>On Mon, May 18, 2026 at 8:40 Ilias Apalodimas <ilias.apalodimas at linaro.org>
>wrote:
>
>> On Mon, 18 May 2026 at 09:30, Heinrich Schuchardt <xypron.glpk at gmx.de>
>> wrote:
>> >
>> > When applying base relocations from a PE-COFF binary all data must
>> > be treated as untrusted. Add the following checks to
>> > efi_loader_relocate():
>> >
>> > * Reject relocation blocks that don't start on a 32-bit aligned
>> > address.
>> > * Reject relocation blocks whose SizeOfBlock is smaller than the
>> > block header, which would cause an unsigned underflow when computing
>> > the entry count.
>> > * A block with SizeOfBlock == 0 is invalid and does not mark the end of
>> > the relocation table.
>> > * Reject relocation blocks that extend beyond the end of the
>> > relocation section.
>> > * Reject individual relocation entries whose target offset, together
>> > with the access width, exceeds the mapped image size, preventing
>> > out-of-bounds writes.
>> >
>> > Pass virt_size to efi_loader_relocate() from efi_load_pe() to enable
>> > the per-entry bounds check.
>> >
>> > Reported-by: Anas Cherni <anas at calif.io>
>> > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> > ---
>>
>> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>
>> > lib/efi_loader/efi_image_loader.c | 76 ++++++++++++++++++++++++++-----
>> > 1 file changed, 65 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/lib/efi_loader/efi_image_loader.c
>> b/lib/efi_loader/efi_image_loader.c
>> > index d002eb0c744..0bc66a0f732 100644
>> > --- a/lib/efi_loader/efi_image_loader.c
>> > +++ b/lib/efi_loader/efi_image_loader.c
>> > @@ -111,8 +111,9 @@ void efi_print_image_infos(void *pc)
>> > * Return: status code
>> > */
>> > static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION
>> *rel,
>> > - unsigned long rel_size, void *efi_reloc,
>> > - unsigned long pref_address)
>> > + unsigned long rel_size, void
>> *efi_reloc,
>> > + unsigned long pref_address,
>> > + unsigned long virt_size)
>> > {
>> > unsigned long delta = (unsigned long)efi_reloc - pref_address;
>> > const IMAGE_BASE_RELOCATION *end;
>> > @@ -122,34 +123,86 @@ static efi_status_t efi_loader_relocate(const
>> IMAGE_BASE_RELOCATION *rel,
>> > return EFI_SUCCESS;
>> >
>> > end = (const IMAGE_BASE_RELOCATION *)((const char *)rel +
>> rel_size);
>> > - while (rel + 1 < end && rel->SizeOfBlock) {
>> > + while (rel + 1 < end) {
>> > const uint16_t *relocs = (const uint16_t *)(rel + 1);
>> > +
>> > + /* Each block must start on a 32-bit boundary */
>> > + if (!IS_ALIGNED((uintptr_t)rel, sizeof(uint32_t))) {
>> > + log_debug("Relocation block not 32-bit
>> aligned\n");
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > + /* Relocation block cannot be shorter than its header */
>> > + if (rel->SizeOfBlock < sizeof(*rel)) {
>> > + log_debug("Relocation block too small: %u\n",
>> > + rel->SizeOfBlock);
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > + /* All relocation entries must be inside the .reloc
>> section */
>> > + if ((const char *)rel + rel->SizeOfBlock > (const char
>> *)end) {
>> > + log_debug("Relocation block exceeds relocation
>> data\n");
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > i = (rel->SizeOfBlock - sizeof(*rel)) / sizeof(uint16_t);
>> > while (i--) {
>> > - uint32_t offset = (uint32_t)(*relocs & 0xfff) +
>> > - rel->VirtualAddress;
>> > + uint32_t entry_offset = *relocs & 0xfff;
>> > + unsigned long offset;
>> > int type = *relocs >> EFI_PAGE_SHIFT;
>> > - uint64_t *x64 = efi_reloc + offset;
>> > - uint32_t *x32 = efi_reloc + offset;
>> > - uint16_t *x16 = efi_reloc + offset;
>> > + uint64_t *x64;
>> > + uint32_t *x32;
>> > + uint16_t *x16;
>> > +
>> > + /*
>> > + * This first check covers VirtualAddress +
>> Offset
>> > + * resulting in an overflow.
>> > + */
>> > + if (rel->VirtualAddress > virt_size ||
>> > + entry_offset > virt_size -
>> rel->VirtualAddress) {
>> > + log_debug("relocation address out of
>> bounds\n");
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > +
>> > + offset = rel->VirtualAddress + entry_offset;
>> > + x64 = efi_reloc + offset;
>> > + x32 = efi_reloc + offset;
>> > + x16 = efi_reloc + offset;
>> >
>> > switch (type) {
>> > case IMAGE_REL_BASED_ABSOLUTE:
>> > break;
>> > case IMAGE_REL_BASED_HIGH:
>> > + if (sizeof(uint16_t) > virt_size -
>> offset) {
>> > + log_debug("relocation address
>> out of bounds\n");
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > *x16 += ((uint32_t)delta) >> 16;
>> > break;
>> > case IMAGE_REL_BASED_LOW:
>> > + if (sizeof(uint16_t) > virt_size -
>> offset) {
>> > + log_debug("relocation address
>> out of bounds\n");
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > *x16 += (uint16_t)delta;
>> > break;
>> > case IMAGE_REL_BASED_HIGHLOW:
>> > + if (sizeof(uint32_t) > virt_size -
>> offset) {
>> > + log_debug("relocation address
>> out of bounds\n");
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > *x32 += (uint32_t)delta;
>> > break;
>> > case IMAGE_REL_BASED_DIR64:
>> > + if (sizeof(uint64_t) > virt_size -
>> offset) {
>> > + log_debug("relocation address
>> out of bounds\n");
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > *x64 += (uint64_t)delta;
>> > break;
>> > #ifdef __riscv
>> > case IMAGE_REL_BASED_RISCV_HI20:
>> > + if (sizeof(uint32_t) > virt_size -
>> offset) {
>> > + log_debug("relocation address
>> out of bounds\n");
>> > + return EFI_LOAD_ERROR;
>> > + }
>> > *x32 = ((*x32 & 0xfffff000) +
>> (uint32_t)delta) |
>> > (*x32 & 0x00000fff);
>> > break;
>> > @@ -163,7 +216,7 @@ static efi_status_t efi_loader_relocate(const
>> IMAGE_BASE_RELOCATION *rel,
>> > break;
>> > #endif
>> > default:
>> > - log_err("Unknown Relocation off %x type
>> %x\n",
>> > + log_err("Unknown Relocation off %lx type
>> %x\n",
>> > offset, type);
>> > return EFI_LOAD_ERROR;
>> > }
>> > @@ -970,8 +1023,9 @@ efi_status_t efi_load_pe(struct
>> efi_loaded_image_obj *handle,
>> >
>> > /* Run through relocations */
>> > if (efi_loader_relocate(rel, rel_size, efi_reloc,
>> > - (unsigned long)image_base) !=
>> EFI_SUCCESS) {
>> > - efi_free_pages((uintptr_t) efi_reloc,
>> > + (unsigned long)image_base,
>> > + virt_size) != EFI_SUCCESS) {
>> > + efi_free_pages((uintptr_t)efi_reloc,
>> > (virt_size + EFI_PAGE_MASK) >>
>> EFI_PAGE_SHIFT);
>> > ret = EFI_LOAD_ERROR;
>> > goto err;
>> > --
>> > 2.53.0
>> >
>>
More information about the U-Boot
mailing list