From 6af07aabe46eb4abe71cd88a995224f4cf57e63a Mon Sep 17 00:00:00 2001 From: Chun-Yi Lee Date: Tue, 15 Apr 2025 10:17:38 +0800 Subject: [PATCH] pe: handle_image: skipping update memory attributes when image does not support NX By default, the MOK_POLICY_REQUIRE_NX bit is disabled which means that it is permissible to load binaries which do not support NX mitigations. But many old grub2 are not really tested with update memory attributes protocol. Especially they are not page alignment. We may got similar page fault exception as the following: memattrs.c:269:efi_get_mem_attrs() efi_get_mem_attrs called on 0x7DA7AA00-0x7DA7B9FF and attrs 0x7FED45A0 Invalid call efi_update_mem_attrs(addr:0x7DA7AA00-0x7DA7B9FF, size:0x1000, +r, -) addr is not page aligned !!!! X64 Exception Type - 0E(#PF - Page-Fault) CPU Apic ID - 00000000 !!!! ExceptionData - 0000000000000011 I:1 R:0 U:0 W:0 P:1 PK:0 SS:0 SGX:0 RIP - 000000007D972400, CS - 0000000000000038, RFLAGS - 0000000000010202 RAX - 0000000000000000, RCX - 000000007E467A98, RDX - 000000007F9EC018 RBX - 0000000000000000, RSP - 000000007FED46D8, RBP - 000000007E467A98 RSI - 000000007D972000, RDI - 000000007DD5AD40 R8 - 0000000000000036, R9 - 000000007DD51D18, R10 - 0000000000000000 R11 - 0000000000000002, R12 - 000000007E467A98, R13 - 000000007DC14CE8 R14 - 000000007DC14BA0, R15 - 000000007DC42410 DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 GS - 0000000000000030, SS - 0000000000000030 CR0 - 0000000080010033, CR2 - 000000007D972400, CR3 - 000000007FC01000 CR4 - 0000000000000668, CR8 - 0000000000000000 DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 GDTR - 000000007F9DC000 0000000000000047, LDTR - 0000000000000000 IDTR - 000000007F192018 0000000000000FFF, TR - 0000000000000000 FXSAVE_STATE - 000000007FED4330 !!!! Find image based on IP(0x7DBAE663) (No PDB) (ImageBase=000000007DB87000, EntryPoint=000000007DBAC000) !!!! So let's skip updating memory attributes when the image does not support NX. (aka. the EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT dll characteristics is not raised) Signed-off-by: Chun-Yi Lee --- pe.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/pe.c b/pe.c index 7e48aaab1..c95035f39 100644 --- a/pe.c +++ b/pe.c @@ -605,6 +605,7 @@ handle_image (void *data, unsigned int datasize, int found_entry_point = 0; UINT8 sha1hash[SHA1_DIGEST_SIZE]; UINT8 sha256hash[SHA256_DIGEST_SIZE]; + bool nx_compat = 0; /* * The binary header contains relevant context and section pointers @@ -697,12 +698,16 @@ handle_image (void *data, unsigned int datasize, return EFI_OUT_OF_RESOURCES; } + nx_compat = (context.DllCharacteristics & EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT); buffer = (void *)ALIGN_VALUE((unsigned long)*alloc_address, *alloc_alignment); dprint(L"Loading 0x%llx bytes at 0x%llx\n", (unsigned long long)context.ImageSize, (unsigned long long)(uintptr_t)buffer); - update_mem_attrs((uintptr_t)buffer, alloc_size, MEM_ATTR_R|MEM_ATTR_W, - MEM_ATTR_X); + if (nx_compat) + update_mem_attrs((uintptr_t)buffer, alloc_size, + MEM_ATTR_R|MEM_ATTR_W, MEM_ATTR_X); + else + perror(L"Image does not support NX\n"); CopyMem(buffer, data, context.SizeOfHeaders); @@ -872,7 +877,6 @@ handle_image (void *data, unsigned int datasize, uint64_t clear_attrs = MEM_ATTR_W|MEM_ATTR_X; uintptr_t addr; uint64_t raw_length; - uint64_t length; /* * Skip discardable sections with zero size @@ -899,17 +903,25 @@ handle_image (void *data, unsigned int datasize, // granularity, but the section length (unlike the section // address) is not required to be aligned. raw_length = (uintptr_t)end - (uintptr_t)base + 1; - length = ALIGN_VALUE(raw_length, PAGE_SIZE); - if (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) { - set_attrs |= MEM_ATTR_W; - clear_attrs &= ~MEM_ATTR_W; - } - if (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) { - set_attrs |= MEM_ATTR_X; - clear_attrs &= ~MEM_ATTR_X; + /* + * Now set the page permissions appropriately if we're + * enforcing NX compat. + */ + if (!nx_compat) { + uint64_t length; + length = ALIGN_VALUE(raw_length, PAGE_SIZE); + + if (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) { + set_attrs |= MEM_ATTR_W; + clear_attrs &= ~MEM_ATTR_W; + } + if (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) { + set_attrs |= MEM_ATTR_X; + clear_attrs &= ~MEM_ATTR_X; + } + update_mem_attrs(addr, length, set_attrs, clear_attrs); } - update_mem_attrs(addr, length, set_attrs, clear_attrs); /* * We only cache CODE and INITIALIZED data sections that