[PATCH] efi_loader: don't load beyond VirtualSize

[Heinrich Schuchardt]

Am 9. Februar 2021 07:19:48 MEZ schrieb Asherah Connor <ashe at kivikakk.ee>:
>PE section table entries' SizeOfRawData must be a multiple of
>FileAlignment, and thus may be rounded up and larger than their
>VirtualSize.
>
>We should not load beyond the VirtualSize, which is "the total size of
>the section when loaded into memory" -- we may clobber real data at the
>target in some other section, since we load sections in reverse order
>and sections are usually laid out sequentially.

Thank you for reporting and addressing the issue.

Is this patch related to an observed problem or is it resulting from code review?

Should we load in forward order?

>
>Signed-off-by: Asherah Connor <ashe at kivikakk.ee>
>CC: Heinrich Schuchardt <xypron.glpk at gmx.de>
>---
> lib/efi_loader/efi_image_loader.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/efi_loader/efi_image_loader.c
>b/lib/efi_loader/efi_image_loader.c
>index d4dd9e9433..f53ef367ec 100644
>--- a/lib/efi_loader/efi_image_loader.c
>+++ b/lib/efi_loader/efi_image_loader.c
>@@ -843,7 +843,7 @@ efi_status_t efi_load_pe(struct
>efi_loaded_image_obj *handle,
> 		       sec->Misc.VirtualSize);
> 		memcpy(efi_reloc + sec->VirtualAddress,
> 		       efi + sec->PointerToRawData,
>-		       sec->SizeOfRawData);
>+		       min(sec->Misc.VirtualSize, sec->SizeOfRawData));
> 	}

If SizeOfRawData must be >= VirtualSize, why do we have to consider both fields?

Best regards

Heinrich


> 
> 	/* Run through relocations */