[PATCH 1/1] efi_loader: validate PE-COFF relocation data
Anas Cherni
anas at calif.io
Mon May 18 10:03:14 CEST 2026
Hello,
Could you please change the reporter section to:
Anthropic Research and Claude, in collaboration with calif.io
Best regards,
Anas
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