From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.120; helo=mga04.intel.com; envelope-from=chen.a.chen@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D3C26211CD63E for ; Thu, 28 Feb 2019 00:55:10 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Feb 2019 00:55:10 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,422,1544515200"; d="scan'208";a="324113058" Received: from chenche4.ccr.corp.intel.com ([10.239.9.12]) by fmsmga005.fm.intel.com with ESMTP; 28 Feb 2019 00:55:09 -0800 From: Chen A Chen To: edk2-devel@lists.01.org Cc: Chen A Chen , Ray Ni , Eric Dong Date: Thu, 28 Feb 2019 16:55:07 +0800 Message-Id: <20190228085507.19836-1-chen.a.chen@intel.com> X-Mailer: git-send-email 2.16.2.windows.1 Subject: [PATCH v2] UefiCpuPkg/Microcode: Add verification logic before parsing payload X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 08:55:11 -0000 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 Cc: Ray Ni Cc: Eric Dong --- 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