Re: [PATCH 4/4] efi: loader: bounds-check relocation offsets against image size

Ahmad Fatoum
2026-04-14T08:52:50Z

Hi,

On 4/13/26 2:36 PM, Sascha Hauer wrote:
> From: Sascha Hauer <sascha@saschahauer.de>
>
> The relocation VirtualAddress and page offset are read from the PE
> image and used to compute a write offset into the allocated image
> buffer without any bounds checking. A crafted relocation entry with a
> large VirtualAddress writes past the efi_reloc allocation.
>
> Pass the image size into efi_loader_relocate() and use size_add() to
> check that the relocation offset plus the access width fits within the
> buffer. size_add() saturates to SIZE_MAX on overflow, so the check
> remains correct on 32-bit architectures.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>
> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

AFAICS, a 32-bit relocation at the absolute end of a binary would
erroneously trigger this check, but I think that's not worth
complicating the logic over.

Should that ever happen there's a helpful error message..

Cheers,
Ahmad

> ---
> efi/loader/pe.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/efi/loader/pe.c b/efi/loader/pe.c
> index ea385c8795..7d36d8c334 100644
> --- a/efi/loader/pe.c
> +++ b/efi/loader/pe.c
> @@ -108,7 +108,8 @@ void efi_print_image_infos(void *pc)
> */
> 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 pref_address,
> + unsigned long image_size)
> {
> unsigned long delta = (unsigned long)efi_reloc - pref_address;
> const IMAGE_BASE_RELOCATION *end;
> @@ -133,6 +134,11 @@ static efi_status_t efi_loader_relocate(const IMAGE_BASE_RELOCATION *rel,
> uint32_t *x32 = efi_reloc + offset;
> uint16_t *x16 = efi_reloc + offset;
>
> + if (size_add(offset, sizeof(uint64_t)) > image_size) {
> + pr_err("Relocation offset exceeds image size\n");
> + return EFI_LOAD_ERROR;
> + }
> +
> switch (type) {
> case IMAGE_REL_BASED_ABSOLUTE:
> break;
> @@ -722,7 +728,8 @@ 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) {
> + (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;

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |