From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 5379821DF806E for ; Tue, 29 Aug 2017 08:59:41 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP; 29 Aug 2017 09:02:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,445,1498546800"; d="scan'208";a="1212019246" Received: from orsmsx103.amr.corp.intel.com ([10.22.225.130]) by fmsmga002.fm.intel.com with ESMTP; 29 Aug 2017 09:02:21 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.25]) by ORSMSX103.amr.corp.intel.com ([169.254.5.176]) with mapi id 14.03.0319.002; Tue, 29 Aug 2017 09:02:21 -0700 From: "Kinney, Michael D" To: "Johnson, Brian (EXL - Eagan)" , "Yao, Jiewen" , "Wang, Jian J" , "edk2-devel@lists.01.org" , "Kinney, Michael D" Thread-Topic: [edk2] [PATCH 0/2] Implement NULL pointer detection feature Thread-Index: AQHTH6igmlFL3+eCMU2g9vAosof696KZjFAAgAAD+YCAAAQhgIACVTWA//+XPeA= Date: Tue, 29 Aug 2017 16:02:20 +0000 Message-ID: References: <20170828025109.5032-1-jian.j.wang@intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A9A0884@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A9A09CB@shsmsx102.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.22.254.140] MIME-Version: 1.0 Subject: Re: [PATCH 0/2] Implement NULL pointer detection feature 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: Tue, 29 Aug 2017 15:59:41 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Brian, Do you prefer 4 new PCDs or one new PCD with a bitmask to enable detection in different phases? Mike > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > Behalf Of Johnson, Brian (EXL - Eagan) > Sent: Tuesday, August 29, 2017 8:17 AM > To: Yao, Jiewen ; Wang, Jian J > ; edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH 0/2] Implement NULL pointer > detection feature >=20 > Thank you for implementing this feature! It is very helpful > for catching pointer-related problems. We have used a similar > scheme on our systems for years, and caught several important > bugs. Some comments: >=20 > * It's possible to implement similar protections in PEI (IA32) > by modifying the GDT descriptors. >=20 > * For flexibility, I'd like NULL pointer protection to be > controlled independently in PEI, DXE, and SMM, using separate > PCDs. >=20 > * We have seen various option ROMs and OS boot loaders which > have NULL pointer issues, but are outside of our control. It > is useful to enable NULL pointer protection during as much of > the boot as possible, but disable it before running these other > executables. So I'd suggest adding another PCD, perhaps > PcdNullPointerDetectionPostDxe, to control NULL pointer > protection late in boot. If PcdNullPointerDetection !=3D > PcdNullPointerDetectionPostDxe, install an end-of-DXE event > (gEfiEndOfDxeEventGroupGuid) which changes the protection of > page 0 using a call to EFI_CPU_ARCH_PROTOCOL. > SetMemoryAttributes(CpuArch, 0, EFI_PAGE_SIZE, 0). >=20 > So ideally I'd like to have 4 PCDs: >=20 > PcdNullPointerDetectionPei > PcdNullPointerDetectionDxe > PcdNullPointerDetectionSmm > PcdNullPointerDetectionPostDxe >=20 > Thanks, > Brian Johnson > HPE >=20 > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > Behalf Of Yao, Jiewen > Sent: Sunday, August 27, 2017 10:39 PM > To: Wang, Jian J ; edk2- > devel@lists.01.org > Subject: Re: [edk2] [PATCH 0/2] Implement NULL pointer > detection feature >=20 > Comment in line. >=20 > From: Wang, Jian J > Sent: Monday, August 28, 2017 11:24 AM > To: Yao, Jiewen ; edk2-devel@lists.01.org > Subject: RE: [edk2] [PATCH 0/2] Implement NULL pointer > detection feature >=20 >=20 > 1) I think this feature should be 'FALSE' by default. I > forgot to reset its default value. This feature makes use of > page mechanism to detect NULL pointer. So any ARCH doesn't > support paging in EDK-II can't use it. Currently validated > platform includes VLV2 and Denlow. Let me know if all platform > must be validated or not. >=20 > [Jiewen] Yes, I am OK to set to be FALSE to provide best > compatibility. > I suggest we validate all open source IA platforms, such as > Quark, and OVMF with TRUE. >=20 >=20 > 2) It's hard to make it a dynamic feature because we need > to setup page table for physical address 0-4095 in advance. If > there's no memory alloc/free action after enabling this > feature, there's no chance to make those change in page table. > Then the usage of feature will be limited in such case. >=20 > [Jiewen] Dynamic means the initial value is dynamic. I am not > saying we need modify the page table. >=20 > An implementation could be: > A platform can set this PCD in PEI phase based upon a setup > variable to choose CSM enable or disable. >=20 > The IPL sets page table based upon this PCD value. The DXE Core > cannot consume PCD directly, because it might be dynamic. But > we can pass the information from IPL via HOB. All the DXE > module just checks the value based upon HOB. >=20 >=20 >=20 >=20 > From: Yao, Jiewen > Sent: Monday, August 28, 2017 11:10 AM > To: Wang, Jian J > >; edk2- > devel@lists.01.org > Subject: RE: [edk2] [PATCH 0/2] Implement NULL pointer > detection feature >=20 > Thank you to enable this feature. >=20 > I have 2 comments, after a very quick review. >=20 >=20 > 1) I notice it is enabled by default > "gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetection|TRUE". >=20 > Would you please provide the information on how many open > source platforms are validated? > Such as, IA platform (VLV2, Quark), emu platform (OVMF, NT32)? > Or do we need validate any ARM platform? >=20 >=20 >=20 > 2) I am thinking about CSM platform. Do you think we can > make it dynamic, as such, a platform may set the validate based > upon CSM enable/disable? >=20 >=20 > Or if we need update the CSM module to patch the page table > automatically? Once this is feature is ON. >=20 >=20 > Thank you > Yao Jiewen >=20 >=20 > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > Behalf Of Wang, > > Jian J > > Sent: Monday, August 28, 2017 10:51 AM > > To: edk2-devel@lists.01.org > > Subject: [edk2] [PATCH 0/2] Implement NULL pointer detection > feature > > > > This patch is the implementation of NULL pointer detection > feature, > > which is one of the small features of Special Pool. > > > > Wang, Jian J (2): > > Implement NULL pointer detection for EDK-II Core > > Implement NULL pointer detection for EDK-II SMM Core and > driver > > > > MdeModulePkg/Core/Dxe/DxeMain.inf | 3 ++- > > MdeModulePkg/Core/Dxe/Mem/Page.c | 5 +++-- > > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 + > > MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 6 ++++-- > > MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 26 > > ++++++++++++++++-------- > > MdeModulePkg/MdeModulePkg.dec | 7 > +++++++ > > UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 12 > +++++++++++ > > UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c | 25 > > ++++++++++++++++++++++- > > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + > > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 12 > +++++++++++ > > 10 files changed, 84 insertions(+), 14 deletions(-) > > > > -- > > 2.11.0.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel