Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg: Move MigrateGdt from DiscoverMemory to TempRamDone. (CVE-2019-11098)
Guomin, Now since CpuMpPei module doesn't consume the PcdMigrateTemporaryRamFirmwareVolumes, can you remove the PCD reference from its INF? Thanks, Ray > -Original Message- > From: Guomin Jiang > Sent: Friday, January 29, 2021 2:45 PM > To: devel@edk2.groups.io > Cc: Dong, Eric ; Ni, Ray ; Laszlo > Ersek ; Kumar, Rahul1 > ; De, Debkumar ; Han, Harry > ; West, Catharine > > Subject: [PATCH 1/1] UefiCpuPkg: Move MigrateGdt from DiscoverMemory to > TempRamDone. (CVE-2019-11098) > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1614 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3160 > > The GDT still in flash with commit 60b12e69fb1c8c7180fdda92f008248b9ec83db1 > after TempRamDone > > So move the action to TempRamDone event to avoid reading GDT from flash. > > Signed-off-by: Guomin Jiang > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Cc: Rahul Kumar > Cc: Debkumar De > Cc: Harry Han > Cc: Catharine West > --- > UefiCpuPkg/SecCore/SecCore.inf | 1 + > UefiCpuPkg/CpuMpPei/CpuMpPei.c | 37 --- > UefiCpuPkg/CpuMpPei/CpuPaging.c | 8 -- > UefiCpuPkg/SecCore/SecMain.c| 45 + > 4 files changed, 46 insertions(+), 45 deletions(-) > > diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf > index 545781d6b4b3..ded83beb5272 100644 > --- a/UefiCpuPkg/SecCore/SecCore.inf > +++ b/UefiCpuPkg/SecCore/SecCore.inf > @@ -77,6 +77,7 @@ [Guids] > > [Pcd] >gUefiCpuPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes ## > CONSUMES > > [UserExtensions.TianoCore."ExtraFiles"] >SecCoreExtra.uni > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c > index d07540cf7471..07ccbe7c6a91 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c > @@ -429,43 +429,6 @@ GetGdtr ( >AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer); > } > > -/** > - 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 > -MigrateGdt ( > - VOID > - ) > -{ > - EFI_STATUS Status; > - UINTN GdtBufferSize; > - IA32_DESCRIPTOR Gdtr; > - VOID*GdtBuffer; > - > - AsmReadGdtr ((IA32_DESCRIPTOR *) ); > - GdtBufferSize = sizeof (IA32_SEGMENT_DESCRIPTOR) -1 + Gdtr.Limit + 1; > - > - Status = PeiServicesAllocatePool ( > - GdtBufferSize, > - > - ); > - ASSERT (GdtBuffer != NULL); > - if (EFI_ERROR (Status)) { > -return EFI_OUT_OF_RESOURCES; > - } > - > - GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_SEGMENT_DESCRIPTOR)); > - CopyMem (GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1); > - Gdtr.Base = (UINTN) GdtBuffer; > - AsmWriteGdtr (); > - > - return EFI_SUCCESS; > -} > - > /** >Initializes CPU exceptions handlers for the sake of stack switch > requirement. > > diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c > index 50ad4277af79..3e261d6657b3 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c > +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c > @@ -605,17 +605,9 @@ MemoryDiscoveredPpiNotifyCallback ( > { >EFI_STATUS Status; >BOOLEAN InitStackGuard; > - BOOLEAN InterruptState; >EDKII_MIGRATED_FV_INFO *MigratedFvInfo; >EFI_PEI_HOB_POINTERSHob; > > - if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) { > -InterruptState = SaveAndDisableInterrupts (); > -Status = MigrateGdt (); > -ASSERT_EFI_ERROR (Status); > -SetInterruptState (InterruptState); > - } > - >// >// Paging must be setup first. Otherwise the exception TSS setup during MP >// initialization later will not contain paging information and then fail > diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c > index 155be49a6011..2416c4ce56b2 100644 > --- a/UefiCpuPkg/SecCore/SecMain.c > +++ b/UefiCpuPkg/SecCore/SecMain.c > @@ -35,6 +35,43 @@ EFI_PEI_PPI_DESCRIPTOR > mPeiSecPlatformInformationPpi[] = { >} > }; > > +/** > + 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 > +MigrateGdt ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + UINTN GdtBufferSize; > + IA32_DESCRIPTOR Gdtr; > + VOID*GdtBuffer; > + > + AsmReadGdtr ((IA32_DESCRIPTOR *) ); > + GdtBufferSize = sizeof (IA32_SEGMENT_DESCRIPTOR) -1 + Gdtr.Limit + 1; > + > + Status = PeiServicesAllocatePool ( > + GdtBufferSize, > +
[edk2-devel] [PATCH 1/1] UefiCpuPkg: Move MigrateGdt from DiscoverMemory to TempRamDone. (CVE-2019-11098)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1614 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3160 The GDT still in flash with commit 60b12e69fb1c8c7180fdda92f008248b9ec83db1 after TempRamDone So move the action to TempRamDone event to avoid reading GDT from flash. Signed-off-by: Guomin Jiang Cc: Eric Dong Cc: Ray Ni Cc: Laszlo Ersek Cc: Rahul Kumar Cc: Debkumar De Cc: Harry Han Cc: Catharine West --- UefiCpuPkg/SecCore/SecCore.inf | 1 + UefiCpuPkg/CpuMpPei/CpuMpPei.c | 37 --- UefiCpuPkg/CpuMpPei/CpuPaging.c | 8 -- UefiCpuPkg/SecCore/SecMain.c| 45 + 4 files changed, 46 insertions(+), 45 deletions(-) diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf index 545781d6b4b3..ded83beb5272 100644 --- a/UefiCpuPkg/SecCore/SecCore.inf +++ b/UefiCpuPkg/SecCore/SecCore.inf @@ -77,6 +77,7 @@ [Guids] [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes ## CONSUMES [UserExtensions.TianoCore."ExtraFiles"] SecCoreExtra.uni diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c index d07540cf7471..07ccbe7c6a91 100644 --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c @@ -429,43 +429,6 @@ GetGdtr ( AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer); } -/** - 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 -MigrateGdt ( - VOID - ) -{ - EFI_STATUS Status; - UINTN GdtBufferSize; - IA32_DESCRIPTOR Gdtr; - VOID*GdtBuffer; - - AsmReadGdtr ((IA32_DESCRIPTOR *) ); - GdtBufferSize = sizeof (IA32_SEGMENT_DESCRIPTOR) -1 + Gdtr.Limit + 1; - - Status = PeiServicesAllocatePool ( - GdtBufferSize, - - ); - ASSERT (GdtBuffer != NULL); - if (EFI_ERROR (Status)) { -return EFI_OUT_OF_RESOURCES; - } - - GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_SEGMENT_DESCRIPTOR)); - CopyMem (GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1); - Gdtr.Base = (UINTN) GdtBuffer; - AsmWriteGdtr (); - - return EFI_SUCCESS; -} - /** Initializes CPU exceptions handlers for the sake of stack switch requirement. diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c index 50ad4277af79..3e261d6657b3 100644 --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c @@ -605,17 +605,9 @@ MemoryDiscoveredPpiNotifyCallback ( { EFI_STATUS Status; BOOLEAN InitStackGuard; - BOOLEAN InterruptState; EDKII_MIGRATED_FV_INFO *MigratedFvInfo; EFI_PEI_HOB_POINTERSHob; - if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) { -InterruptState = SaveAndDisableInterrupts (); -Status = MigrateGdt (); -ASSERT_EFI_ERROR (Status); -SetInterruptState (InterruptState); - } - // // Paging must be setup first. Otherwise the exception TSS setup during MP // initialization later will not contain paging information and then fail diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c index 155be49a6011..2416c4ce56b2 100644 --- a/UefiCpuPkg/SecCore/SecMain.c +++ b/UefiCpuPkg/SecCore/SecMain.c @@ -35,6 +35,43 @@ EFI_PEI_PPI_DESCRIPTOR mPeiSecPlatformInformationPpi[] = { } }; +/** + 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 +MigrateGdt ( + VOID + ) +{ + EFI_STATUS Status; + UINTN GdtBufferSize; + IA32_DESCRIPTOR Gdtr; + VOID*GdtBuffer; + + AsmReadGdtr ((IA32_DESCRIPTOR *) ); + GdtBufferSize = sizeof (IA32_SEGMENT_DESCRIPTOR) -1 + Gdtr.Limit + 1; + + Status = PeiServicesAllocatePool ( + GdtBufferSize, + + ); + ASSERT (GdtBuffer != NULL); + if (EFI_ERROR (Status)) { +return EFI_OUT_OF_RESOURCES; + } + + GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_SEGMENT_DESCRIPTOR)); + CopyMem (GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1); + Gdtr.Base = (UINTN) GdtBuffer; + AsmWriteGdtr (); + + return EFI_SUCCESS; +} + // // These are IDT entries pointing to 10:FFE4h. // @@ -409,6 +446,14 @@ SecTemporaryRamDone ( // State = SaveAndDisableInterrupts (); + // + // Migrate GDT before NEM near down + // + if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) { +Status = MigrateGdt (); +ASSERT_EFI_ERROR (Status); + } + // // Disable Temporary RAM after Stack and Heap have been migrated at this point. // --