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 <jlee@suse.com>

Author: joeyli

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