From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web12.1723.1593675375991985222 for ; Thu, 02 Jul 2020 00:36:16 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: ray.ni@intel.com) IronPort-SDR: dHFNplZd1cCoLmaIFMP0YSRCdMYV6HaJiOMDGswCXhDnYoSZy0t04bgOSr4v1uXwhcIupxWn3G 5eGRshcLIdWQ== X-IronPort-AV: E=McAfee;i="6000,8403,9669"; a="148363692" X-IronPort-AV: E=Sophos;i="5.75,303,1589266800"; d="scan'208";a="148363692" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Jul 2020 00:36:14 -0700 IronPort-SDR: U05OS7fOrz/UgjsClpoxCqbeelcorcF0Q37I4v4qKxn0fWTLtDoftrLD2vgJyR7EP/R6hYYL2y CjWa08hlUwZQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,303,1589266800"; d="scan'208";a="322010196" Received: from fmsmsx605.amr.corp.intel.com ([10.18.126.85]) by orsmga007.jf.intel.com with ESMTP; 02 Jul 2020 00:36:14 -0700 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 2 Jul 2020 00:36:14 -0700 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Thu, 2 Jul 2020 00:36:14 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 2 Jul 2020 00:36:13 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.161]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.85]) with mapi id 14.03.0439.000; Thu, 2 Jul 2020 15:36:10 +0800 From: "Ni, Ray" To: "devel@edk2.groups.io" , "Jiang, Guomin" CC: Michael Kubacki , "Dong, Eric" , Laszlo Ersek , "Kumar, Rahul1" Subject: Re: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098) Thread-Topic: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098) Thread-Index: AQHWUC/Y5Ascvo2NCka0nkXtWfWatajz5ldg Date: Thu, 2 Jul 2020 07:36:09 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C5EA3FD@SHSMSX104.ccr.corp.intel.com> References: <20200702051525.1102-1-guomin.jiang@intel.com> <20200702051525.1102-3-guomin.jiang@intel.com> In-Reply-To: <20200702051525.1102-3-guomin.jiang@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Guomin, Can you please separate the coding style change and the functionality chan= ge in different patches? > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Guomin Ji= ang > Sent: Thursday, July 2, 2020 1:15 PM > To: devel@edk2.groups.io > Cc: Michael Kubacki ; Dong, Eric ; Ni, Ray ; Laszlo > Ersek ; Kumar, Rahul1 > Subject: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and ID= T migration support (CVE-2019-11098) >=20 > From: Michael Kubacki >=20 > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D1614 >=20 > Moves the GDT and IDT to permanent memory in a memory discovered > callback. This is done to ensure the GDT and IDT authenticated in > pre-memory is not fetched from outside a verified location after > the permanent memory transition. >=20 > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > Signed-off-by: Michael Kubacki > --- > UefiCpuPkg/CpuMpPei/CpuMpPei.c | 40 ++++++++++++++++++- > UefiCpuPkg/CpuMpPei/CpuMpPei.h | 13 ++++++ > UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++-- > .../Ia32/ArchExceptionHandler.c | 4 +- > .../SecPeiCpuException.c | 2 +- > 5 files changed, 65 insertions(+), 8 deletions(-) >=20 > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpP= ei.c > index 07ccbe7c6a91..2d6f1bc98851 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c > @@ -429,6 +429,44 @@ GetGdtr ( > AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer); > } >=20 > +/** > + Migrates the Global Descriptor Table (GDT) to permanent memory. > + > + @retval EFI_SUCCESS The GDT was migrated successfully. > + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to = lack of available memory. > + > +**/ > +EFI_STATUS > +EFIAPI > +MigrateGdt ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + UINTN GdtBufferSize; > + IA32_DESCRIPTOR Gdtr; > + UINT8 *GdtBuffer; > + > + AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr); > + GdtBufferSize =3D sizeof (IA32_TSS_DESCRIPTOR) + Gdtr.Limit + 1; > + > + Status =3D PeiServicesAllocatePool ( > + GdtBufferSize, > + (VOID **) &GdtBuffer > + ); > + ASSERT (GdtBuffer !=3D NULL); > + if (EFI_ERROR (Status)) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + GdtBuffer =3D ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR))= ; > + CopyMem ((VOID *) (UINTN) GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit += 1); > + Gdtr.Base =3D (UINT32)(UINTN) GdtBuffer; > + AsmWriteGdtr (&Gdtr); > + > + return EFI_SUCCESS; > +} > + > /** > Initializes CPU exceptions handlers for the sake of stack switch requ= irement. >=20 > @@ -644,7 +682,7 @@ InitializeCpuMpWorker ( > &gEfiVectorHandoffInfoPpiGuid, > 0, > NULL, > - (VOID **)&VectorHandoffInfoPpi > + (VOID **) &VectorHandoffInfoPpi > ); > if (Status =3D=3D EFI_SUCCESS) { > VectorInfo =3D VectorHandoffInfoPpi->Info; > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpP= ei.h > index 7d5c527d6006..5dc956409594 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > @@ -397,6 +397,19 @@ SecPlatformInformation2 ( > OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecor= d2 > ); >=20 > +/** > + Migrates the Global Descriptor Table (GDT) to permanent memory. > + > + @retval EFI_SUCCESS The GDT was migrated successfully. > + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to = lack of available memory. > + > +**/ > +EFI_STATUS > +EFIAPI > +MigrateGdt ( > + VOID > + ); > + > /** > Initializes MP and exceptions handlers. >=20 > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPa= ging.c > index a462e7ee1e38..d0cbebf70bbf 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c > @@ -152,7 +152,7 @@ GetPhysicalAddressWidth ( > Get the type of top level page table. >=20 > @retval Page512G PML4 paging. > - @retval Page1G PAE paing. > + @retval Page1G PAE paging. >=20 > **/ > PAGE_ATTRIBUTE > @@ -582,7 +582,7 @@ SetupStackGuardPage ( > } >=20 > /** > - Enabl/setup stack guard for each processor if PcdCpuStackGuard is set= to TRUE. > + Enable/setup stack guard for each processor if PcdCpuStackGuard is se= t to TRUE. >=20 > Doing this in the memory-discovered callback is to make sure the Stac= k Guard > feature to cover as most PEI code as possible. > @@ -602,8 +602,14 @@ MemoryDiscoveredPpiNotifyCallback ( > IN VOID *Ppi > ) > { > - EFI_STATUS Status; > - BOOLEAN InitStackGuard; > + EFI_STATUS Status; > + BOOLEAN InitStackGuard; > + BOOLEAN InterruptState; > + > + InterruptState =3D SaveAndDisableInterrupts (); > + Status =3D MigrateGdt (); > + ASSERT_EFI_ERROR (Status); > + SetInterruptState (InterruptState); >=20 > // > // Paging must be setup first. Otherwise the exception TSS setup duri= ng MP > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptio= nHandler.c > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c > index 1aafb7dac139..903449e0daa9 100644 > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandle= r.c > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandle= r.c > @@ -18,8 +18,8 @@ > **/ > VOID > ArchUpdateIdtEntry ( > - IN IA32_IDT_GATE_DESCRIPTOR *IdtEntry, > - IN UINTN InterruptHandler > + OUT IA32_IDT_GATE_DESCRIPTOR *IdtEntry, > + IN UINTN InterruptHandler > ) > { > IdtEntry->Bits.OffsetLow =3D (UINT16)(UINTN)InterruptHandler; > diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptio= n.c > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c > index 20148db74cf8..d4ae153c5742 100644 > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c > @@ -87,7 +87,7 @@ InitializeCpuExceptionHandlers ( > IdtEntryCount =3D (IdtDescriptor.Limit + 1) / sizeof (IA32_IDT_GATE_D= ESCRIPTOR); > if (IdtEntryCount > CPU_EXCEPTION_NUM) { > // > - // CPU exeption library only setup CPU_EXCEPTION_NUM exception hand= ler at most > + // CPU exception library only setup CPU_EXCEPTION_NUM exception han= dler at most > // > IdtEntryCount =3D CPU_EXCEPTION_NUM; > } > -- > 2.25.1.windows.1 >=20 >=20 >=20