* [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