From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E6C9D2118C500 for ; Tue, 6 Nov 2018 16:53:17 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Nov 2018 16:53:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,473,1534834800"; d="scan'208";a="278917652" Received: from shwdeopenpsi777.ccr.corp.intel.com ([10.239.158.27]) by fmsmga006.fm.intel.com with ESMTP; 06 Nov 2018 16:53:16 -0800 From: Jian J Wang To: edk2-devel@lists.01.org Cc: Star Zeng , Jiewen Yao , Ruiyu Ni , Leif Lindholm Date: Wed, 7 Nov 2018 08:53:11 +0800 Message-Id: <20181107005311.5912-3-jian.j.wang@intel.com> X-Mailer: git-send-email 2.16.2.windows.1 In-Reply-To: <20181107005311.5912-1-jian.j.wang@intel.com> References: <20181107005311.5912-1-jian.j.wang@intel.com> Subject: [PATCH v2 2/2] MdeModulePkg/Core: fix ineffective guard page issue X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Nov 2018 00:53:18 -0000 > v2: re-generate this patch per Leif's comments. No logic changes. 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 --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 36 +++++++++++++-------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 98fdc2c618..30798b05b9 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -1135,26 +1135,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 -- 2.16.2.windows.1