From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from g4t3427.houston.hpe.com (g4t3427.houston.hpe.com [15.241.140.73]) (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 C673921DF807C for ; Wed, 30 Aug 2017 14:36:11 -0700 (PDT) Received: from G9W8456.americas.hpqcorp.net (g9w8456.houston.hp.com [16.216.161.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by g4t3427.houston.hpe.com (Postfix) with ESMTPS id 0288A7A; Wed, 30 Aug 2017 21:36:15 +0000 (UTC) Received: from G4W9122.americas.hpqcorp.net (2002:10d2:1511::10d2:1511) by G9W8456.americas.hpqcorp.net (2002:10d8:a15f::10d8:a15f) with Microsoft SMTP Server (TLS) id 15.0.1178.4; Wed, 30 Aug 2017 21:36:14 +0000 Received: from NAM01-SN1-obe.outbound.protection.outlook.com (15.241.52.12) by G4W9122.americas.hpqcorp.net (16.210.21.17) with Microsoft SMTP Server (TLS) id 15.0.1178.4 via Frontend Transport; Wed, 30 Aug 2017 21:36:14 +0000 Received: from DF4PR84MB0155.NAMPRD84.PROD.OUTLOOK.COM (10.162.192.29) by DF4PR84MB0316.NAMPRD84.PROD.OUTLOOK.COM (10.162.193.30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.13.10; Wed, 30 Aug 2017 21:36:12 +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.01.1385.014; Wed, 30 Aug 2017 21:36:12 +0000 From: "Johnson, Brian (EXL - Eagan)" To: "afish@apple.com" , "Yao, Jiewen" CC: "Wang, Jian J" , "edk2-devel@lists.01.org" Thread-Topic: [edk2] [PATCH 0/2] Implement NULL pointer detection feature Thread-Index: AQHTH6iYqSMZzfKr2UKq1NoCvUgNjKKZFvgAgAAD+YCAAAQggIACRs+QgAGayoCAABm0AIAAU1pQ Date: Wed, 30 Aug 2017 21:36:12 +0000 Message-ID: References: <20170828025109.5032-1-jian.j.wang@intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A9A0884@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A9A09CB@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A9A43E1@shsmsx102.ccr.corp.intel.com> <57E6C9C0-7505-4040-9461-B9836934956F@apple.com> In-Reply-To: <57E6C9C0-7505-4040-9461-B9836934956F@apple.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; DF4PR84MB0316; 6:smHaw7GZrG4dMjQzQJraiFK0Gyw+CGPAKD8UD+L8wfNLqT5Ei4Ga0k8gZSYJFR1fZP78dtTCgYAXXUes4gdGBJd3juC/RZLx90L9eZ5pbgb+6NPZFqGH7huGnlVzk9TtkmITtRYh6YCWdSnBYwLDC5ZJsw/OgisDqcOcFu4YkUPkjueGp++iAnYNnLWbmesiwVbztvF5lyQ5uj71DAESs2aGCbQ4TpNF4qDOxFVwpvyj1LhOaxIso6vK2LT1uo4CScTZ+Ji5k0OKM4czuCryDL3AGsallxIekp6Hh7V4tBeUCKfY+IU6Z56oFqjIiPn0qDsiSPT7vqNW3xb+YmZYzQ==; 5:tzrvOvu2AZyr4GVcpDqZxBiN8rPUUKpTVTt5LnXFRZ87CProgLWeDkSgURm16sJKMgM9Lzr/1GLVfeCwbqYWFCUUPiF9FiqyFLOsAGhuuy8qRJ85cJVnHPL4QYpmDD5MlQDDRzvCSVGEzCU4OWcIqg==; 24:MBN5bRtHKgbx1dkNsV/bvoSyJBqqmRwcNZwEVZcLXX2712FWMrGU0WC7ngp0bfh6Q/+wo0fWIbsFHnkZ9WWERuX/R9Bg85V4Q3gm2MVdW0g=; 7:K1R9YAN2zhU4CaS1DrqQzMxwienhsSgII2QnxapLROTybImGBsCr5ARQ7wOvI9Za/IBttFjjS+8LPqv83qWDXadbCaBIpx3c9cGXHuN7AAX0nJ7lr0LN721QHZ/sLKL+TclvVYA7kU1I/7XR9EvLdCQuDQJdl5Rh/lOQFPGU69tZVkln7TodzPqVlRNQ55ZXSeMgZ7RoEUUX23UbTokfswAOV2gjl8fDNTU+ThzzI00= x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-correlation-id: 0da80e8f-b8f7-42ea-8873-08d4efef22f6 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:DF4PR84MB0316; x-ms-traffictypediagnostic: DF4PR84MB0316: x-exchange-antispam-report-test: UriScan:(227479698468861)(162533806227266)(31960201722614)(228905959029699); x-microsoft-antispam-prvs: x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(5005006)(8121501046)(93006095)(93001095)(3002001)(10201501046)(100000703101)(100105400095)(6055026)(6041248)(20161123555025)(20161123558100)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123562025)(20161123564025)(20161123560025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:DF4PR84MB0316; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:DF4PR84MB0316; x-forefront-prvs: 041517DFAB x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(39860400002)(57704003)(24454002)(189002)(377454003)(199003)(13464003)(6306002)(3660700001)(3846002)(305945005)(53936002)(8666007)(33656002)(25786009)(105586002)(106356001)(14454004)(55016002)(66066001)(68736007)(102836003)(6116002)(966005)(189998001)(74316002)(4326008)(6436002)(54906002)(3280700002)(9686003)(2900100001)(7736002)(2501003)(6246003)(8936002)(2950100002)(76176999)(54356999)(93886005)(53546010)(77096006)(50986999)(229853002)(2906002)(5660300001)(97736004)(6506006)(8676002)(478600001)(86362001)(345774005)(81156014)(101416001)(81166006)(7696004)(19627235001); DIR:OUT; SFP:1102; SCL:1; SRVR:DF4PR84MB0316; 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: 30 Aug 2017 21:36:12.8105 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: DF4PR84MB0316 X-OriginatorOrg: hpe.com 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: Wed, 30 Aug 2017 21:36:12 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jiewen, Certainly PEI could be done later. There's no need to get all the code in = at once, and you have the DXE/SMM part pretty much ready to go. I'm willing to go with a single PCD with a bitmap, or multiple PCDs. Whate= ver seems best. IIRC protecting page 0 in PEI involves modifying the main GCD descriptor to= be a grows-downward variety, with a limit above page 0. That way you get = an exception if page 0 is accessed. I'd have to check on the steps needed = to release the actual code, which may take quite a while. You may be bette= r off just doing it yourself. I'll take another look at the code to make s= ure I'm explaining this properly, though. CSM is one of the few pieces of BIOS which has a legitimate need to access = page 0. We get the best protection by having it enable, access, and then d= isable page 0. But if that affects performance too badly, we may need to h= ave it leave page 0 enabled. I don't enable CSM on the platforms I work on= , so it's not something I have much to say about. Those who use CSM would = want to weigh in. Brian Johnson -----Original Message----- From: afish@apple.com [mailto:afish@apple.com]=20 Sent: Wednesday, August 30, 2017 11:27 AM To: Yao, Jiewen Cc: Johnson, Brian (EXL - Eagan) ; Wang, Jian J ; edk2-devel@lists.01.org Subject: Re: [edk2] [PATCH 0/2] Implement NULL pointer detection feature > On Aug 30, 2017, at 7:55 AM, Yao, Jiewen wrote: >=20 > Hi Brian > Good feedback. > Comment in line. >=20 > From: Johnson, Brian (EXL - Eagan) [mailto:brian.johnson@hpe.com] > Sent: Tuesday, August 29, 2017 11:17 PM > 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 catchin= g pointer-related problems. We have used a similar scheme on our systems f= or years, and caught several important bugs. Some comments: >=20 > * It's possible to implement similar protections in PEI (IA32) by modifyi= ng the GDT descriptors. > [Jiewen] Enabling PEI is interesting. > Since we are working on DXE and SMM as our first priority, can we treat t= he PEI enabling as a separate task? > If you can help to create a patch for PEI and submit, that will be even b= etter. >=20 Jiewen, This is slightly off topic but I've noticed in early PEI the issue on some = x86 chipsets is not that a NULL pointer reference is bad, but any reference= to an address space that is not decoded will triple bus fault the processo= r and that is really hard to debug. If the GDT trick could be used to cause= an exception that drops into the debugger vs. a triple bus fault that woul= d be a real win for debugging. While the memory ranges would be platform sp= ecific it would be awesome to have some plumbing in the edk2 to make this e= asier to implement.=20 Thanks, Andrew Fish >=20 >=20 > * For flexibility, I'd like NULL pointer protection to be controlled inde= pendently in PEI, DXE, and SMM, using separate PCDs. > [Jiewen] I think we can existing example on using one PCD to control all = phase or using one PCD to control different phase. > For example, we use below PCD to control all phases, PEI/DXE/SMM > ## The mask is used to control PerformanceLib behavior.

> # BIT0 - Enable Performance Measurement.
> # @Prompt Performance Measurement Property. > # @Expression 0x80000002 | (gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibr= aryPropertyMask & 0xFE) =3D=3D 0 > gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask|0|UINT8|0x000= 00009 >=20 > And we use below PCD to control different phase, DXE/SMM > ## The mask is used to control memory profile behavior.

> # BIT0 - Enable UEFI memory profile.
> # BIT1 - Enable SMRAM profile.
> # BIT7 - Disable recording at the start.
> # @Prompt Memory Profile Property. > # @Expression 0x80000002 | (gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryPro= filePropertyMask & 0x7C) =3D=3D 0 > gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfilePropertyMask|0x0|UINT8|0x= 30001041 >=20 > In order to make the code consistent with the existing PCD, I am thinking= to use one PCD named PcdNullPointerDetectionPropertyMask > # BIT0 - Enable NULL pointer detection for UEFI.
> # BIT1 - Enable NULL pointer detection for SMM.
> # BIT2 - Enable NULL pointer detection for PEI.
> # BIT7 - Disable NULL pointer detection after EndOfDxe.
>=20 > I am not so worried about pre-memory initialization PEI phase, because pa= ge 0 is always invalid. > For post-memory initialization PEI phase, we can modify the page table in= memory, and dynamic PCD can be used at that time. >=20 >=20 > * We have seen various option ROMs and OS boot loaders which have NULL po= inter 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 b= efore running these other executables. So I'd suggest adding another PCD, = perhaps PcdNullPointerDetectionPostDxe, to control NULL pointer protection = late in boot. If PcdNullPointerDetection !=3D PcdNullPointerDetectionPostD= xe, install an end-of-DXE event (gEfiEndOfDxeEventGroupGuid) which changes = the protection of page 0 using a call to EFI_CPU_ARCH_PROTOCOL. SetMemoryAt= tributes(CpuArch, 0, EFI_PAGE_SIZE, 0). > [Jiewen] Good point for the compatibility consideration. > Another thing is that CSM module may need access address 0. We have 2 cho= ices: > 1) CSM32 can call CpuArch->SetMemoryAttribute() to enable page 0, access = page 0, then disable page 0. > 2) CSM32 can call CpuArch->SetMemoryAttribute() to enable page 0 at the b= eginning, and always keep it enabled. > Any though on that? >=20 >=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 Ya= o, Jiewen > Sent: Sunday, August 27, 2017 10:39 PM > To: Wang, Jian J >; e= dk2-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 re= set its default value. This feature makes use of page mechanism to detect N= ULL pointer. So any ARCH doesn't support paging in EDK-II can't use it. Cur= rently validated platform includes VLV2 and Denlow. Let me know if all plat= form 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 OV= MF with TRUE. >=20 >=20 > 2) It's hard to make it a dynamic feature because we need to setup p= age table for physical address 0-4095 in advance. If there's no memory allo= c/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 cas= e. >=20 > [Jiewen] Dynamic means the initial value is dynamic. I am not saying we n= eed modify the page table. >=20 > An implementation could be: > A platform can set this PCD in PEI phase based upon a setup variable to c= hoose CSM enable or disable. >=20 > The IPL sets page table based upon this PCD value. The DXE Core cannot co= nsume PCD directly, because it might be dynamic. But we can pass the inform= ation 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@list= s.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 platform= s 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 dyn= amic, as such, a platform may set the validate based upon CSM enable/disabl= e? >=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 W= ang, >> 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 >>=20 >> This patch is the implementation of NULL pointer detection feature, >> which is one of the small features of Special Pool. >>=20 >> Wang, Jian J (2): >> Implement NULL pointer detection for EDK-II Core >> Implement NULL pointer detection for EDK-II SMM Core and driver >>=20 >> 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(-) >>=20 >> -- >> 2.11.0.windows.1 >>=20 >> _______________________________________________ >> 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