rhboot / shim

UEFI shim loader
Other
848 stars 290 forks source link

shim fails to load mokmanager image with error "2 sections contain entry point" #517

Closed iokomin closed 1 year ago

iokomin commented 1 year ago

Seen mokmanager image load failure "2 sections contain entry point" for shim built on Oracle Linux 9 aarch64:

pe.c:574:generate_hash() 00000010  08 56 85 05 fa 24 3b 9f  9f f0 51 ed 20 fc c6 fc  |.V...$;...Q. ...|
pe.c:1192:handle_image() Loading 0xC3000 bytes at 0x238237000
2 sections contain entry point
Failed to load image: Unsupported

Root cause of this failure is supposedly incorrect entry points detection. In pe.c:handle_image() found_entry_point counter uses SizeOfRawData to calculate image section boundary: https://github.com/rhboot/shim/blob/5c537b3d0cf8c393dad2e61d49aade68f3af1401/pe.c#L1261-L1264

According to PE spec:

VirtualSize
The total size of the section when loaded into memory. If this value is greater than SizeOfRawData, 
the section is zero-padded. This field is valid only for executable images and should be set to zero for object files.

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. 

Proposed replacing Section->SizeOfRawData to Section->Misc.VirtualSize as it should define actual size of the section loaded in memory.

dennis-tseng99 commented 1 year ago

As the spec says: Because the SizeOfRawData field is rounded but the VirtualSize field is not, it is possible for SizeOfRawData to be greater than VirtualSize.

On the other hand, if the image wants to reserve space in memory for any uninitialized data, then the VirtualSize will be larger than the SizeOfRawData.

Therefore, I suggest we could change the codes to:

if (Section->Characteristics & EFI_IMAGE_SCN_CNT_UNINITIALIZED_DATA) {
    if (Section->Misc.VirtualSize > Section->SizeOfRawData) {
        size = Section->Misc.VirtualSize;
    }
}
else {
       size = Section->SizeOfRawData;
}

if (Section->VirtualAddress <= context.EntryPoint &&
    (Section->VirtualAddress + size - 1) > context.EntryPoint)
    found_entry_point++;

Besides, may I suggest you to dump the values of VirtualAddress and EntryPoint ? Normally, only one section, for example, .text can match the following top-half comparison:

if (Section->VirtualAddress <= context.EntryPoint
iokomin commented 1 year ago

@dennis-tseng99 as I understand condition on the line 1261 it checks if entrypoint is within a section in image memory. Then we are ensuring only single section satisfies this requirement.

The issue in our case was that Section->SizeOfRawData was greater than Section->Misc.VirtualSize for section .eh_frame, as a result two sections .eh_frame and .text sections hit this condition. Both .eh_frame and .text are larger in disk than when loaded into memory. Thus proposed solution should solve cases when SizeofRawData exceeds VirtualSize, VirtualSize supposed to define size of the section loaded image into memory not the SizeofRawData.

-- dumping debug values on mmaa64 image load

pe.c:1190:handle_image() Loading 0xC3000 bytes at 0x23824E000
pe.c:1264:handle_image() Section 0 VirtualAddress 0x5000, context.EntryPoint 0x1C000, Section->SizeofRawData 0x17C00, Section->Misc.VirtualSize 0x16C2C
pe.c:1260:handle_image() Section 1 VirtualAddress 0x1C000, context.EntryPoint 0x1C000, Section->SizeofRawData 0x5D000, Section->Misc.VirtualSize 0x5C8BC
pe.c:1264:handle_image() Section 2 VirtualAddress 0x79000, context.EntryPoint 0x1C000, Section->SizeofRawData 0x200, Section->Misc.VirtualSize 0xA
pe.c:1264:handle_image() Section 3 VirtualAddress 0x7B000, context.EntryPoint 0x1C000, Section->SizeofRawData 0x28200, Section->Misc.VirtualSize 0x280BC
pe.c:1264:handle_image() Section 4 VirtualAddress 0xA4000, context.EntryPoint 0x1C000, Section->SizeofRawData 0x200, Section->Misc.VirtualSize 0xF0
pe.c:1264:handle_image() Section 5 VirtualAddress 0xA5000, context.EntryPoint 0x1C000, Section->SizeofRawData 0x1C200, Section->Misc.VirtualSize 0x1C080
pe.c:1264:handle_image() Section 6 VirtualAddress 0xC2000, context.EntryPoint 0x1C000, Section->SizeofRawData 0x200, Section->Misc.VirtualSize 0xBD

-- objdump

# objdump -h mmaa64.efi

mmaa64.efi:     file format pei-aarch64-little

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  0 .eh_frame     00016c2c  0000000000005000  0000000000005000  00000400  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  1 .text         0005c8bc  000000000001c000  000000000001c000  00018000  2**12
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .reloc        0000000a  0000000000079000  0000000000079000  00075000  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 .data         000280bc  000000000007b000  000000000007b000  00075200  2**3
                  CONTENTS, ALLOC, LOAD, DATA
  4 .dynamic      000000f0  00000000000a4000  00000000000a4000  0009d400  2**3
                  CONTENTS, ALLOC, LOAD, DATA
  5 .rela         0001c080  00000000000a5000  00000000000a5000  0009d600  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  6 .sbat         000000bd  00000000000c2000  00000000000c2000  000b9800  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
dennis-tseng99 commented 1 year ago

@iokomin I tested MokManager.efi in different environments, and found that the EntryPoint is always larger than (VirtualAddress(0x5000) + SizeOfRawData - 1) of the section[0], i.e. eh_frame. For example:

VirtualAddress=0x5000, EntryPoint=0x1F000                                           
Misc.VirtualSize=0x19BE8, SizeOfRawData=0x19C00, end-base+1=0x19BE8                 
0000:  2F 34 00 00 00 00 00 00  E8 9B 01 00 00 50 00 00    /4...........P..         
0010:  00 9C 01 00 00 04 00 00  00 00 00 00 00 00 00 00    ................         
0030:  00 00 00 00 40 00 00 40                             ....@..@                 
Section->Characteristics=0x40000040                                                 
handle_image(pe.c): VirtualAddress(0x5000) <= EntryPoint(0x1F000) ?            
handle_image(pe.c): (VirtualAddress(0x5000) + SizeOfRawData(0x19C00) - 1) > EntryPoint(0x1F000) ?                                                                  
found_entry_point=0

Since .eh_frame section is not a executable section, its virtual address must not be counted for found_entry_point.

BTW, can I know your value of file alignment used for SizeOfRawData ?

iokomin commented 1 year ago

@dennis-tseng99 this is exactly the issue, that actual section .eh_frame size in memory is less than limits checked in the condition using in disk size value, calculation introduces "overlap" with .text section. SizeOfRawData should not apply to the loaded data.

As for your testing I need to notice, mokmanager built on OL8 also has EntryPoint larger than (VirtualAddress + SizeOfRawData - 1). This issue observed with newer compiler and binutils versions available on OL9. The difference between scenarios is apparently that in OL9 the sections are bigger.

gcc-11.2.1-9.4.0.2.el9.aarch64
binutils-2.35.2-17.0.1.el9.aarch64

shim version is built from 15.6 tag, file alignment set according to default project build instructions, result binary - mmaa64.zip

dennis-tseng99 commented 1 year ago

@iokomin I just upgraded my gcc version from v7.xxx to v11.3.0, and then re-compiled and re-loaded MokManager.efi. It seems everything is fine. But because .eh_frame section is closely related to what our compiler selected, maybe we should take a look about what the gcc code base is for OL9. Many thanks.