From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 CDAB921E8796F for ; Wed, 13 Sep 2017 02:22:27 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP; 13 Sep 2017 02:25:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,387,1500966000"; d="scan'208";a="150787236" Received: from jwang36-mobl2.ccr.corp.intel.com ([10.239.192.50]) by fmsmga005.fm.intel.com with ESMTP; 13 Sep 2017 02:25:22 -0700 From: "Wang, Jian J" To: edk2-devel@lists.01.org Cc: Jiewen Yao , Eric Dong , Star Zeng , Laszlo Ersek , Justen, Jordan L , Kinney, Michael D , Wolman, Ayellet Date: Wed, 13 Sep 2017 17:25:05 +0800 Message-Id: <20170913092507.12504-3-jian.j.wang@intel.com> X-Mailer: git-send-email 2.14.1.windows.1 In-Reply-To: <20170913092507.12504-1-jian.j.wang@intel.com> References: <20170913092507.12504-1-jian.j.wang@intel.com> Subject: [PATCH 2/4] UefiCpuPkg/PiSmmCpuDxeSmm: Implement NULL pointer detection for SMM mode code. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Sep 2017 09:22:28 -0000 The mechanism behind is the same as NULL pointer detection enabled in EDK-II core. SMM has its own page table and we have to disable page 0 again in SMM mode. Cc: Jiewen Yao Cc: Eric Dong Cc: Star Zeng Cc: Laszlo Ersek Cc: Justen, Jordan L Cc: Kinney, Michael D Cc: Wolman, Ayellet Suggested-by: Wolman, Ayellet Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Wang, Jian J --- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 11 +++++++++++ UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 25 ++++++++++++++++++++++++- UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 2 ++ UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 17 +++++++++-------- UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 11 +++++++++++ 5 files changed, 57 insertions(+), 9 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c index f295c2ebf2..d423958783 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c @@ -155,6 +155,17 @@ SmiPFHandler ( } } + // + // If NULL pointer was just accessed + // + if (NULL_DETECTION_ENABLED && (PFAddress >= 0 && PFAddress < EFI_PAGE_SIZE)) { + DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n")); + DEBUG_CODE ( + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Rip); + ); + CpuDeadLoop (); + } + if (FeaturePcdGet (PcdCpuSmmProfileEnable)) { SmmProfilePFHandler ( SystemContext.SystemContextIa32->Eip, diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c index f086b97c30..81c5ac9d11 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c @@ -855,10 +855,10 @@ Gen4GPageTable ( Pte[Index] = (Index << 21) | mAddressEncMask | IA32_PG_PS | PAGE_ATTRIBUTE_BITS; } + Pdpte = (UINT64*)PageTable; if (FeaturePcdGet (PcdCpuSmmStackGuard)) { Pages = (UINTN)PageTable + EFI_PAGES_TO_SIZE (5); GuardPage = mSmmStackArrayBase + EFI_PAGE_SIZE; - Pdpte = (UINT64*)PageTable; for (PageIndex = Low2MBoundary; PageIndex <= High2MBoundary; PageIndex += SIZE_2MB) { Pte = (UINT64*)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30, 31)] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1)); Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] = (UINT64)Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS; @@ -886,6 +886,29 @@ Gen4GPageTable ( } } + if (NULL_DETECTION_ENABLED) { + Pte = (UINT64*)(UINT64)(Pdpte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1)); + if ((Pte[0] & IA32_PG_PS) == 0) { + // 4K-page entries are already mapped. Just hide the first one anyway. + Pte = (UINT64*)(UINT64)(Pte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1)); + Pte[0] &= ~1; // Hide page 0 + } else { + // Create 4K-page entries + Pages = (UINTN)AllocatePageTableMemory (1); + ASSERT (Pages != 0); + + Pte[0] = (UINT64)(Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS); + + Pte = (UINT64*)Pages; + PageAddress = 0; + Pte[0] = PageAddress | mAddressEncMask; // Hide page 0 but present left + for (Index = 1; Index < EFI_PAGE_SIZE / sizeof (*Pte); Index++) { + PageAddress += EFI_PAGE_SIZE; + Pte[Index] = PageAddress | mAddressEncMask | PAGE_ATTRIBUTE_BITS; + } + } + } + return (UINT32)(UINTN)PageTable; } diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h index 1cf85c1481..bcb3032db8 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -153,6 +153,8 @@ typedef UINT32 SMM_CPU_ARRIVAL_EXCEPTIONS; #define ARRIVAL_EXCEPTION_DELAYED 0x2 #define ARRIVAL_EXCEPTION_SMI_DISABLED 0x4 +#define NULL_DETECTION_ENABLED ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT1) != 0) + // // Private structure for the SMM CPU module that is stored in DXE Runtime memory // Contains the SMM Configuration Protocols that is produced. diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf index 099792e6ce..57a14d9f24 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf @@ -138,14 +138,14 @@ gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable [FeaturePcd] - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ## CONSUMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmBlockStartupThisAp ## CONSUMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection ## CONSUMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport ## CONSUMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileEnable ## CONSUMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer ## CONSUMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmBlockStartupThisAp ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileEnable ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer ## CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock ## CONSUMES [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOMETIMES_CONSUMES @@ -159,6 +159,7 @@ gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES [Depex] gEfiMpServiceProtocolGuid diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c index 3dde80f9ba..e67bcfe0f6 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c @@ -872,6 +872,17 @@ SmiPFHandler ( } } + // + // If NULL pointer was just accessed + // + if (NULL_DETECTION_ENABLED && (PFAddress >= 0 && PFAddress < EFI_PAGE_SIZE)) { + DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n")); + DEBUG_CODE ( + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip); + ); + CpuDeadLoop (); + } + if (FeaturePcdGet (PcdCpuSmmProfileEnable)) { SmmProfilePFHandler ( SystemContext.SystemContextX64->Rip, -- 2.14.1.windows.1