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

["Ni, Ray" <ray.ni@intel.com>]

Pushed.

> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, February 4, 2021 1:44 PM
> To: Fu, Siyuan <siyuan.fu@intel.com>; Li, Aaron <aaron.li@intel.com>;
> devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
> Subject: RE: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add
> microcode header verification.
> 
> Reviewed-by: Ray Ni <ray.ni@intel.com>
> 
> > -----Original Message-----
> > From: Fu, Siyuan <siyuan.fu@intel.com>
> > Sent: Thursday, February 4, 2021 1:40 PM
> > To: Li, Aaron <aaron.li@intel.com>; devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@intel.com>
> > Subject: RE: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add
> > microcode header verification.
> >
> > 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/ShadowMicrocodeP
> > ei.c
> > > | 30 +++++++++++++++++++-
> > >  1 file changed, 29 insertions(+), 1 deletion(-)
> > >
> > > diff --git
> > >
> >
> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > ePe
> > > i.c
> > >
> >
> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > ePe
> > > i.c
> > > index 1494397a8e36..98a7aed69757 100644
> > > ---
> > >
> >
> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > ePe
> > > i.c
> > > +++
> > >
> >
> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocod
> > ePe
> > > 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