Re: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add microcode header verification.

["Siyuan, Fu" <siyuan.fu@intel.com>]

Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>

> -----Original Message-----
> From: Li, Aaron <aaron.li@intel.com>
> Sent: 2021年2月3日 15:06
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add microcode
> header verification.
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3196
> 
> Microcode header should be checked before calling
> IsMicrocodePatchNeedLoad(). This is to make sure garbage value after
> remove microcode from FV would not cause stack overflow in
> IsMicrocodePatchNeedLoad().
> 
> Signed-off-by: Aaron Li <aaron.li@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> ---
> 
> Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.c
> | 30 +++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git
> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.c
> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.c
> index 1494397a8e36..98a7aed69757 100644
> ---
> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.c
> +++
> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePe
> i.c
> @@ -402,6 +402,7 @@ ShadowMicrocode (
>    UINTN                             MaxPatchNumber;
> 
>    CPU_MICROCODE_HEADER              *MicrocodeEntryPoint;
> 
>    UINTN                             PatchCount;
> 
> +  UINTN                             DataSize;
> 
>    UINTN                             TotalSize;
> 
>    UINTN                             TotalLoadSize;
> 
> 
> 
> @@ -446,7 +447,34 @@ ShadowMicrocode (
>    for (Index = 0; Index < EntryNum; Index++) {
> 
>      if (FitEntry[Index].Type == FIT_TYPE_01_MICROCODE) {
> 
>        MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> FitEntry[Index].Address;
> 
> -      TotalSize = (MicrocodeEntryPoint->DataSize == 0) ? 2048 :
> MicrocodeEntryPoint->TotalSize;
> 
> +
> 
> +      if (*(UINT32 *) MicrocodeEntryPoint == 0xFFFFFFFF) {
> 
> +        //
> 
> +        // An empty slot for reserved microcode update, skip to check next entry.
> 
> +        //
> 
> +        continue;
> 
> +      }
> 
> +
> 
> +      if (MicrocodeEntryPoint->HeaderVersion != 0x1) {
> 
> +        //
> 
> +        // Not a valid microcode header, skip to check next entry.
> 
> +        //
> 
> +        continue;
> 
> +      }
> 
> +
> 
> +      DataSize  = MicrocodeEntryPoint->DataSize;
> 
> +      TotalSize = (DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;
> 
> +      if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||
> 
> +           (DataSize & 0x3) != 0 ||
> 
> +           (TotalSize & (SIZE_1KB - 1)) != 0 ||
> 
> +           TotalSize < DataSize
> 
> +        ) {
> 
> +        //
> 
> +        // Not a valid microcode header, skip to check next entry.
> 
> +        //
> 
> +        continue;
> 
> +      }
> 
> +
> 
>        if (IsMicrocodePatchNeedLoad (CpuIdCount, MicrocodeCpuId,
> MicrocodeEntryPoint)) {
> 
>          PatchInfoBuffer[PatchCount].Address     = (UINTN) MicrocodeEntryPoint;
> 
>          PatchInfoBuffer[PatchCount].Size        = TotalSize;
> 
> --
> 2.29.2.windows.2