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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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 |