Skipping update memory attributes when image does not support NX

[joeyli]

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)

[vathpela]

I think this makes some sense, but along with the conflict needing to be resolved, we need to decide what to do in the UKI case with an embedded kernel section.

[vathpela]

So I think I've got it fixed up for the UKI differences here, but I wonder if we don't actually want to be testing for /enforcement/ of NX and failing earlier as well, because if NX is being enforced, with the current patch here we'll load everything right here but the loaded image won't be executable.

[joeyli]

So I think I've got it fixed up for the UKI differences here, but I wonder if we don't actually want to be testing for /enforcement/ of NX and failing earlier as well, because if NX is being enforced, with the current patch here we'll load everything right here but the loaded image won't be executable.

Hi vathpela,

Very thank for your review and fix to my submit patch against UKI. Sorry for I do not familier with UKI, so I did not give any meaningful suggestion.

About testing enforcement of NX and failing earlier, I agree with your opinion. If enforce NX earlier, then we do not need to load image that's without NX flag.