* [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add microcode header verification. @ 2021-02-03 7:05 Aaron Li 2021-02-04 5:40 ` Siyuan, Fu 0 siblings, 1 reply; 4+ messages in thread From: Aaron Li @ 2021-02-03 7:05 UTC (permalink / raw) To: devel; +Cc: Ray Ni, Rangasai V Chaganty, Siyuan Fu 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/ShadowMicrocodePei.c b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.c index 1494397a8e36..98a7aed69757 100644 --- a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.c +++ b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add microcode header verification. 2021-02-03 7:05 [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add microcode header verification Aaron Li @ 2021-02-04 5:40 ` Siyuan, Fu 2021-02-04 5:44 ` Ni, Ray 0 siblings, 1 reply; 4+ messages in thread From: Siyuan, Fu @ 2021-02-04 5:40 UTC (permalink / raw) To: Li, Aaron, devel@edk2.groups.io; +Cc: Ni, Ray, Chaganty, Rangasai V 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add microcode header verification. 2021-02-04 5:40 ` Siyuan, Fu @ 2021-02-04 5:44 ` Ni, Ray 2021-02-05 4:51 ` Ni, Ray 0 siblings, 1 reply; 4+ messages in thread From: Ni, Ray @ 2021-02-04 5:44 UTC (permalink / raw) To: Fu, Siyuan, Li, Aaron, devel@edk2.groups.io; +Cc: Chaganty, Rangasai V 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add microcode header verification. 2021-02-04 5:44 ` Ni, Ray @ 2021-02-05 4:51 ` Ni, Ray 0 siblings, 0 replies; 4+ messages in thread From: Ni, Ray @ 2021-02-05 4:51 UTC (permalink / raw) To: Fu, Siyuan, Li, Aaron, devel@edk2.groups.io; +Cc: Chaganty, Rangasai V 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-05 4:52 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-02-03 7:05 [PATCH v1 1/1] IntelSiliconPkg/ShadowMicrocodePei: Add microcode header verification Aaron Li 2021-02-04 5:40 ` Siyuan, Fu 2021-02-04 5:44 ` Ni, Ray 2021-02-05 4:51 ` Ni, Ray
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox