* [PATCH v2] UefiCpuPkg/Microcode: Add verification logic before parsing payload
@ 2019-02-28 8:55 Chen A Chen
0 siblings, 0 replies; only message in thread
From: Chen A Chen @ 2019-02-28 8:55 UTC (permalink / raw)
To: edk2-devel; +Cc: Chen A Chen, Ray Ni, Eric Dong
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1020
The function MicrocodeDetect may receive untrusted input. Add verification
logic to check Microcode Payload Header.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
---
UefiCpuPkg/Library/MpInitLib/Microcode.c | 187 ++++++++++++++++++++-----------
1 file changed, 124 insertions(+), 63 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 5f9ae22794..c06e51f5e6 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -32,6 +32,66 @@ GetCurrentMicrocodeSignature (
return BiosSignIdMsr.Bits.MicrocodeUpdateSignature;
}
+/**
+ Verify whether Microcode payload is valid.
+
+ @param[in] MicrocodeHeader Point to the Microcode payload buffer.
+
+ @retval TRUE Valid payload
+ @retval FALSE Invalid payload
+**/
+BOOLEAN
+IsMicrocodeHeaderValid (
+ IN CPU_MICROCODE_HEADER *MicrocodeHeader
+ )
+{
+ UINT32 DataSize;
+
+ ASSERT (MicrocodeHeader != NULL);
+
+ ///
+ /// Verify Version
+ ///
+ if (MicrocodeHeader->HeaderVersion != 0x1) {
+ return FALSE;
+ }
+ if (MicrocodeHeader->LoaderRevision != 0x1) {
+ return FALSE;
+ }
+
+ ///
+ /// Check whether DataSize is aligned with 4 bytes.
+ ///
+ if (MicrocodeHeader->DataSize != 0 && (MicrocodeHeader->DataSize & 0x3) != 0) {
+ return FALSE;
+ }
+
+ ///
+ /// Check whether TotalSize is aligned with 1KB.
+ ///
+ if ((MicrocodeHeader->TotalSize & (SIZE_1KB - 1)) != 0) {
+ return FALSE;
+ }
+
+ ///
+ /// Verify DataSize and TotalSize.
+ ///
+ if (MicrocodeHeader->DataSize == 0) {
+ DataSize = 2000;
+ } else {
+ DataSize = MicrocodeHeader->DataSize;
+ }
+
+ if ( (MicrocodeHeader->TotalSize <= sizeof (CPU_MICROCODE_HEADER)) ||
+ (MicrocodeHeader->TotalSize <= DataSize) ||
+ (MicrocodeHeader->TotalSize != sizeof (CPU_MICROCODE_HEADER) + DataSize)
+ ) {
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
/**
Detect whether specified processor can find matching microcode patch and load it.
@@ -166,6 +226,18 @@ MicrocodeDetect (
//
CorrectMicrocode = FALSE;
+ if (!IsMicrocodeHeaderValid (MicrocodeEntryPoint)) {
+ //
+ // It is the padding data between the microcode patches for microcode patches alignment.
+ // Because the microcode patch is the multiple of 1-KByte, the padding data should not
+ // exist if the microcode patch alignment value is not larger than 1-KByte. So, the microcode
+ // alignment value should be larger than 1-KByte. We could skip SIZE_1KB padding data to
+ // find the next possible microcode patch header.
+ //
+ MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);
+ continue;
+ }
+
//
// Save an in-complete CheckSum32 from CheckSum Part1 for common parts.
//
@@ -184,90 +256,79 @@ MicrocodeDetect (
InCompleteCheckSum32 -= MicrocodeEntryPoint->ProcessorFlags;
InCompleteCheckSum32 -= MicrocodeEntryPoint->Checksum;
- if (MicrocodeEntryPoint->HeaderVersion == 0x1) {
+ //
+ // It is the microcode header. It is not the padding data between microcode patches
+ // because the padding data should not include 0x00000001 and it should be the repeated
+ // byte format (like 0xXYXYXYXY....).
+ //
+ if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&
+ MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
+ (MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
+ ) {
//
- // It is the microcode header. It is not the padding data between microcode patches
- // because the padding data should not include 0x00000001 and it should be the repeated
- // byte format (like 0xXYXYXYXY....).
+ // Calculate CheckSum Part1.
//
- if (MicrocodeEntryPoint->ProcessorSignature.Uint32 == Eax.Uint32 &&
- MicrocodeEntryPoint->UpdateRevision > LatestRevision &&
- (MicrocodeEntryPoint->ProcessorFlags & (1 << PlatformId))
- ) {
+ CheckSum32 = InCompleteCheckSum32;
+ CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;
+ CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;
+ CheckSum32 += MicrocodeEntryPoint->Checksum;
+ if (CheckSum32 == 0) {
+ CorrectMicrocode = TRUE;
+ ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
+ }
+ } else if ((MicrocodeEntryPoint->DataSize != 0) &&
+ (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
+ ExtendedTableLength = MicrocodeEntryPoint->TotalSize - (MicrocodeEntryPoint->DataSize +
+ sizeof (CPU_MICROCODE_HEADER));
+ if (ExtendedTableLength != 0) {
//
- // Calculate CheckSum Part1.
+ // Extended Table exist, check if the CPU in support list
//
- CheckSum32 = InCompleteCheckSum32;
- CheckSum32 += MicrocodeEntryPoint->ProcessorSignature.Uint32;
- CheckSum32 += MicrocodeEntryPoint->ProcessorFlags;
- CheckSum32 += MicrocodeEntryPoint->Checksum;
- if (CheckSum32 == 0) {
- CorrectMicrocode = TRUE;
- ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
- }
- } else if ((MicrocodeEntryPoint->DataSize != 0) &&
- (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
- ExtendedTableLength = MicrocodeEntryPoint->TotalSize - (MicrocodeEntryPoint->DataSize +
- sizeof (CPU_MICROCODE_HEADER));
- if (ExtendedTableLength != 0) {
- //
- // Extended Table exist, check if the CPU in support list
- //
- ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *) ((UINT8 *) (MicrocodeEntryPoint)
- + MicrocodeEntryPoint->DataSize + sizeof (CPU_MICROCODE_HEADER));
+ ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *) ((UINT8 *) (MicrocodeEntryPoint)
+ + MicrocodeEntryPoint->DataSize + sizeof (CPU_MICROCODE_HEADER));
+ //
+ // Calculate Extended Checksum
+ //
+ if ((ExtendedTableLength % 4) == 0) {
//
- // Calculate Extended Checksum
+ // Calculate CheckSum Part2.
//
- if ((ExtendedTableLength % 4) == 0) {
+ CheckSum32 = CalculateSum32 ((UINT32 *) ExtendedTableHeader, ExtendedTableLength);
+ if (CheckSum32 == 0) {
//
- // Calculate CheckSum Part2.
+ // Checksum correct
//
- CheckSum32 = CalculateSum32 ((UINT32 *) ExtendedTableHeader, ExtendedTableLength);
- if (CheckSum32 == 0) {
+ ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
+ ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *) (ExtendedTableHeader + 1);
+ for (Index = 0; Index < ExtendedTableCount; Index ++) {
//
- // Checksum correct
+ // Calculate CheckSum Part3.
//
- ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
- ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *) (ExtendedTableHeader + 1);
- for (Index = 0; Index < ExtendedTableCount; Index ++) {
+ CheckSum32 = InCompleteCheckSum32;
+ CheckSum32 += ExtendedTable->ProcessorSignature.Uint32;
+ CheckSum32 += ExtendedTable->ProcessorFlag;
+ CheckSum32 += ExtendedTable->Checksum;
+ if (CheckSum32 == 0) {
//
- // Calculate CheckSum Part3.
+ // Verify Header
//
- CheckSum32 = InCompleteCheckSum32;
- CheckSum32 += ExtendedTable->ProcessorSignature.Uint32;
- CheckSum32 += ExtendedTable->ProcessorFlag;
- CheckSum32 += ExtendedTable->Checksum;
- if (CheckSum32 == 0) {
+ if ((ExtendedTable->ProcessorSignature.Uint32 == Eax.Uint32) &&
+ (ExtendedTable->ProcessorFlag & (1 << PlatformId)) ) {
//
- // Verify Header
+ // Find one
//
- if ((ExtendedTable->ProcessorSignature.Uint32 == Eax.Uint32) &&
- (ExtendedTable->ProcessorFlag & (1 << PlatformId)) ) {
- //
- // Find one
- //
- CorrectMicrocode = TRUE;
- ProcessorFlags = ExtendedTable->ProcessorFlag;
- break;
- }
+ CorrectMicrocode = TRUE;
+ ProcessorFlags = ExtendedTable->ProcessorFlag;
+ break;
}
- ExtendedTable ++;
}
+ ExtendedTable ++;
}
}
}
}
- } else {
- //
- // It is the padding data between the microcode patches for microcode patches alignment.
- // Because the microcode patch is the multiple of 1-KByte, the padding data should not
- // exist if the microcode patch alignment value is not larger than 1-KByte. So, the microcode
- // alignment value should be larger than 1-KByte. We could skip SIZE_1KB padding data to
- // find the next possible microcode patch header.
- //
- MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);
- continue;
}
+
//
// Get the next patch.
//
--
2.16.2.windows.1
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2019-02-28 8:55 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-28 8:55 [PATCH v2] UefiCpuPkg/Microcode: Add verification logic before parsing payload Chen A Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox