[edk2-devel] [PATCH v5 7/9] UefiCpuPkg/CpuMpPei: Enable paging and set NP flag to avoid TOCTOU (CVE-2019-11098)

[Wang, Jian J]

Guomin,


> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Guomin
> Jiang
> Sent: Thursday, July 09, 2020 9:57 AM
> To: devel at edk2.groups.io
> Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Laszlo Ersek
> <lersek at redhat.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>
> Subject: [edk2-devel] [PATCH v5 7/9] UefiCpuPkg/CpuMpPei: Enable paging and
> set NP flag to avoid TOCTOU (CVE-2019-11098)
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
> 
> To avoid the TOCTOU, enable paging and set Not Present flag so when
> access any code in the flash range, it will trigger #NP exception.

It's #PF, not #NP.

> 
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Rahul Kumar <rahul1.kumar at intel.com>
> Signed-off-by: Guomin Jiang <guomin.jiang at intel.com>
> Acked-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  UefiCpuPkg/CpuMpPei/CpuMpPei.inf |  3 +++
>  UefiCpuPkg/CpuMpPei/CpuPaging.c  | 26 ++++++++++++++++++++++++--
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> index f4d11b861f77..7e511325d8b8 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> @@ -46,6 +46,9 @@ [LibraryClasses]
>    BaseMemoryLib
>    CpuLib
> 
> +[Guids]
> +  gEdkiiMigratedFvInfoGuid                                             ##
> SOMETIMES_CONSUMES     ## HOB
> +
>  [Ppis]
>    gEfiPeiMpServicesPpiGuid                      ## PRODUCES
>    gEfiSecPlatformInformationPpiGuid             ## SOMETIMES_CONSUMES
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index 3bf0574b34c6..04a16fb2b620 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/CpuLib.h>
>  #include <Library/BaseLib.h>
> +#include <Guid/MigratedFvInfo.h>
> 
>  #include "CpuMpPei.h"
> 
> @@ -605,6 +606,8 @@ MemoryDiscoveredPpiNotifyCallback (
>    EFI_STATUS  Status;
>    BOOLEAN     InitStackGuard;
>    BOOLEAN     InterruptState;
> +  EDKII_MIGRATED_FV_INFO *MigratedFvInfo;
> +  EFI_PEI_HOB_POINTERS   Hob;
> 

Please align the variable names.

>    if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) {
>      InterruptState = SaveAndDisableInterrupts ();
> @@ -619,9 +622,14 @@ MemoryDiscoveredPpiNotifyCallback (
>    // the task switch (for the sake of stack switch).
>    //
>    InitStackGuard = FALSE;
> -  if (IsIa32PaeSupported () && PcdGetBool (PcdCpuStackGuard)) {
> +  Hob.Raw = NULL;
> +  if (IsIa32PaeSupported ()) {
> +    Hob.Raw  = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
> +    InitStackGuard = PcdGetBool (PcdCpuStackGuard);
> +  }
> +

PcdMigrateTemporaryRamFirmwareVolumes is only effective along
with PcdShadowPeimOnBoot or PcdShadowPeimOnS3Boot in PeiCore.

Using it here without other two doesn't make sense. Need further
discussion to clarify the usage of all of them.

> +  if (InitStackGuard || Hob.Raw != NULL) {
>      EnablePaging ();
> -    InitStackGuard = TRUE;
>    }
> 
>    Status = InitializeCpuMpWorker ((CONST EFI_PEI_SERVICES **)PeiServices);
> @@ -631,6 +639,20 @@ MemoryDiscoveredPpiNotifyCallback (
>      SetupStackGuardPage ();
>    }
> 
> +  while (Hob.Raw != NULL) {
> +    MigratedFvInfo = GET_GUID_HOB_DATA (Hob);
> +
> +    //
> +    // Enable #NP exception, so if the code access after disable NEM, it will
> generate

It's #PF, not #NP.

> +    // to avoid potential vulnerability.
> +    //
> +    ConvertMemoryPageAttributes (MigratedFvInfo->FvOrgBase,
> MigratedFvInfo->FvLength, 0);
> +
> +    Hob.Raw = GET_NEXT_HOB (Hob);
> +    Hob.Raw = GetNextGuidHob (&gEdkiiMigratedFvInfoGuid, Hob.Raw);
> +  }
> +  CpuFlushTlb ();
> +
>    return Status;
>  }
> 
> --
> 2.25.1.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62401): https://edk2.groups.io/g/devel/message/62401
Mute This Topic: https://groups.io/mt/75390181/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-