Am 18. Mai 2026 10:03:14 MESZ schrieb Anas Cherni <[email protected]>: >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 <[email protected]> >wrote: > >> On Mon, 18 May 2026 at 09:30, Heinrich Schuchardt <[email protected]> >> 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 <[email protected]> >> > Signed-off-by: Heinrich Schuchardt <[email protected]> >> > --- >> >> Reviewed-by: Ilias Apalodimas <[email protected]> >> >> > 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 >> > >>
