On 2/9/21 7:48 AM, Heinrich Schuchardt wrote:
Am 9. Februar 2021 07:19:48 MEZ schrieb Asherah Connor <a...@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 <a...@kivikakk.ee>
CC: Heinrich Schuchardt <xypron.g...@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?

The PE-COFF spec [1] says:

SizeOfRawData

The size of the section (for object files) or the size of the
initialized data on disk (for image files). For executable images, this
must be a multiple of FileAlignment from the optional header. If this is
less than VirtualSize, the remainder of the section is zero-filled.
Because the SizeOfRawData field is rounded but the VirtualSize field is
not, it is possible for SizeOfRawData to be greater than VirtualSize as
well. When a section contains only uninitialized data, this field should
be zero.

So SizeOfRawData can be both smaller and greater then VirtualSize.

We zero the memory before copying the data. This covers the case
SizeOfRawData < VirtualSize.

Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de>

[1] https://docs.microsoft.com/en-us/windows/win32/debug/pe-format


Best regards

Heinrich



        /* Run through relocations */


Reply via email to