From 0adc6eae9480ca1ae7ac01e239e09b58e447e467 Mon Sep 17 00:00:00 2001 From: Jian J Wang Date: Wed, 7 Nov 2018 08:48:09 +0800 Subject: MdeModulePkg/Core: fix ineffective guard page issue REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1295 This issue originates from following patch which allows to enable paging if PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy (in addition to PcdSetNxForStack) are set to enable related features. 5267926134d17e86672b84fd57b438f05ffa68e1 Due to above change, PcdImageProtectionPolicy will be set to 0 by default in many platforms, which, in turn, cause following code in MdeModulePkg\Core\Dxe\Misc\MemoryProtection.c fail the creation of notify event of CpuArchProtocol. 1138: if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { 1139: Status = CoreCreateEvent ( ... 1142: MemoryProtectionCpuArchProtocolNotify, ... 1145: ); Then following call flow won't be done and Guard pages will not be set as not-present in SetAllGuardPages() eventually. MemoryProtectionCpuArchProtocolNotify() => HeapGuardCpuArchProtocolNotify() => SetAllGuardPages() The solution is removing the if(...) statement so that the notify event will always be created and registered. This won't cause unnecessary code execution because, in the notify event handler, the related PCDs like PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy will be checked again before doing related jobs. Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Leif Lindholm Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Reviewed-by: Star Zeng --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 36 +++++++++++++-------------- 1 file changed, 17 insertions(+), 19 deletions(-) (limited to 'MdeModulePkg/Core/Dxe') diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 8a93c5362a..c43e0d08ae 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -1136,26 +1136,24 @@ CoreInitializeMemoryProtection ( ASSERT (GetPermissionAttributeForMemoryType (EfiBootServicesData) == GetPermissionAttributeForMemoryType (EfiConventionalMemory)); - if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { - Status = CoreCreateEvent ( - EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, - MemoryProtectionCpuArchProtocolNotify, - NULL, - &Event - ); - ASSERT_EFI_ERROR(Status); + Status = CoreCreateEvent ( + EVT_NOTIFY_SIGNAL, + TPL_CALLBACK, + MemoryProtectionCpuArchProtocolNotify, + NULL, + &Event + ); + ASSERT_EFI_ERROR(Status); - // - // Register for protocol notifactions on this event - // - Status = CoreRegisterProtocolNotify ( - &gEfiCpuArchProtocolGuid, - Event, - &Registration - ); - ASSERT_EFI_ERROR(Status); - } + // + // Register for protocol notifactions on this event + // + Status = CoreRegisterProtocolNotify ( + &gEfiCpuArchProtocolGuid, + Event, + &Registration + ); + ASSERT_EFI_ERROR(Status); // // Register a callback to disable NULL pointer detection at EndOfDxe -- cgit v1.2.3