From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from g4t3425.houston.hpe.com (g4t3425.houston.hpe.com [15.241.140.78]) (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 7D85C21D492DE for ; Wed, 13 Sep 2017 09:30:50 -0700 (PDT) Received: from G1W8107.americas.hpqcorp.net (g1w8107.austin.hp.com [16.193.72.59]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by g4t3425.houston.hpe.com (Postfix) with ESMTPS id 91E50BD; Wed, 13 Sep 2017 16:33:47 +0000 (UTC) Received: from G1W8107.americas.hpqcorp.net (2002:10c1:483b::10c1:483b) by G1W8107.americas.hpqcorp.net (2002:10c1:483b::10c1:483b) with Microsoft SMTP Server (TLS) id 15.0.1178.4; Wed, 13 Sep 2017 16:33:40 +0000 Received: from NAM01-BY2-obe.outbound.protection.outlook.com (15.241.52.10) by G1W8107.americas.hpqcorp.net (16.193.72.59) with Microsoft SMTP Server (TLS) id 15.0.1178.4 via Frontend Transport; Wed, 13 Sep 2017 16:33:40 +0000 Received: from DF4PR84MB0155.NAMPRD84.PROD.OUTLOOK.COM (10.162.192.29) by DF4PR84MB0250.NAMPRD84.PROD.OUTLOOK.COM (10.162.193.155) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.35.12; Wed, 13 Sep 2017 16:33:37 +0000 Received: from DF4PR84MB0155.NAMPRD84.PROD.OUTLOOK.COM ([10.162.192.29]) by DF4PR84MB0155.NAMPRD84.PROD.OUTLOOK.COM ([10.162.192.29]) with mapi id 15.20.0056.010; Wed, 13 Sep 2017 16:33:37 +0000 From: "Johnson, Brian (EXL - Eagan)" To: "Wang, Jian J" , "edk2-devel@lists.01.org" CC: "Justen@ml01.01.org" , Eric Dong , "Kinney@ml01.01.org" , Jordan L , "Wolman@ml01.01.org" , Jiewen Yao , Ayellet , Michael D , Laszlo Ersek , Star Zeng Thread-Topic: [edk2] [PATCH 2/4] UefiCpuPkg/PiSmmCpuDxeSmm: Implement NULL pointer detection for SMM mode code. Thread-Index: AQHTLHJFBm3N8RT2wkKIcfTiDyYjXqKy/7eg Date: Wed, 13 Sep 2017 16:33:37 +0000 Message-ID: References: <20170913092507.12504-1-jian.j.wang@intel.com> <20170913092507.12504-3-jian.j.wang@intel.com> In-Reply-To: <20170913092507.12504-3-jian.j.wang@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=brian.johnson@hpe.com; x-originating-ip: [192.48.192.5] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DF4PR84MB0250; 6:lICZ+PYRHIlKcZNPKDamzHMHDCjmBWithV92CNS3vY0Dzb4RBVAgYMBe9pta4qe1GTz0egXXqa5Tlz06d4+8NUZIKRNCouLq6odSh2JNYLznBNZf/78fo2Ry6ggV3iq3I5ICkEYBHH6/E4vVIISu0pkRIsdGUombuJAJ18gnuLG7Ak+hKC3pjD3jIkVFKHR0Z6IiL6mM01XfX77rA8oEC5a5XTjzK/VW+ZrZDQ/4DNAYQPoy8Ylqdbqtg0Ceb9OQ55NV0qI/6EtasSsjOApv4mnyiaq/M5nki6t1XHL6QWMzXRLpSPX9y5rgSz4xw4Gy7gp/zIL89k5xv2SH33ughA==; 5:KUpsoD8ExcYzjv4rX+YJ+LmKVOo/KAkjTRTLl+iErRPlUM6OAeg8u5MMVazKcaOQo82z7esh2JjSU8cXhT+MT3moiQ4wfv0WPAYwyeuun5mWujzMgo4jEDkOivqsoicEEwJLIJXy4pLdW14C/AegGQ==; 24:2lo8beHjSNyGYw9nDrBhsGpNHIZ8LiQ6yFFNe05JrV2VwpuXmivwjwZny5FHHdVRxol9NLbHAFlPD2P28lVLqUnACqlgjWrY2CekKWPEW90=; 7:IHJVEn9ftoIZuDQibIfyctNWRzOD00PNMOZi85bsEmzza+Wc69N2nl1XaI+84SfmJCEXXsU89SYHgw6Zz3IigivA+cwaLN0gCHp42G3u4jcV79/uE0Ej6dbTdGLM96pjHOBo8jFaPV4WVC/x45GWuvknOqsJKTTKp/hLFWTmZ+OHHSE/9yGlkIvTbQ7wKqiC6mM9U/fqxxA0Cuj6tUwo++GVXfpGU4pescwKk9FjKQ4= x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: a003751b-0c37-48cd-4671-08d4fac52f32 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(300000502095)(300135100095)(22001)(2017030254152)(48565401081)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:DF4PR84MB0250; x-ms-traffictypediagnostic: DF4PR84MB0250: x-exchange-antispam-report-test: UriScan:(162533806227266)(228905959029699); x-microsoft-antispam-prvs: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(3002001)(93006095)(93001095)(100000703101)(100105400095)(10201501046)(6055026)(6041248)(20161123564025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123558100)(20161123555025)(20161123562025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:DF4PR84MB0250; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:DF4PR84MB0250; x-forefront-prvs: 042957ACD7 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(6029001)(39860400002)(346002)(376002)(366002)(13464003)(189002)(377454003)(199003)(8936002)(6506006)(77096006)(68736007)(6436002)(2900100001)(74316002)(966005)(81166006)(316002)(8676002)(7696004)(86362001)(575784001)(5660300001)(33656002)(25786009)(4326008)(189998001)(2950100002)(105586002)(9686003)(6306002)(3280700002)(2501003)(305945005)(101416001)(6116002)(102836003)(3846002)(3660700001)(55016002)(54906002)(106356001)(229853002)(53546010)(7736002)(76176999)(97736004)(14454004)(66066001)(478600001)(2906002)(6246003)(7416002)(53936002)(50986999)(81156014)(54356999)(19627235001); DIR:OUT; SFP:1102; SCL:1; SRVR:DF4PR84MB0250; H:DF4PR84MB0155.NAMPRD84.PROD.OUTLOOK.COM; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; received-spf: None (protection.outlook.com: hpe.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Sep 2017 16:33:37.2626 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR84MB0250 X-OriginatorOrg: hpe.com Subject: Re: [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 16:30:50 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Comments below. Brian -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wang= , Jian J Sent: Wednesday, September 13, 2017 4:25 AM To: edk2-devel@lists.01.org Cc: Justen@ml01.01.org; Eric Dong ; Kinney@ml01.01.org= ; Jordan L ; Wolman@ml01.01.org; Jiewen Yao ; Ayellet ; Michael D ; Laszlo Ersek ; Star Zeng Subject: [edk2] [PATCH 2/4] UefiCpuPkg/PiSmmCpuDxeSmm: Implement NULL point= er detection for SMM mode code. The mechanism behind is the same as NULL pointer detection enabled in EDK-I= I core. SMM has its own page table and we have to disable page 0 again in S= MM 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/PiSmmCpu= DxeSmm/Ia32/PageTbl.c index f295c2ebf2..d423958783 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c @@ -155,6 +155,17 @@ SmiPFHandler ( } } =20 + // + // If NULL pointer was just accessed + // + if (NULL_DETECTION_ENABLED && (PFAddress >=3D 0 && PFAddress < EFI_PAGE_= SIZE)) { [Brian] PFAddress is unsigned, so it will always be >=3D 0. Some compilers= complain about this.... Should probably remove that part of the test. + 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/PiSmmCpuDxe= Smm/MpService.c index f086b97c30..81c5ac9d11 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c @@ -855,10 +855,10 @@ Gen4GPageTable ( Pte[Index] =3D (Index << 21) | mAddressEncMask | IA32_PG_PS | PAGE_ATT= RIBUTE_BITS; } =20 + Pdpte =3D (UINT64*)PageTable; if (FeaturePcdGet (PcdCpuSmmStackGuard)) { Pages =3D (UINTN)PageTable + EFI_PAGES_TO_SIZE (5); GuardPage =3D mSmmStackArrayBase + EFI_PAGE_SIZE; - Pdpte =3D (UINT64*)PageTable; for (PageIndex =3D Low2MBoundary; PageIndex <=3D High2MBoundary; PageI= ndex +=3D SIZE_2MB) { Pte =3D (UINT64*)(UINTN)(Pdpte[BitFieldRead32 ((UINT32)PageIndex, 30= , 31)] & ~mAddressEncMask & ~(EFI_PAGE_SIZE - 1)); Pte[BitFieldRead32 ((UINT32)PageIndex, 21, 29)] =3D (UINT64)Pages | = mAddressEncMask | PAGE_ATTRIBUTE_BITS; @@ -886,6 +886,29 @@ Gen4GPageTable ( } } =20 + if (NULL_DETECTION_ENABLED) { + Pte =3D (UINT64*)(UINT64)(Pdpte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZ= E - 1)); [Brian] Shouldn't the inner cast be (UINTN), not (UINT64)? That would matc= h the PcdCpuSmmStackGuard section above. + if ((Pte[0] & IA32_PG_PS) =3D=3D 0) { + // 4K-page entries are already mapped. Just hide the first one anywa= y. + Pte =3D (UINT64*)(UINT64)(Pte[0] & ~mAddressEncMask & ~(EFI_PAGE_SIZ= E - 1)); [Brian] Same comment re. the inner cast. + Pte[0] &=3D ~1; // Hide page 0 + } else { + // Create 4K-page entries + Pages =3D (UINTN)AllocatePageTableMemory (1); + ASSERT (Pages !=3D 0); + + Pte[0] =3D (UINT64)(Pages | mAddressEncMask | PAGE_ATTRIBUTE_BITS); + + Pte =3D (UINT64*)Pages; + PageAddress =3D 0; + Pte[0] =3D PageAddress | mAddressEncMask; // Hide page 0 but present= left + for (Index =3D 1; Index < EFI_PAGE_SIZE / sizeof (*Pte); Index++) { + PageAddress +=3D EFI_PAGE_SIZE; + Pte[Index] =3D PageAddress | mAddressEncMask | PAGE_ATTRIBUTE_BITS= ; + } + } + } + return (UINT32)(UINTN)PageTable; } =20 diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmC= puDxeSmm/PiSmmCpuDxeSmm.h index 1cf85c1481..bcb3032db8 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -153,6 +153,8 @@ typedef UINT32 SMM_CPU_ARR= IVAL_EXCEPTIONS; #define ARRIVAL_EXCEPTION_DELAYED 0x2 #define ARRIVAL_EXCEPTION_SMI_DISABLED 0x4 =20 +#define NULL_DETECTION_ENABLED ((PcdGet8(PcdNullPointerDetectionPropert= yMask) & BIT1) !=3D 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/PiSm= mCpuDxeSmm/PiSmmCpuDxeSmm.inf index 099792e6ce..57a14d9f24 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf @@ -138,14 +138,14 @@ gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable =20 [FeaturePcd] - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ## CONS= UMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmBlockStartupThisAp ## CONS= UMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection ## CONS= UMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport ## CONS= UMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONS= UMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileEnable ## CONS= UMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer ## CONS= UMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock ## CONS= UMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ## CO= NSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmBlockStartupThisAp ## CO= NSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection ## CO= NSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugSupport ## CO= NSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CO= NSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileEnable ## CO= NSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer ## CO= NSUMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock ## CO= NSUMES =20 [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## SOME= TIMES_CONSUMES @@ -159,6 +159,7 @@ gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable ## CONS= UMES gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## CONS= UMES gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask ##= CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ##= CONSUMES =20 [Depex] gEfiMpServiceProtocolGuid diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuD= xeSmm/X64/PageTbl.c index 3dde80f9ba..e67bcfe0f6 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c @@ -872,6 +872,17 @@ SmiPFHandler ( } } =20 + // + // If NULL pointer was just accessed + // + if (NULL_DETECTION_ENABLED && (PFAddress >=3D 0 && PFAddress < EFI_PAGE_= SIZE)) { [Brian] PFAddress is unsigned, so it will always be >=3D 0. Some compilers= complain about this.... Should probably remove that part of the test. + DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n")); + DEBUG_CODE ( + DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextX64->Rip); + ); + CpuDeadLoop (); + } + if (FeaturePcdGet (PcdCpuSmmProfileEnable)) { SmmProfilePFHandler ( SystemContext.SystemContextX64->Rip, --=20 2.14.1.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel