From b8316529d841907bb624d71f70ee42f3b6635f4a Mon Sep 17 00:00:00 2001 From: miaoyubo Date: Tue, 31 Aug 2021 09:52:08 +0800 Subject: [PATCH 1/3] Fix CVE-2021-28211 --- ...aCustomDecompressLib-catch-4GB-uncom.patch | 93 +++++++++++++++++++ edk2.spec | 6 +- 2 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 0017-MdeModulePkg-LzmaCustomDecompressLib-catch-4GB-uncom.patch diff --git a/0017-MdeModulePkg-LzmaCustomDecompressLib-catch-4GB-uncom.patch b/0017-MdeModulePkg-LzmaCustomDecompressLib-catch-4GB-uncom.patch new file mode 100644 index 0000000..4888c63 --- /dev/null +++ b/0017-MdeModulePkg-LzmaCustomDecompressLib-catch-4GB-uncom.patch @@ -0,0 +1,93 @@ +From e7bd0dd26db7e56aa8ca70132d6ea916ee6f3db0 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Thu, 19 Nov 2020 12:50:34 +0100 +Subject: [PATCH] MdeModulePkg/LzmaCustomDecompressLib: catch 4GB+ uncompressed + buffer sizes +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The LzmaUefiDecompressGetInfo() function +[MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompress.c] currently +silently truncates the UINT64 "DecodedSize" property of the compressed +blob to the UINT32 "DestinationSize" output parameter. + +If "DecodedSize" is 0x1_0000_0100, for example, then the subsequent memory +allocation (for decompression) will likely succeed (allocating 0x100 bytes +only), but then the LzmaUefiDecompress() function (which re-fetches the +uncompressed buffer size from the same LZMA header into a "SizeT" +variable) will overwrite the buffer. + +Catch (DecodedSize > MAX_UINT32) in LzmaUefiDecompressGetInfo() at once. +This should not be a practical limitation. (The issue cannot be fixed for +32-bit systems without spec modifications anyway, given that the +"OutputSize" output parameter of +EFI_GUIDED_SECTION_EXTRACTION_PROTOCOL.ExtractSection() has type UINTN, +not UINT64.) + +Cc: Dandan Bi +Cc: Hao A Wu +Cc: Jian J Wang +Cc: Liming Gao +Cc: Philippe Mathieu-Daudé +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1816 +Signed-off-by: Laszlo Ersek +Reviewed-by: Liming Gao +Reviewed-by: Philippe Mathieu-Daudé +Message-Id: <20201119115034.12897-2-lersek@redhat.com> +--- + .../Library/LzmaCustomDecompressLib/LzmaDecompress.c | 7 +++++++ + .../LzmaCustomDecompressLib/LzmaDecompressLibInternal.h | 5 +++++ + 2 files changed, 12 insertions(+) + +diff --git a/MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompress.c b/MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompress.c +index c58912eb6a..8f7c242dca 100644 +--- a/MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompress.c ++++ b/MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompress.c +@@ -127,6 +127,10 @@ GetDecodedSizeOfBuf( + in DestinationSize and the size of the scratch + buffer was returned in ScratchSize. + ++ @retval RETURN_UNSUPPORTED DestinationSize cannot be output because the ++ uncompressed buffer size (in bytes) does not fit ++ in a UINT32. Output parameters have not been ++ modified. + **/ + RETURN_STATUS + EFIAPI +@@ -142,6 +146,9 @@ LzmaUefiDecompressGetInfo ( + ASSERT(SourceSize >= LZMA_HEADER_SIZE); + + DecodedSize = GetDecodedSizeOfBuf((UINT8*)Source); ++ if (DecodedSize > MAX_UINT32) { ++ return RETURN_UNSUPPORTED; ++ } + + *DestinationSize = (UINT32)DecodedSize; + *ScratchSize = SCRATCH_BUFFER_REQUEST_SIZE; +diff --git a/MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompressLibInternal.h b/MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompressLibInternal.h +index 26f110ba2a..fbafd5f100 100644 +--- a/MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompressLibInternal.h ++++ b/MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaDecompressLibInternal.h +@@ -9,6 +9,7 @@ + #ifndef __LZMADECOMPRESSLIB_INTERNAL_H__ + #define __LZMADECOMPRESSLIB_INTERNAL_H__ + ++#include + #include + #include + #include +@@ -45,6 +46,10 @@ + in DestinationSize and the size of the scratch + buffer was returned in ScratchSize. + ++ @retval RETURN_UNSUPPORTED DestinationSize cannot be output because the ++ uncompressed buffer size (in bytes) does not fit ++ in a UINT32. Output parameters have not been ++ modified. + **/ + RETURN_STATUS + EFIAPI +-- +2.27.0 + diff --git a/edk2.spec b/edk2.spec index ee3248b..23cc167 100644 --- a/edk2.spec +++ b/edk2.spec @@ -5,7 +5,7 @@ Name: edk2 Version: %{stable_date} -Release: 7 +Release: 8 Summary: EFI Development Kit II License: BSD-2-Clause-Patent URL: https://github.com/tianocore/edk2 @@ -28,6 +28,7 @@ Patch0013: 0013-ArmVirtPkg-ArmVirtQemu-enable-TPM2-based-measured-bo.patch Patch0014: 0014-MdeModulePkg-Core-Dxe-assert-SectionInstance-invariant-in-FindChildNode.patch Patch0015: 0015-MdeModulePkg-Core-Dxe-limit-FwVol-encapsulation-section-recursion.patch Patch0016: 0016-ArmPkg-CompilerIntrinsicsLib-provide-atomics-intrins.patch +Patch0017: 0017-MdeModulePkg-LzmaCustomDecompressLib-catch-4GB-uncom.patch BuildRequires: acpica-tools gcc gcc-c++ libuuid-devel python3 bc nasm python3-unversioned-command @@ -225,6 +226,9 @@ chmod +x %{buildroot}%{_bindir}/Rsa2048Sha256GenerateKeys %endif %changelog +* Tue Aug 31 2021 miaoyubo - 202002-8 +- MdeModulePkg/LzmaCustomDecompressLib: catch 4GB+ uncompressed + * Fri Jul 30 2021 Zhenyu Ye - 202002-7 - ArmPkg/CompilerIntrinsicsLib: provide atomics intrinsics -- Gitee From 5a3c9bbbde91d031f2ddab4ff8912243f25e67aa Mon Sep 17 00:00:00 2001 From: imxcc Date: Wed, 22 Sep 2021 17:31:48 +0800 Subject: [PATCH 2/3] fix cve-2021-38575 Signed-off-by: imxcc --- ...Dxe-wrap-IScsiCHAP-source-files-to-8.patch | 244 ++++++++++++++++++ ...Dxe-simplify-ISCSI_CHAP_AUTH_DATA.In.patch | 64 +++++ ...Dxe-clean-up-ISCSI_CHAP_AUTH_DATA.Ou.patch | 95 +++++++ ...Dxe-clean-up-library-class-dependenc.patch | 94 +++++++ ...Dxe-fix-potential-integer-overflow-i.patch | 147 +++++++++++ ...Dxe-assert-that-IScsiBinToHex-always.patch | 88 +++++++ ...Dxe-reformat-IScsiHexToBin-leading-c.patch | 86 ++++++ ...csiDxe-fix-IScsiHexToBin-hex-parsing.patch | 97 +++++++ ...Dxe-fix-IScsiHexToBin-buffer-overflo.patch | 106 ++++++++ ...Dxe-check-IScsiHexToBin-return-value.patch | 84 ++++++ edk2.spec | 15 +- 11 files changed, 1119 insertions(+), 1 deletion(-) create mode 100644 0018-NetworkPkg-IScsiDxe-wrap-IScsiCHAP-source-files-to-8.patch create mode 100644 0019-NetworkPkg-IScsiDxe-simplify-ISCSI_CHAP_AUTH_DATA.In.patch create mode 100644 0020-NetworkPkg-IScsiDxe-clean-up-ISCSI_CHAP_AUTH_DATA.Ou.patch create mode 100644 0021-NetworkPkg-IScsiDxe-clean-up-library-class-dependenc.patch create mode 100644 0022-NetworkPkg-IScsiDxe-fix-potential-integer-overflow-i.patch create mode 100644 0023-NetworkPkg-IScsiDxe-assert-that-IScsiBinToHex-always.patch create mode 100644 0024-NetworkPkg-IScsiDxe-reformat-IScsiHexToBin-leading-c.patch create mode 100644 0025-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-hex-parsing.patch create mode 100644 0026-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-buffer-overflo.patch create mode 100644 0027-NetworkPkg-IScsiDxe-check-IScsiHexToBin-return-value.patch diff --git a/0018-NetworkPkg-IScsiDxe-wrap-IScsiCHAP-source-files-to-8.patch b/0018-NetworkPkg-IScsiDxe-wrap-IScsiCHAP-source-files-to-8.patch new file mode 100644 index 0000000..8b41381 --- /dev/null +++ b/0018-NetworkPkg-IScsiDxe-wrap-IScsiCHAP-source-files-to-8.patch @@ -0,0 +1,244 @@ +From 83761337ec91fbd459c55d7d956fcc25df3bfa50 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:50 +0200 +Subject: [PATCH 18/27] NetworkPkg/IScsiDxe: wrap IScsiCHAP source files to 80 + characters +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Working with overlong lines is difficult for me; rewrap the CHAP-related +source files in IScsiDxe to 80 characters width. No functional changes. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Maciej Rabeda +Reviewed-by: Philippe Mathieu-Daudé +Message-Id: <20210608121259.32451-2-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiCHAP.c | 90 +++++++++++++++++++++++++-------- + NetworkPkg/IScsiDxe/IScsiCHAP.h | 3 +- + 2 files changed, 71 insertions(+), 22 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c +index 355c6f129f..cbbc56ae5b 100644 +--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c ++++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c +@@ -1,5 +1,6 @@ + /** @file +- This file is for Challenge-Handshake Authentication Protocol (CHAP) Configuration. ++ This file is for Challenge-Handshake Authentication Protocol (CHAP) ++ Configuration. + + Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent +@@ -18,9 +19,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent + @param[in] ChallengeLength The length of iSCSI CHAP challenge message. + @param[out] ChapResponse The calculation of the expected hash value. + +- @retval EFI_SUCCESS The expected hash value was calculatedly successfully. +- @retval EFI_PROTOCOL_ERROR The length of the secret should be at least the +- length of the hash value for the hashing algorithm chosen. ++ @retval EFI_SUCCESS The expected hash value was calculatedly ++ successfully. ++ @retval EFI_PROTOCOL_ERROR The length of the secret should be at least ++ the length of the hash value for the hashing ++ algorithm chosen. + @retval EFI_PROTOCOL_ERROR MD5 hash operation fail. + @retval EFI_OUT_OF_RESOURCES Fail to allocate resource to complete MD5. + +@@ -94,8 +97,10 @@ Exit: + @param[in] AuthData iSCSI CHAP authentication data. + @param[in] TargetResponse The response from target. + +- @retval EFI_SUCCESS The response from target passed authentication. +- @retval EFI_SECURITY_VIOLATION The response from target was not expected value. ++ @retval EFI_SUCCESS The response from target passed ++ authentication. ++ @retval EFI_SECURITY_VIOLATION The response from target was not expected ++ value. + @retval Others Other errors as indicated. + + **/ +@@ -193,7 +198,10 @@ IScsiCHAPOnRspReceived ( + // + // The first Login Response. + // +- Value = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_TARGET_PORTAL_GROUP_TAG); ++ Value = IScsiGetValueByKeyFromList ( ++ KeyValueList, ++ ISCSI_KEY_TARGET_PORTAL_GROUP_TAG ++ ); + if (Value == NULL) { + goto ON_EXIT; + } +@@ -205,13 +213,17 @@ IScsiCHAPOnRspReceived ( + + Session->TargetPortalGroupTag = (UINT16) Result; + +- Value = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_AUTH_METHOD); ++ Value = IScsiGetValueByKeyFromList ( ++ KeyValueList, ++ ISCSI_KEY_AUTH_METHOD ++ ); + if (Value == NULL) { + goto ON_EXIT; + } + // +- // Initiator mandates CHAP authentication but target replies without "CHAP", or +- // initiator suggets "None" but target replies with some kind of auth method. ++ // Initiator mandates CHAP authentication but target replies without ++ // "CHAP", or initiator suggets "None" but target replies with some kind of ++ // auth method. + // + if (Session->AuthType == ISCSI_AUTH_TYPE_NONE) { + if (AsciiStrCmp (Value, ISCSI_KEY_VALUE_NONE) != 0) { +@@ -236,7 +248,10 @@ IScsiCHAPOnRspReceived ( + // + // The Target replies with CHAP_A= CHAP_I= CHAP_C= + // +- Value = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_ALGORITHM); ++ Value = IScsiGetValueByKeyFromList ( ++ KeyValueList, ++ ISCSI_KEY_CHAP_ALGORITHM ++ ); + if (Value == NULL) { + goto ON_EXIT; + } +@@ -249,12 +264,18 @@ IScsiCHAPOnRspReceived ( + goto ON_EXIT; + } + +- Identifier = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_IDENTIFIER); ++ Identifier = IScsiGetValueByKeyFromList ( ++ KeyValueList, ++ ISCSI_KEY_CHAP_IDENTIFIER ++ ); + if (Identifier == NULL) { + goto ON_EXIT; + } + +- Challenge = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_CHALLENGE); ++ Challenge = IScsiGetValueByKeyFromList ( ++ KeyValueList, ++ ISCSI_KEY_CHAP_CHALLENGE ++ ); + if (Challenge == NULL) { + goto ON_EXIT; + } +@@ -269,7 +290,11 @@ IScsiCHAPOnRspReceived ( + + AuthData->InIdentifier = (UINT32) Result; + AuthData->InChallengeLength = ISCSI_CHAP_AUTH_MAX_LEN; +- IScsiHexToBin ((UINT8 *) AuthData->InChallenge, &AuthData->InChallengeLength, Challenge); ++ IScsiHexToBin ( ++ (UINT8 *) AuthData->InChallenge, ++ &AuthData->InChallengeLength, ++ Challenge ++ ); + Status = IScsiCHAPCalculateResponse ( + AuthData->InIdentifier, + AuthData->AuthConfig->CHAPSecret, +@@ -303,7 +328,10 @@ IScsiCHAPOnRspReceived ( + goto ON_EXIT; + } + +- Response = IScsiGetValueByKeyFromList (KeyValueList, ISCSI_KEY_CHAP_RESPONSE); ++ Response = IScsiGetValueByKeyFromList ( ++ KeyValueList, ++ ISCSI_KEY_CHAP_RESPONSE ++ ); + if (Response == NULL) { + goto ON_EXIT; + } +@@ -341,7 +369,8 @@ ON_EXIT: + @param[in, out] Pdu The PDU to send out. + + @retval EFI_SUCCESS All check passed and the phase-related CHAP +- authentication info is filled into the iSCSI PDU. ++ authentication info is filled into the iSCSI ++ PDU. + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. + @retval EFI_PROTOCOL_ERROR Some kind of protocol error occurred. + +@@ -392,7 +421,11 @@ IScsiCHAPToSendReq ( + // It's the initial Login Request. Fill in the key=value pairs mandatory + // for the initial Login Request. + // +- IScsiAddKeyValuePair (Pdu, ISCSI_KEY_INITIATOR_NAME, mPrivate->InitiatorName); ++ IScsiAddKeyValuePair ( ++ Pdu, ++ ISCSI_KEY_INITIATOR_NAME, ++ mPrivate->InitiatorName ++ ); + IScsiAddKeyValuePair (Pdu, ISCSI_KEY_SESSION_TYPE, "Normal"); + IScsiAddKeyValuePair ( + Pdu, +@@ -413,7 +446,8 @@ IScsiCHAPToSendReq ( + + case ISCSI_CHAP_STEP_ONE: + // +- // First step, send the Login Request with CHAP_A= key-value pair. ++ // First step, send the Login Request with CHAP_A= key-value ++ // pair. + // + AsciiSPrint (ValueStr, sizeof (ValueStr), "%d", ISCSI_CHAP_ALGORITHM_MD5); + IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_ALGORITHM, ValueStr); +@@ -429,11 +463,20 @@ IScsiCHAPToSendReq ( + // + // CHAP_N= + // +- IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_NAME, (CHAR8 *) &AuthData->AuthConfig->CHAPName); ++ IScsiAddKeyValuePair ( ++ Pdu, ++ ISCSI_KEY_CHAP_NAME, ++ (CHAR8 *) &AuthData->AuthConfig->CHAPName ++ ); + // + // CHAP_R= + // +- IScsiBinToHex ((UINT8 *) AuthData->CHAPResponse, ISCSI_CHAP_RSP_LEN, Response, &RspLen); ++ IScsiBinToHex ( ++ (UINT8 *) AuthData->CHAPResponse, ++ ISCSI_CHAP_RSP_LEN, ++ Response, ++ &RspLen ++ ); + IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_RESPONSE, Response); + + if (AuthData->AuthConfig->CHAPType == ISCSI_CHAP_MUTUAL) { +@@ -448,7 +491,12 @@ IScsiCHAPToSendReq ( + // + IScsiGenRandom ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN); + AuthData->OutChallengeLength = ISCSI_CHAP_RSP_LEN; +- IScsiBinToHex ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN, Challenge, &ChallengeLen); ++ IScsiBinToHex ( ++ (UINT8 *) AuthData->OutChallenge, ++ ISCSI_CHAP_RSP_LEN, ++ Challenge, ++ &ChallengeLen ++ ); + IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_CHALLENGE, Challenge); + + Conn->AuthStep = ISCSI_CHAP_STEP_FOUR; +diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h b/NetworkPkg/IScsiDxe/IScsiCHAP.h +index 140bba0dcd..5e59fb678b 100644 +--- a/NetworkPkg/IScsiDxe/IScsiCHAP.h ++++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h +@@ -88,7 +88,8 @@ IScsiCHAPOnRspReceived ( + @param[in, out] Pdu The PDU to send out. + + @retval EFI_SUCCESS All check passed and the phase-related CHAP +- authentication info is filled into the iSCSI PDU. ++ authentication info is filled into the iSCSI ++ PDU. + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. + @retval EFI_PROTOCOL_ERROR Some kind of protocol error occurred. + +-- +2.27.0 + diff --git a/0019-NetworkPkg-IScsiDxe-simplify-ISCSI_CHAP_AUTH_DATA.In.patch b/0019-NetworkPkg-IScsiDxe-simplify-ISCSI_CHAP_AUTH_DATA.In.patch new file mode 100644 index 0000000..7ddeeaa --- /dev/null +++ b/0019-NetworkPkg-IScsiDxe-simplify-ISCSI_CHAP_AUTH_DATA.In.patch @@ -0,0 +1,64 @@ +From 29cab43bb7912a12efa5a78dac15394aee866e4c Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:51 +0200 +Subject: [PATCH 19/27] NetworkPkg/IScsiDxe: simplify + "ISCSI_CHAP_AUTH_DATA.InChallenge" size +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The ISCSI_CHAP_AUTH_MAX_LEN macro is defined with value 1024. + +The usage of this macro currently involves a semantic (not functional) +bug, which we're going to fix in a subsequent patch, eliminating +ISCSI_CHAP_AUTH_MAX_LEN altogether. + +For now, remove the macro's usage from all +"ISCSI_CHAP_AUTH_DATA.InChallenge" contexts. This is doable without +duplicating open-coded constants. + +No changes in functionality. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Maciej Rabeda +Message-Id: <20210608121259.32451-3-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiCHAP.c | 2 +- + NetworkPkg/IScsiDxe/IScsiCHAP.h | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c +index cbbc56ae5b..df3c2eb120 100644 +--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c ++++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c +@@ -289,7 +289,7 @@ IScsiCHAPOnRspReceived ( + } + + AuthData->InIdentifier = (UINT32) Result; +- AuthData->InChallengeLength = ISCSI_CHAP_AUTH_MAX_LEN; ++ AuthData->InChallengeLength = (UINT32) sizeof (AuthData->InChallenge); + IScsiHexToBin ( + (UINT8 *) AuthData->InChallenge, + &AuthData->InChallengeLength, +diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h b/NetworkPkg/IScsiDxe/IScsiCHAP.h +index 5e59fb678b..1fc1d96ea3 100644 +--- a/NetworkPkg/IScsiDxe/IScsiCHAP.h ++++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h +@@ -49,7 +49,7 @@ typedef struct _ISCSI_CHAP_AUTH_CONFIG_NVDATA { + typedef struct _ISCSI_CHAP_AUTH_DATA { + ISCSI_CHAP_AUTH_CONFIG_NVDATA *AuthConfig; + UINT32 InIdentifier; +- UINT8 InChallenge[ISCSI_CHAP_AUTH_MAX_LEN]; ++ UINT8 InChallenge[1024]; + UINT32 InChallengeLength; + // + // Calculated CHAP Response (CHAP_R) value. +-- +2.27.0 + diff --git a/0020-NetworkPkg-IScsiDxe-clean-up-ISCSI_CHAP_AUTH_DATA.Ou.patch b/0020-NetworkPkg-IScsiDxe-clean-up-ISCSI_CHAP_AUTH_DATA.Ou.patch new file mode 100644 index 0000000..82ee449 --- /dev/null +++ b/0020-NetworkPkg-IScsiDxe-clean-up-ISCSI_CHAP_AUTH_DATA.Ou.patch @@ -0,0 +1,95 @@ +From 95616b866187b00355042953efa5c198df07250f Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:52 +0200 +Subject: [PATCH 20/27] NetworkPkg/IScsiDxe: clean up + "ISCSI_CHAP_AUTH_DATA.OutChallengeLength" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The "ISCSI_CHAP_AUTH_DATA.OutChallenge" field is declared as a UINT8 array +with ISCSI_CHAP_AUTH_MAX_LEN (1024) elements. However, when the challenge +is generated and formatted, only ISCSI_CHAP_RSP_LEN (16) octets are used +in the array. + +Change the array size to ISCSI_CHAP_RSP_LEN, and remove the (now unused) +ISCSI_CHAP_AUTH_MAX_LEN macro. + +Remove the "ISCSI_CHAP_AUTH_DATA.OutChallengeLength" field, which is +superfluous too. + +Most importantly, explain in a new comment *why* tying the challenge size +to the digest size (ISCSI_CHAP_RSP_LEN) has always made sense. (See also +Linux kernel commit 19f5f88ed779, "scsi: target: iscsi: tie the challenge +length to the hash digest size", 2019-11-06.) For sure, the motivation +that the new comment now explains has always been there, and has always +been the same, for IScsiDxe; it's just that now we spell it out too. + +No change in peer-visible behavior. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Maciej Rabeda +Message-Id: <20210608121259.32451-4-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiCHAP.c | 3 +-- + NetworkPkg/IScsiDxe/IScsiCHAP.h | 9 ++++++--- + 2 files changed, 7 insertions(+), 5 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c +index df3c2eb120..9e192ce292 100644 +--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c ++++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c +@@ -122,7 +122,7 @@ IScsiCHAPAuthTarget ( + AuthData->AuthConfig->ReverseCHAPSecret, + SecretSize, + AuthData->OutChallenge, +- AuthData->OutChallengeLength, ++ ISCSI_CHAP_RSP_LEN, // ChallengeLength + VerifyRsp + ); + +@@ -490,7 +490,6 @@ IScsiCHAPToSendReq ( + // CHAP_C= + // + IScsiGenRandom ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN); +- AuthData->OutChallengeLength = ISCSI_CHAP_RSP_LEN; + IScsiBinToHex ( + (UINT8 *) AuthData->OutChallenge, + ISCSI_CHAP_RSP_LEN, +diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.h b/NetworkPkg/IScsiDxe/IScsiCHAP.h +index 1fc1d96ea3..35d5d6ec29 100644 +--- a/NetworkPkg/IScsiDxe/IScsiCHAP.h ++++ b/NetworkPkg/IScsiDxe/IScsiCHAP.h +@@ -19,7 +19,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent + + #define ISCSI_CHAP_ALGORITHM_MD5 5 + +-#define ISCSI_CHAP_AUTH_MAX_LEN 1024 + /// + /// MD5_HASHSIZE + /// +@@ -59,9 +58,13 @@ typedef struct _ISCSI_CHAP_AUTH_DATA { + // + // Auth-data to be sent out for mutual authentication. + // ++ // While the challenge size is technically independent of the hashing ++ // algorithm, it is good practice to avoid hashing *fewer bytes* than the ++ // digest size. In other words, it's good practice to feed *at least as many ++ // bytes* to the hashing algorithm as the hashing algorithm will output. ++ // + UINT32 OutIdentifier; +- UINT8 OutChallenge[ISCSI_CHAP_AUTH_MAX_LEN]; +- UINT32 OutChallengeLength; ++ UINT8 OutChallenge[ISCSI_CHAP_RSP_LEN]; + } ISCSI_CHAP_AUTH_DATA; + + /** +-- +2.27.0 + diff --git a/0021-NetworkPkg-IScsiDxe-clean-up-library-class-dependenc.patch b/0021-NetworkPkg-IScsiDxe-clean-up-library-class-dependenc.patch new file mode 100644 index 0000000..2be51c1 --- /dev/null +++ b/0021-NetworkPkg-IScsiDxe-clean-up-library-class-dependenc.patch @@ -0,0 +1,94 @@ +From e8f28b09e63dfdbb4169969a43c65f86c44b035a Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:53 +0200 +Subject: [PATCH 21/27] NetworkPkg/IScsiDxe: clean up library class + dependencies +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Sort the library class dependencies in the #include directives and in the +INF file. Remove the DpcLib class from the #include directives -- it is +not listed in the INF file, and IScsiDxe doesn't call either DpcLib API +(QueueDpc(), DispatchDpc()). No functional changes. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Maciej Rabeda +Message-Id: <20210608121259.32451-5-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiDxe.inf | 6 +++--- + NetworkPkg/IScsiDxe/IScsiImpl.h | 17 ++++++++--------- + 2 files changed, 11 insertions(+), 12 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDxe.inf +index 0ffb340ce0..543c408302 100644 +--- a/NetworkPkg/IScsiDxe/IScsiDxe.inf ++++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf +@@ -65,6 +65,7 @@ + NetworkPkg/NetworkPkg.dec + + [LibraryClasses] ++ BaseCryptLib + BaseLib + BaseMemoryLib + DebugLib +@@ -72,14 +73,13 @@ + HiiLib + MemoryAllocationLib + NetLib +- TcpIoLib + PrintLib ++ TcpIoLib + UefiBootServicesTableLib + UefiDriverEntryPoint ++ UefiHiiServicesLib + UefiLib + UefiRuntimeServicesTableLib +- UefiHiiServicesLib +- BaseCryptLib + + [Protocols] + gEfiAcpiTableProtocolGuid ## SOMETIMES_CONSUMES ## SystemTable +diff --git a/NetworkPkg/IScsiDxe/IScsiImpl.h b/NetworkPkg/IScsiDxe/IScsiImpl.h +index 387ab9765e..d895c7feb9 100644 +--- a/NetworkPkg/IScsiDxe/IScsiImpl.h ++++ b/NetworkPkg/IScsiDxe/IScsiImpl.h +@@ -35,21 +35,20 @@ SPDX-License-Identifier: BSD-2-Clause-Patent + #include + #include + +-#include +-#include +-#include +-#include ++#include + #include + #include ++#include ++#include ++#include + #include ++#include + #include ++#include + #include +-#include ++#include + #include +-#include +-#include +-#include +-#include ++#include + + #include + #include +-- +2.27.0 + diff --git a/0022-NetworkPkg-IScsiDxe-fix-potential-integer-overflow-i.patch b/0022-NetworkPkg-IScsiDxe-fix-potential-integer-overflow-i.patch new file mode 100644 index 0000000..f1eddbe --- /dev/null +++ b/0022-NetworkPkg-IScsiDxe-fix-potential-integer-overflow-i.patch @@ -0,0 +1,147 @@ +From cf01b2dc8fc3ff9cf49fb891af5703dc03e3193e Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:54 +0200 +Subject: [PATCH 22/27] NetworkPkg/IScsiDxe: fix potential integer overflow in + IScsiBinToHex() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Considering IScsiBinToHex(): + +> if (((*HexLength) - 3) < BinLength * 2) { +> *HexLength = BinLength * 2 + 3; +> } + +the following subexpressions are problematic: + + (*HexLength) - 3 + BinLength * 2 + BinLength * 2 + 3 + +The first one may wrap under zero, the latter two may wrap over +MAX_UINT32. + +Rewrite the calculation using SafeIntLib. + +While at it, change the type of the "Index" variable from UINTN to UINT32. +The largest "Index"-based value that we calculate is + + Index * 2 + 2 (with (Index == BinLength)) + +Because the patch makes + + BinLength * 2 + 3 + +safe to calculate in UINT32, using UINT32 for + + Index * 2 + 2 (with (Index == BinLength)) + +is safe too. Consistently using UINT32 improves readability. + +This patch is best reviewed with "git show -W". + +The integer overflows that this patch fixes are theoretical; a subsequent +patch in the series will audit the IScsiBinToHex() call sites, and show +that none of them can fail. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Maciej Rabeda +Reviewed-by: Philippe Mathieu-Daudé +Message-Id: <20210608121259.32451-6-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiDxe.inf | 1 + + NetworkPkg/IScsiDxe/IScsiImpl.h | 1 + + NetworkPkg/IScsiDxe/IScsiMisc.c | 19 +++++++++++++++---- + NetworkPkg/IScsiDxe/IScsiMisc.h | 1 + + 4 files changed, 18 insertions(+), 4 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiDxe.inf b/NetworkPkg/IScsiDxe/IScsiDxe.inf +index 543c408302..1dde56d00c 100644 +--- a/NetworkPkg/IScsiDxe/IScsiDxe.inf ++++ b/NetworkPkg/IScsiDxe/IScsiDxe.inf +@@ -74,6 +74,7 @@ + MemoryAllocationLib + NetLib + PrintLib ++ SafeIntLib + TcpIoLib + UefiBootServicesTableLib + UefiDriverEntryPoint +diff --git a/NetworkPkg/IScsiDxe/IScsiImpl.h b/NetworkPkg/IScsiDxe/IScsiImpl.h +index d895c7feb9..ac3a25730e 100644 +--- a/NetworkPkg/IScsiDxe/IScsiImpl.h ++++ b/NetworkPkg/IScsiDxe/IScsiImpl.h +@@ -44,6 +44,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent + #include + #include + #include ++#include + #include + #include + #include +diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c +index b8fef3ff6f..42988e15cb 100644 +--- a/NetworkPkg/IScsiDxe/IScsiMisc.c ++++ b/NetworkPkg/IScsiDxe/IScsiMisc.c +@@ -316,6 +316,7 @@ IScsiMacAddrToStr ( + @retval EFI_SUCCESS The binary data is converted to the hexadecimal string + and the length of the string is updated. + @retval EFI_BUFFER_TOO_SMALL The string is too small. ++ @retval EFI_BAD_BUFFER_SIZE BinLength is too large for hex encoding. + @retval EFI_INVALID_PARAMETER The IP string is malformatted. + + **/ +@@ -327,18 +328,28 @@ IScsiBinToHex ( + IN OUT UINT32 *HexLength + ) + { +- UINTN Index; ++ UINT32 HexLengthMin; ++ UINT32 HexLengthProvided; ++ UINT32 Index; + + if ((HexStr == NULL) || (BinBuffer == NULL) || (BinLength == 0)) { + return EFI_INVALID_PARAMETER; + } + +- if (((*HexLength) - 3) < BinLength * 2) { +- *HexLength = BinLength * 2 + 3; ++ // ++ // Safely calculate: HexLengthMin := BinLength * 2 + 3. ++ // ++ if (RETURN_ERROR (SafeUint32Mult (BinLength, 2, &HexLengthMin)) || ++ RETURN_ERROR (SafeUint32Add (HexLengthMin, 3, &HexLengthMin))) { ++ return EFI_BAD_BUFFER_SIZE; ++ } ++ ++ HexLengthProvided = *HexLength; ++ *HexLength = HexLengthMin; ++ if (HexLengthProvided < HexLengthMin) { + return EFI_BUFFER_TOO_SMALL; + } + +- *HexLength = BinLength * 2 + 3; + // + // Prefix for Hex String. + // +diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h +index 46c725aab3..231413993b 100644 +--- a/NetworkPkg/IScsiDxe/IScsiMisc.h ++++ b/NetworkPkg/IScsiDxe/IScsiMisc.h +@@ -150,6 +150,7 @@ IScsiAsciiStrToIp ( + @retval EFI_SUCCESS The binary data is converted to the hexadecimal string + and the length of the string is updated. + @retval EFI_BUFFER_TOO_SMALL The string is too small. ++ @retval EFI_BAD_BUFFER_SIZE BinLength is too large for hex encoding. + @retval EFI_INVALID_PARAMETER The IP string is malformatted. + + **/ +-- +2.27.0 + diff --git a/0023-NetworkPkg-IScsiDxe-assert-that-IScsiBinToHex-always.patch b/0023-NetworkPkg-IScsiDxe-assert-that-IScsiBinToHex-always.patch new file mode 100644 index 0000000..82c659e --- /dev/null +++ b/0023-NetworkPkg-IScsiDxe-assert-that-IScsiBinToHex-always.patch @@ -0,0 +1,88 @@ +From d90fff40cb2502b627370a77f5608c8a178c3f78 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:55 +0200 +Subject: [PATCH 23/27] NetworkPkg/IScsiDxe: assert that IScsiBinToHex() always + succeeds +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +IScsiBinToHex() is called for encoding: + +- the answer to the target's challenge; that is, CHAP_R; + +- the challenge for the target, in case mutual authentication is enabled; + that is, CHAP_C. + +The initiator controls the size of both blobs, the sizes of their hex +encodings are correctly calculated in "RspLen" and "ChallengeLen". +Therefore the IScsiBinToHex() calls never fail; assert that. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Maciej Rabeda +Message-Id: <20210608121259.32451-7-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiCHAP.c | 27 +++++++++++++++------------ + 1 file changed, 15 insertions(+), 12 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c +index 9e192ce292..dbe3c8ef46 100644 +--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c ++++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c +@@ -391,6 +391,7 @@ IScsiCHAPToSendReq ( + UINT32 RspLen; + CHAR8 *Challenge; + UINT32 ChallengeLen; ++ EFI_STATUS BinToHexStatus; + + ASSERT (Conn->CurrentStage == ISCSI_SECURITY_NEGOTIATION); + +@@ -471,12 +472,13 @@ IScsiCHAPToSendReq ( + // + // CHAP_R= + // +- IScsiBinToHex ( +- (UINT8 *) AuthData->CHAPResponse, +- ISCSI_CHAP_RSP_LEN, +- Response, +- &RspLen +- ); ++ BinToHexStatus = IScsiBinToHex ( ++ (UINT8 *) AuthData->CHAPResponse, ++ ISCSI_CHAP_RSP_LEN, ++ Response, ++ &RspLen ++ ); ++ ASSERT_EFI_ERROR (BinToHexStatus); + IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_RESPONSE, Response); + + if (AuthData->AuthConfig->CHAPType == ISCSI_CHAP_MUTUAL) { +@@ -490,12 +492,13 @@ IScsiCHAPToSendReq ( + // CHAP_C= + // + IScsiGenRandom ((UINT8 *) AuthData->OutChallenge, ISCSI_CHAP_RSP_LEN); +- IScsiBinToHex ( +- (UINT8 *) AuthData->OutChallenge, +- ISCSI_CHAP_RSP_LEN, +- Challenge, +- &ChallengeLen +- ); ++ BinToHexStatus = IScsiBinToHex ( ++ (UINT8 *) AuthData->OutChallenge, ++ ISCSI_CHAP_RSP_LEN, ++ Challenge, ++ &ChallengeLen ++ ); ++ ASSERT_EFI_ERROR (BinToHexStatus); + IScsiAddKeyValuePair (Pdu, ISCSI_KEY_CHAP_CHALLENGE, Challenge); + + Conn->AuthStep = ISCSI_CHAP_STEP_FOUR; +-- +2.27.0 + diff --git a/0024-NetworkPkg-IScsiDxe-reformat-IScsiHexToBin-leading-c.patch b/0024-NetworkPkg-IScsiDxe-reformat-IScsiHexToBin-leading-c.patch new file mode 100644 index 0000000..2a3f310 --- /dev/null +++ b/0024-NetworkPkg-IScsiDxe-reformat-IScsiHexToBin-leading-c.patch @@ -0,0 +1,86 @@ +From dc469f137110fe79704b8b92c552972c739bb915 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:56 +0200 +Subject: [PATCH 24/27] NetworkPkg/IScsiDxe: reformat IScsiHexToBin() leading + comment block +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +We'll need further return values for IScsiHexToBin() in a subsequent +patch; make room for them in the leading comment block of the function. +While at it, rewrap the comment block to 80 characters width. + +No functional changes. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Maciej Rabeda +Reviewed-by: Philippe Mathieu-Daudé +Message-Id: <20210608121259.32451-8-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiMisc.c | 16 ++++++++-------- + NetworkPkg/IScsiDxe/IScsiMisc.h | 16 ++++++++-------- + 2 files changed, 16 insertions(+), 16 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c +index 42988e15cb..014700e87a 100644 +--- a/NetworkPkg/IScsiDxe/IScsiMisc.c ++++ b/NetworkPkg/IScsiDxe/IScsiMisc.c +@@ -370,14 +370,14 @@ IScsiBinToHex ( + /** + Convert the hexadecimal string into a binary encoded buffer. + +- @param[in, out] BinBuffer The binary buffer. +- @param[in, out] BinLength Length of the binary buffer. +- @param[in] HexStr The hexadecimal string. +- +- @retval EFI_SUCCESS The hexadecimal string is converted into a binary +- encoded buffer. +- @retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the converted data. +- ++ @param[in, out] BinBuffer The binary buffer. ++ @param[in, out] BinLength Length of the binary buffer. ++ @param[in] HexStr The hexadecimal string. ++ ++ @retval EFI_SUCCESS The hexadecimal string is converted into a ++ binary encoded buffer. ++ @retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the ++ converted data. + **/ + EFI_STATUS + IScsiHexToBin ( +diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h +index 231413993b..28cf408cd5 100644 +--- a/NetworkPkg/IScsiDxe/IScsiMisc.h ++++ b/NetworkPkg/IScsiDxe/IScsiMisc.h +@@ -165,14 +165,14 @@ IScsiBinToHex ( + /** + Convert the hexadecimal string into a binary encoded buffer. + +- @param[in, out] BinBuffer The binary buffer. +- @param[in, out] BinLength Length of the binary buffer. +- @param[in] HexStr The hexadecimal string. +- +- @retval EFI_SUCCESS The hexadecimal string is converted into a binary +- encoded buffer. +- @retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the converted data. +- ++ @param[in, out] BinBuffer The binary buffer. ++ @param[in, out] BinLength Length of the binary buffer. ++ @param[in] HexStr The hexadecimal string. ++ ++ @retval EFI_SUCCESS The hexadecimal string is converted into a ++ binary encoded buffer. ++ @retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the ++ converted data. + **/ + EFI_STATUS + IScsiHexToBin ( +-- +2.27.0 + diff --git a/0025-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-hex-parsing.patch b/0025-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-hex-parsing.patch new file mode 100644 index 0000000..0996638 --- /dev/null +++ b/0025-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-hex-parsing.patch @@ -0,0 +1,97 @@ +From 47b76780b487dbfde4efb6843b16064c4a97e94d Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:57 +0200 +Subject: [PATCH 25/27] NetworkPkg/IScsiDxe: fix IScsiHexToBin() hex parsing +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The IScsiHexToBin() function has the following parser issues: + +(1) If the *subject sequence* in "HexStr" is empty, the function returns + EFI_SUCCESS (with "BinLength" set to 0 on output). Such inputs should + be rejected. + +(2) The function mis-handles a "HexStr" that ends with a stray nibble. For + example, if "HexStr" is "0xABC", the function decodes it to the bytes + {0xAB, 0x0C}, sets "BinLength" to 2 on output, and returns + EFI_SUCCESS. Such inputs should be rejected. + +(3) If an invalid hex char is found in "HexStr", the function treats it as + end-of-hex-string, and returns EFI_SUCCESS. Such inputs should be + rejected. + +All of the above cases are remotely triggerable, as shown in a subsequent +patch, which adds error checking to the IScsiHexToBin() call sites. While +the initiator is not immediately compromised, incorrectly parsing CHAP_R +from the target, in case of mutual authentication, is not great. + +Extend the interface contract of IScsiHexToBin() with +EFI_INVALID_PARAMETER, for reporting issues (1) through (3), and implement +the new checks. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Maciej Rabeda +Reviewed-by: Philippe Mathieu-Daudé +Message-Id: <20210608121259.32451-9-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiMisc.c | 12 ++++++++++-- + NetworkPkg/IScsiDxe/IScsiMisc.h | 1 + + 2 files changed, 11 insertions(+), 2 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c +index 014700e87a..f0f4992b07 100644 +--- a/NetworkPkg/IScsiDxe/IScsiMisc.c ++++ b/NetworkPkg/IScsiDxe/IScsiMisc.c +@@ -376,6 +376,7 @@ IScsiBinToHex ( + + @retval EFI_SUCCESS The hexadecimal string is converted into a + binary encoded buffer. ++ @retval EFI_INVALID_PARAMETER Invalid hex encoding found in HexStr. + @retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the + converted data. + **/ +@@ -402,14 +403,21 @@ IScsiHexToBin ( + + Length = AsciiStrLen (HexStr); + ++ // ++ // Reject an empty hex string; reject a stray nibble. ++ // ++ if (Length == 0 || Length % 2 != 0) { ++ return EFI_INVALID_PARAMETER; ++ } ++ + for (Index = 0; Index < Length; Index ++) { + TemStr[0] = HexStr[Index]; + Digit = (UINT8) AsciiStrHexToUint64 (TemStr); + if (Digit == 0 && TemStr[0] != '0') { + // +- // Invalid Lun Char. ++ // Invalid Hex Char. + // +- break; ++ return EFI_INVALID_PARAMETER; + } + if ((Index & 1) == 0) { + BinBuffer [Index/2] = Digit; +diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h +index 28cf408cd5..404a482e57 100644 +--- a/NetworkPkg/IScsiDxe/IScsiMisc.h ++++ b/NetworkPkg/IScsiDxe/IScsiMisc.h +@@ -171,6 +171,7 @@ IScsiBinToHex ( + + @retval EFI_SUCCESS The hexadecimal string is converted into a + binary encoded buffer. ++ @retval EFI_INVALID_PARAMETER Invalid hex encoding found in HexStr. + @retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the + converted data. + **/ +-- +2.27.0 + diff --git a/0026-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-buffer-overflo.patch b/0026-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-buffer-overflo.patch new file mode 100644 index 0000000..6c2861e --- /dev/null +++ b/0026-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-buffer-overflo.patch @@ -0,0 +1,106 @@ +From 54e90edaed0d7c15230902ac4d74f4304bad2ebd Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:58 +0200 +Subject: [PATCH 26/27] NetworkPkg/IScsiDxe: fix IScsiHexToBin() buffer + overflow +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The IScsiHexToBin() function documents the EFI_BUFFER_TOO_SMALL return +condition, but never actually checks whether the decoded buffer fits into +the caller-provided room (i.e., the input value of "BinLength"), and +EFI_BUFFER_TOO_SMALL is never returned. The decoding of "HexStr" can +overflow "BinBuffer". + +This is remotely exploitable, as shown in a subsequent patch, which adds +error checking to the IScsiHexToBin() call sites. This issue allows the +target to compromise the initiator. + +Introduce EFI_BAD_BUFFER_SIZE, in addition to the existent +EFI_BUFFER_TOO_SMALL, for reporting a special case of the buffer overflow, +plus actually catch the buffer overflow. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Maciej Rabeda +Reviewed-by: Philippe Mathieu-Daudé +Message-Id: <20210608121259.32451-10-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiMisc.c | 20 +++++++++++++++++--- + NetworkPkg/IScsiDxe/IScsiMisc.h | 3 +++ + 2 files changed, 20 insertions(+), 3 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c +index f0f4992b07..4069547867 100644 +--- a/NetworkPkg/IScsiDxe/IScsiMisc.c ++++ b/NetworkPkg/IScsiDxe/IScsiMisc.c +@@ -377,6 +377,9 @@ IScsiBinToHex ( + @retval EFI_SUCCESS The hexadecimal string is converted into a + binary encoded buffer. + @retval EFI_INVALID_PARAMETER Invalid hex encoding found in HexStr. ++ @retval EFI_BAD_BUFFER_SIZE The length of HexStr is too large for decoding: ++ the decoded size cannot be expressed in ++ BinLength on output. + @retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the + converted data. + **/ +@@ -387,6 +390,8 @@ IScsiHexToBin ( + IN CHAR8 *HexStr + ) + { ++ UINTN BinLengthMin; ++ UINT32 BinLengthProvided; + UINTN Index; + UINTN Length; + UINT8 Digit; +@@ -409,6 +414,18 @@ IScsiHexToBin ( + if (Length == 0 || Length % 2 != 0) { + return EFI_INVALID_PARAMETER; + } ++ // ++ // Check if the caller provides enough room for the decoded blob. ++ // ++ BinLengthMin = Length / 2; ++ if (BinLengthMin > MAX_UINT32) { ++ return EFI_BAD_BUFFER_SIZE; ++ } ++ BinLengthProvided = *BinLength; ++ *BinLength = (UINT32)BinLengthMin; ++ if (BinLengthProvided < BinLengthMin) { ++ return EFI_BUFFER_TOO_SMALL; ++ } + + for (Index = 0; Index < Length; Index ++) { + TemStr[0] = HexStr[Index]; +@@ -425,9 +442,6 @@ IScsiHexToBin ( + BinBuffer [Index/2] = (UINT8) ((BinBuffer [Index/2] << 4) + Digit); + } + } +- +- *BinLength = (UINT32) ((Index + 1)/2); +- + return EFI_SUCCESS; + } + +diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.h b/NetworkPkg/IScsiDxe/IScsiMisc.h +index 404a482e57..fddef4f466 100644 +--- a/NetworkPkg/IScsiDxe/IScsiMisc.h ++++ b/NetworkPkg/IScsiDxe/IScsiMisc.h +@@ -172,6 +172,9 @@ IScsiBinToHex ( + @retval EFI_SUCCESS The hexadecimal string is converted into a + binary encoded buffer. + @retval EFI_INVALID_PARAMETER Invalid hex encoding found in HexStr. ++ @retval EFI_BAD_BUFFER_SIZE The length of HexStr is too large for decoding: ++ the decoded size cannot be expressed in ++ BinLength on output. + @retval EFI_BUFFER_TOO_SMALL The binary buffer is too small to hold the + converted data. + **/ +-- +2.27.0 + diff --git a/0027-NetworkPkg-IScsiDxe-check-IScsiHexToBin-return-value.patch b/0027-NetworkPkg-IScsiDxe-check-IScsiHexToBin-return-value.patch new file mode 100644 index 0000000..426abb9 --- /dev/null +++ b/0027-NetworkPkg-IScsiDxe-check-IScsiHexToBin-return-value.patch @@ -0,0 +1,84 @@ +From b8649cf2a3e673a4a8cb6c255e394b354b771550 Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +Date: Tue, 8 Jun 2021 14:12:59 +0200 +Subject: [PATCH 27/27] NetworkPkg/IScsiDxe: check IScsiHexToBin() return + values +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +IScsiDxe (that is, the initiator) receives two hex-encoded strings from +the iSCSI target: + +- CHAP_C, where the target challenges the initiator, + +- CHAP_R, where the target answers the challenge from the initiator (in + case the initiator wants mutual authentication). + +Accordingly, we have two IScsiHexToBin() call sites: + +- At the CHAP_C decoding site, check whether the decoding succeeds. The + decoded buffer ("AuthData->InChallenge") can accommodate 1024 bytes, + which is a permissible restriction on the target, per + . Shorter challenges + from the target are acceptable. + +- At the CHAP_R decoding site, enforce that the decoding both succeed, and + provide exactly ISCSI_CHAP_RSP_LEN bytes. CHAP_R contains the digest + calculated by the target, therefore it must be of fixed size. We may + only call IScsiCHAPAuthTarget() if "TargetRsp" has been fully populated. + +Cc: Jiaxin Wu +Cc: Maciej Rabeda +Cc: Philippe Mathieu-Daudé +Cc: Siyuan Fu +Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3356 +Signed-off-by: Laszlo Ersek +Reviewed-by: Philippe Mathieu-Daudé +Reviewed-by: Maciej Rabeda +Message-Id: <20210608121259.32451-11-lersek@redhat.com> +--- + NetworkPkg/IScsiDxe/IScsiCHAP.c | 20 ++++++++++++++------ + 1 file changed, 14 insertions(+), 6 deletions(-) + +diff --git a/NetworkPkg/IScsiDxe/IScsiCHAP.c b/NetworkPkg/IScsiDxe/IScsiCHAP.c +index dbe3c8ef46..7e930c0d1e 100644 +--- a/NetworkPkg/IScsiDxe/IScsiCHAP.c ++++ b/NetworkPkg/IScsiDxe/IScsiCHAP.c +@@ -290,11 +290,15 @@ IScsiCHAPOnRspReceived ( + + AuthData->InIdentifier = (UINT32) Result; + AuthData->InChallengeLength = (UINT32) sizeof (AuthData->InChallenge); +- IScsiHexToBin ( +- (UINT8 *) AuthData->InChallenge, +- &AuthData->InChallengeLength, +- Challenge +- ); ++ Status = IScsiHexToBin ( ++ (UINT8 *) AuthData->InChallenge, ++ &AuthData->InChallengeLength, ++ Challenge ++ ); ++ if (EFI_ERROR (Status)) { ++ Status = EFI_PROTOCOL_ERROR; ++ goto ON_EXIT; ++ } + Status = IScsiCHAPCalculateResponse ( + AuthData->InIdentifier, + AuthData->AuthConfig->CHAPSecret, +@@ -337,7 +341,11 @@ IScsiCHAPOnRspReceived ( + } + + RspLen = ISCSI_CHAP_RSP_LEN; +- IScsiHexToBin (TargetRsp, &RspLen, Response); ++ Status = IScsiHexToBin (TargetRsp, &RspLen, Response); ++ if (EFI_ERROR (Status) || RspLen != ISCSI_CHAP_RSP_LEN) { ++ Status = EFI_PROTOCOL_ERROR; ++ goto ON_EXIT; ++ } + + // + // Check the CHAP Name and Response replied by Target. +-- +2.27.0 + diff --git a/edk2.spec b/edk2.spec index 23cc167..f126508 100644 --- a/edk2.spec +++ b/edk2.spec @@ -5,7 +5,7 @@ Name: edk2 Version: %{stable_date} -Release: 8 +Release: 9 Summary: EFI Development Kit II License: BSD-2-Clause-Patent URL: https://github.com/tianocore/edk2 @@ -29,6 +29,16 @@ Patch0014: 0014-MdeModulePkg-Core-Dxe-assert-SectionInstance-invariant-in-FindCh Patch0015: 0015-MdeModulePkg-Core-Dxe-limit-FwVol-encapsulation-section-recursion.patch Patch0016: 0016-ArmPkg-CompilerIntrinsicsLib-provide-atomics-intrins.patch Patch0017: 0017-MdeModulePkg-LzmaCustomDecompressLib-catch-4GB-uncom.patch +Patch0018: 0018-NetworkPkg-IScsiDxe-wrap-IScsiCHAP-source-files-to-8.patch +Patch0019: 0019-NetworkPkg-IScsiDxe-simplify-ISCSI_CHAP_AUTH_DATA.In.patch +Patch0020: 0020-NetworkPkg-IScsiDxe-clean-up-ISCSI_CHAP_AUTH_DATA.Ou.patch +Patch0021: 0021-NetworkPkg-IScsiDxe-clean-up-library-class-dependenc.patch +Patch0022: 0022-NetworkPkg-IScsiDxe-fix-potential-integer-overflow-i.patch +Patch0023: 0023-NetworkPkg-IScsiDxe-assert-that-IScsiBinToHex-always.patch +Patch0024: 0024-NetworkPkg-IScsiDxe-reformat-IScsiHexToBin-leading-c.patch +Patch0025: 0025-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-hex-parsing.patch +Patch0026: 0026-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-buffer-overflo.patch +Patch0027: 0027-NetworkPkg-IScsiDxe-check-IScsiHexToBin-return-value.patch BuildRequires: acpica-tools gcc gcc-c++ libuuid-devel python3 bc nasm python3-unversioned-command @@ -226,6 +236,9 @@ chmod +x %{buildroot}%{_bindir}/Rsa2048Sha256GenerateKeys %endif %changelog +* Wed Sep 22 2021 imxcc - 202002-9 +- fix cve-2021-38575 + * Tue Aug 31 2021 miaoyubo - 202002-8 - MdeModulePkg/LzmaCustomDecompressLib: catch 4GB+ uncompressed -- Gitee From e4eb31968af880ae9bc04ee9268e7d4bf6c68fa6 Mon Sep 17 00:00:00 2001 From: Jinhua Cao Date: Wed, 1 Dec 2021 21:38:27 +0800 Subject: [PATCH 3/3] fix CVE-2021-28216 Signed-off-by: Jinhua Cao --- ...T-Lock-boot-performance-table-addres.patch | 982 ++++++++++++++++++ edk2.spec | 6 +- 2 files changed, 987 insertions(+), 1 deletion(-) create mode 100644 0028-MdeModulePkg-FPDT-Lock-boot-performance-table-addres.patch diff --git a/0028-MdeModulePkg-FPDT-Lock-boot-performance-table-addres.patch b/0028-MdeModulePkg-FPDT-Lock-boot-performance-table-addres.patch new file mode 100644 index 0000000..0917f11 --- /dev/null +++ b/0028-MdeModulePkg-FPDT-Lock-boot-performance-table-addres.patch @@ -0,0 +1,982 @@ +From 306307df0e228c73f6ad38ef231db75c4a3478d1 Mon Sep 17 00:00:00 2001 +From: Dandan Bi +Date: Mon, 28 Jun 2021 19:50:22 +0800 +Subject: [PATCH] MdeModulePkg/FPDT: Lock boot performance table address + variable at EndOfDxe + +REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2957 + +1. Allocate performance data table at EndOfDxe and then lock the varible + which store the table address at EndOfDxe. + +2. Enlarge PCD gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize + from 0x20000 to 0x30000 in order to hold the Delta performance data + between EndOfDxe and ReadyToBoot. + +3. SMM performance data is collected by DXE modules through SMM communication + at ReadyToBoot before. + Now to do SMM communication twice, one for allocating the performance + size at EndOfDxe, another is at ReadyToBoot to get SMM performance data. + +4. Make SmmCorePerformanceLib rather than FirmwarePerformanceSmm to communicate + with DxeCorePerformanceLib for SMM performance data and size. + +Cc: Liming Gao +Cc: Hao A Wu +Cc: Jian J Wang +Signed-off-by: Dandan Bi +Reviewed-by: Hao A Wu +Signed-off-by: Jinhua Cao +--- + .../DxeCorePerformanceLib.c | 132 +++++++++++---- + .../DxeCorePerformanceLib.inf | 3 +- + .../SmmCorePerformanceLib.c | 142 ++++++++++++---- + .../SmmCorePerformanceLib.inf | 5 +- + MdeModulePkg/MdeModulePkg.dec | 4 +- + .../FirmwarePerformanceDxe.c | 90 +++++++++-- + .../FirmwarePerformanceDxe.inf | 6 +- + .../FirmwarePerformanceSmm.c | 151 +----------------- + .../FirmwarePerformanceSmm.inf | 4 +- + 9 files changed, 302 insertions(+), 235 deletions(-) + +diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c +index f500e20b32..bcefac6b6c 100644 +--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c ++++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c +@@ -10,7 +10,7 @@ + This library is mainly used by DxeCore to start performance logging to ensure that + Performance Protocol is installed at the very beginning of DXE phase. + +-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
++Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.
+ (C) Copyright 2016 Hewlett Packard Enterprise Development LP
+ SPDX-License-Identifier: BSD-2-Clause-Patent + +@@ -64,7 +64,7 @@ UINT32 mLoadImageCount = 0; + UINT32 mPerformanceLength = 0; + UINT32 mMaxPerformanceLength = 0; + UINT32 mBootRecordSize = 0; +-UINT32 mBootRecordMaxSize = 0; ++UINTN mBootRecordMaxSize = 0; + UINT32 mCachedLength = 0; + + BOOLEAN mFpdtBufferIsReported = FALSE; +@@ -205,25 +205,26 @@ IsKnownID ( + } + + /** +- Allocate buffer for Boot Performance table. ++ This internal function dumps all the SMM performance data and size. + +- @return Status code. ++ @param SmmPerfData Smm Performance data. The buffer contain the SMM perf data is allocated by this function and caller needs to free it. ++ @param SmmPerfDataSize Smm Performance data size. ++ @param SkipGetPerfData Skip to get performance data, just get the size. + + **/ +-EFI_STATUS +-AllocateBootPerformanceTable ( ++VOID ++InternalGetSmmPerfData ( ++ OUT VOID **SmmPerfData, ++ OUT UINTN *SmmPerfDataSize, ++ IN BOOLEAN SkipGetPerfData + ) + { + EFI_STATUS Status; +- UINTN Size; + UINT8 *SmmBootRecordCommBuffer; + EFI_SMM_COMMUNICATE_HEADER *SmmCommBufferHeader; + SMM_BOOT_RECORD_COMMUNICATE *SmmCommData; + UINTN CommSize; +- UINTN BootPerformanceDataSize; +- UINT8 *BootPerformanceData; + EFI_SMM_COMMUNICATION_PROTOCOL *Communication; +- FIRMWARE_PERFORMANCE_VARIABLE PerformanceVariable; + EDKII_PI_SMM_COMMUNICATION_REGION_TABLE *SmmCommRegionTable; + EFI_MEMORY_DESCRIPTOR *SmmCommMemRegion; + UINTN Index; +@@ -237,7 +238,6 @@ AllocateBootPerformanceTable ( + SmmBootRecordCommBuffer = NULL; + SmmCommData = NULL; + SmmBootRecordData = NULL; +- SmmBootRecordDataSize = 0; + ReservedMemSize = 0; + Status = gBS->LocateProtocol (&gEfiSmmCommunicationProtocolGuid, NULL, (VOID **) &Communication); + if (!EFI_ERROR (Status)) { +@@ -284,6 +284,10 @@ AllocateBootPerformanceTable ( + Status = Communication->Communicate (Communication, SmmBootRecordCommBuffer, &CommSize); + + if (!EFI_ERROR (Status) && !EFI_ERROR (SmmCommData->ReturnStatus) && SmmCommData->BootRecordSize != 0) { ++ if (SkipGetPerfData) { ++ *SmmPerfDataSize = SmmCommData->BootRecordSize; ++ return; ++ } + // + // Get all boot records + // +@@ -305,19 +309,45 @@ AllocateBootPerformanceTable ( + } + SmmCommData->BootRecordOffset = SmmCommData->BootRecordOffset + SmmCommData->BootRecordSize; + } ++ *SmmPerfData = SmmBootRecordData; ++ *SmmPerfDataSize = SmmBootRecordDataSize; + } + } + } + } ++} ++ ++/** ++ Allocate buffer for Boot Performance table. ++ ++ @return Status code. ++ ++**/ ++EFI_STATUS ++AllocateBootPerformanceTable ( ++ VOID ++ ) ++{ ++ EFI_STATUS Status; ++ UINTN Size; ++ UINTN BootPerformanceDataSize; ++ UINT8 *BootPerformanceData; ++ FIRMWARE_PERFORMANCE_VARIABLE PerformanceVariable; ++ UINTN SmmBootRecordDataSize; ++ ++ SmmBootRecordDataSize = 0; ++ ++ // ++ // Get SMM performance data size at the point of EndOfDxe in order to allocate the boot performance table. ++ // Will Get all the data at ReadyToBoot. ++ // ++ InternalGetSmmPerfData (NULL, &SmmBootRecordDataSize, TRUE); + + // + // Prepare memory for Boot Performance table. + // Boot Performance table includes BasicBoot record, and one or more appended Boot Records. + // +- BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize); +- if (SmmCommData != NULL && SmmBootRecordData != NULL) { +- BootPerformanceDataSize += SmmBootRecordDataSize; +- } ++ BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + SmmBootRecordDataSize + PcdGet32 (PcdExtFpdtBootRecordPadSize); + + // + // Try to allocate the same runtime buffer as last time boot. +@@ -358,9 +388,6 @@ AllocateBootPerformanceTable ( + DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable)); + + if (mAcpiBootPerformanceTable == NULL) { +- if (SmmCommData != NULL && SmmBootRecordData != NULL) { +- FreePool (SmmBootRecordData); +- } + return EFI_OUT_OF_RESOURCES; + } + +@@ -385,19 +412,10 @@ AllocateBootPerformanceTable ( + mPerformanceLength = 0; + mMaxPerformanceLength = 0; + } +- if (SmmCommData != NULL && SmmBootRecordData != NULL) { +- // +- // Fill Boot records from SMM drivers. +- // +- CopyMem (BootPerformanceData, SmmBootRecordData, SmmBootRecordDataSize); +- FreePool (SmmBootRecordData); +- mAcpiBootPerformanceTable->Header.Length = (UINT32) (mAcpiBootPerformanceTable->Header.Length + SmmBootRecordDataSize); +- BootPerformanceData = BootPerformanceData + SmmBootRecordDataSize; +- } + + mBootRecordBuffer = (UINT8 *) mAcpiBootPerformanceTable; + mBootRecordSize = mAcpiBootPerformanceTable->Header.Length; +- mBootRecordMaxSize = mBootRecordSize + PcdGet32 (PcdExtFpdtBootRecordPadSize); ++ mBootRecordMaxSize = BootPerformanceDataSize; + + return EFI_SUCCESS; + } +@@ -1336,6 +1354,47 @@ ReportFpdtRecordBuffer ( + } + } + ++/** ++ Update Boot Performance table. ++ ++ @param Event The event of notify protocol. ++ @param Context Notify event context. ++ ++**/ ++VOID ++EFIAPI ++UpdateBootPerformanceTable ( ++ IN EFI_EVENT Event, ++ IN VOID *Context ++ ) ++{ ++ VOID *SmmBootRecordData; ++ UINTN SmmBootRecordDataSize; ++ UINTN AppendSize; ++ UINT8 *FirmwarePerformanceTablePtr; ++ ++ // ++ // Get SMM performance data. ++ // ++ SmmBootRecordData = NULL; ++ InternalGetSmmPerfData (&SmmBootRecordData, &SmmBootRecordDataSize, FALSE); ++ ++ FirmwarePerformanceTablePtr = (UINT8 *) mAcpiBootPerformanceTable + mAcpiBootPerformanceTable->Header.Length; ++ ++ if (mAcpiBootPerformanceTable->Header.Length + SmmBootRecordDataSize > mBootRecordMaxSize) { ++ DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: No enough space to save all SMM boot performance data\n")); ++ AppendSize = mBootRecordMaxSize - mAcpiBootPerformanceTable->Header.Length; ++ } else { ++ AppendSize = SmmBootRecordDataSize; ++ } ++ if (SmmBootRecordData != NULL) { ++ CopyMem (FirmwarePerformanceTablePtr, SmmBootRecordData, AppendSize); ++ mAcpiBootPerformanceTable->Header.Length += (UINT32) AppendSize; ++ mBootRecordSize += (UINT32) AppendSize; ++ FreePool (SmmBootRecordData); ++ } ++} ++ + /** + The constructor function initializes Performance infrastructure for DXE phase. + +@@ -1358,6 +1417,7 @@ DxeCorePerformanceLibConstructor ( + { + EFI_STATUS Status; + EFI_HANDLE Handle; ++ EFI_EVENT EndOfDxeEvent; + EFI_EVENT ReadyToBootEvent; + PERFORMANCE_PROPERTY *PerformanceProperty; + +@@ -1386,13 +1446,25 @@ DxeCorePerformanceLibConstructor ( + ASSERT_EFI_ERROR (Status); + + // +- // Register ReadyToBoot event to report StatusCode data ++ // Register EndOfDxe event to allocate the boot performance table and report the table address through status code. + // + Status = gBS->CreateEventEx ( + EVT_NOTIFY_SIGNAL, +- TPL_CALLBACK, ++ TPL_NOTIFY, + ReportFpdtRecordBuffer, + NULL, ++ &gEfiEndOfDxeEventGroupGuid, ++ &EndOfDxeEvent ++ ); ++ ++ // ++ // Register ReadyToBoot event to update the boot performance table for SMM performance data. ++ // ++ Status = gBS->CreateEventEx ( ++ EVT_NOTIFY_SIGNAL, ++ TPL_CALLBACK, ++ UpdateBootPerformanceTable, ++ NULL, + &gEfiEventReadyToBootGuid, + &ReadyToBootEvent + ); +diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf +index 1c1dcc60a6..599d4dea66 100644 +--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf ++++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf +@@ -9,7 +9,7 @@ + # This library is mainly used by DxeCore to start performance logging to ensure that + # Performance and PerformanceEx Protocol are installed at the very beginning of DXE phase. + # +-# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
++# Copyright (c) 2006 - 2021, Intel Corporation. All rights reserved.
+ # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
+ # SPDX-License-Identifier: BSD-2-Clause-Patent + # +@@ -67,6 +67,7 @@ + gZeroGuid ## SOMETIMES_CONSUMES ## GUID + gEfiFirmwarePerformanceGuid ## SOMETIMES_PRODUCES ## UNDEFINED # StatusCode Data + gEdkiiFpdtExtendedFirmwarePerformanceGuid ## SOMETIMES_CONSUMES ## HOB # StatusCode Data ++ gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event + gEfiEventReadyToBootGuid ## CONSUMES ## Event + gEdkiiPiSmmCommunicationRegionTableGuid ## SOMETIMES_CONSUMES ## SystemTable + gEdkiiPerformanceMeasurementProtocolGuid ## PRODUCES ## UNDEFINED # Install protocol +diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c +index b4f22c14ae..d80f37e520 100644 +--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c ++++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.c +@@ -16,7 +16,7 @@ + + SmmPerformanceHandlerEx(), SmmPerformanceHandler() will receive untrusted input and do basic validation. + +-Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
++Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent + + **/ +@@ -48,6 +48,7 @@ CHAR8 *mPlatformLanguage = NULL; + SPIN_LOCK mSmmFpdtLock; + PERFORMANCE_PROPERTY mPerformanceProperty; + UINT32 mCachedLength = 0; ++UINT32 mBootRecordSize = 0; + + // + // Interfaces for SMM PerformanceMeasurement Protocol. +@@ -776,41 +777,116 @@ InsertFpdtRecord ( + } + + /** +- SmmReadyToBoot protocol notification event handler. ++ Communication service SMI Handler entry. + +- @param Protocol Points to the protocol's unique identifier +- @param Interface Points to the interface instance +- @param Handle The handle on which the interface was installed ++ This SMI handler provides services for report MM boot records. + +- @retval EFI_SUCCESS SmmReadyToBootCallback runs successfully ++ Caution: This function may receive untrusted input. ++ Communicate buffer and buffer size are external input, so this function will do basic validation. ++ ++ @param[in] DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister(). ++ @param[in] RegisterContext Points to an optional handler context which was specified when the ++ handler was registered. ++ @param[in, out] CommBuffer A pointer to a collection of data in memory that will ++ be conveyed from a non-MM environment into an MM environment. ++ @param[in, out] CommBufferSize The size of the CommBuffer. ++ ++ @retval EFI_SUCCESS The interrupt was handled and quiesced. No other handlers ++ should still be called. ++ @retval EFI_WARN_INTERRUPT_SOURCE_QUIESCED The interrupt has been quiesced but other handlers should ++ still be called. ++ @retval EFI_WARN_INTERRUPT_SOURCE_PENDING The interrupt is still pending and other handlers should still ++ be called. ++ @retval EFI_INTERRUPT_PENDING The interrupt could not be quiesced. + + **/ + EFI_STATUS + EFIAPI +-SmmReportFpdtRecordData ( +- IN CONST EFI_GUID *Protocol, +- IN VOID *Interface, +- IN EFI_HANDLE Handle ++FpdtSmiHandler ( ++ IN EFI_HANDLE DispatchHandle, ++ IN CONST VOID *RegisterContext, ++ IN OUT VOID *CommBuffer, ++ IN OUT UINTN *CommBufferSize + ) + { +- UINT64 SmmBPDTddr; +- +- if (!mFpdtDataIsReported && mSmmBootPerformanceTable != NULL) { +- SmmBPDTddr = (UINT64)(UINTN)mSmmBootPerformanceTable; +- REPORT_STATUS_CODE_EX ( +- EFI_PROGRESS_CODE, +- EFI_SOFTWARE_SMM_DRIVER, +- 0, +- NULL, +- &gEdkiiFpdtExtendedFirmwarePerformanceGuid, +- &SmmBPDTddr, +- sizeof (UINT64) ++ EFI_STATUS Status; ++ SMM_BOOT_RECORD_COMMUNICATE *SmmCommData; ++ UINTN BootRecordOffset; ++ UINTN BootRecordSize; ++ VOID *BootRecordData; ++ UINTN TempCommBufferSize; ++ UINT8 *BootRecordBuffer; ++ ++ // ++ // If input is invalid, stop processing this SMI ++ // ++ if (CommBuffer == NULL || CommBufferSize == NULL) { ++ return EFI_SUCCESS; ++ } ++ ++ TempCommBufferSize = *CommBufferSize; ++ ++ if(TempCommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) { ++ return EFI_SUCCESS; ++ } ++ ++ if (!SmmIsBufferOutsideSmmValid ((UINTN)CommBuffer, TempCommBufferSize)) { ++ DEBUG ((DEBUG_ERROR, "FpdtSmiHandler: MM communication data buffer in MMRAM or overflow!\n")); ++ return EFI_SUCCESS; ++ } ++ ++ SmmCommData = (SMM_BOOT_RECORD_COMMUNICATE*)CommBuffer; ++ ++ Status = EFI_SUCCESS; ++ ++ switch (SmmCommData->Function) { ++ case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_SIZE : ++ if (mSmmBootPerformanceTable != NULL) { ++ mBootRecordSize = mSmmBootPerformanceTable->Header.Length - sizeof (SMM_BOOT_PERFORMANCE_TABLE); ++ } ++ SmmCommData->BootRecordSize = mBootRecordSize; ++ break; ++ ++ case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA : ++ Status = EFI_UNSUPPORTED; ++ break; ++ ++ case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA_BY_OFFSET : ++ BootRecordOffset = SmmCommData->BootRecordOffset; ++ BootRecordData = SmmCommData->BootRecordData; ++ BootRecordSize = SmmCommData->BootRecordSize; ++ if (BootRecordData == NULL || BootRecordOffset >= mBootRecordSize) { ++ Status = EFI_INVALID_PARAMETER; ++ break; ++ } ++ ++ // ++ // Sanity check ++ // ++ if (BootRecordSize > mBootRecordSize - BootRecordOffset) { ++ BootRecordSize = mBootRecordSize - BootRecordOffset; ++ } ++ SmmCommData->BootRecordSize = BootRecordSize; ++ if (!SmmIsBufferOutsideSmmValid ((UINTN)BootRecordData, BootRecordSize)) { ++ DEBUG ((DEBUG_ERROR, "FpdtSmiHandler: MM Data buffer in MMRAM or overflow!\n")); ++ Status = EFI_ACCESS_DENIED; ++ break; ++ } ++ BootRecordBuffer = ((UINT8 *) (mSmmBootPerformanceTable)) + sizeof (SMM_BOOT_PERFORMANCE_TABLE); ++ CopyMem ( ++ (UINT8*)BootRecordData, ++ BootRecordBuffer + BootRecordOffset, ++ BootRecordSize + ); +- // +- // Set FPDT report state to TRUE. +- // +- mFpdtDataIsReported = TRUE; ++ mFpdtDataIsReported = TRUE; ++ break; ++ ++ default: ++ Status = EFI_UNSUPPORTED; + } ++ ++ SmmCommData->ReturnStatus = Status; ++ + return EFI_SUCCESS; + } + +@@ -830,8 +906,8 @@ InitializeSmmCorePerformanceLib ( + ) + { + EFI_HANDLE Handle; ++ EFI_HANDLE SmiHandle; + EFI_STATUS Status; +- VOID *SmmReadyToBootRegistration; + PERFORMANCE_PROPERTY *PerformanceProperty; + + // +@@ -851,11 +927,13 @@ InitializeSmmCorePerformanceLib ( + ); + ASSERT_EFI_ERROR (Status); + +- Status = gSmst->SmmRegisterProtocolNotify ( +- &gEdkiiSmmReadyToBootProtocolGuid, +- SmmReportFpdtRecordData, +- &SmmReadyToBootRegistration +- ); ++ // ++ // Register SMI handler. ++ // ++ SmiHandle = NULL; ++ Status = gSmst->SmiHandlerRegister (FpdtSmiHandler, &gEfiFirmwarePerformanceGuid, &SmiHandle); ++ ASSERT_EFI_ERROR (Status); ++ + Status = EfiGetSystemConfigurationTable (&gPerformanceProtocolGuid, (VOID **) &PerformanceProperty); + if (EFI_ERROR (Status)) { + // +diff --git a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf +index 6b013b8557..9eecc4b58c 100644 +--- a/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf ++++ b/MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLib.inf +@@ -8,7 +8,7 @@ + # This library is mainly used by SMM Core to start performance logging to ensure that + # SMM Performance and PerformanceEx Protocol are installed at the very beginning of SMM phase. + # +-# Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
++# Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.
+ # SPDX-License-Identifier: BSD-2-Clause-Patent + # + ## +@@ -58,14 +58,13 @@ + + [Protocols] + gEfiSmmBase2ProtocolGuid ## CONSUMES +- gEdkiiSmmReadyToBootProtocolGuid ## NOTIFY + + [Guids] + ## PRODUCES ## SystemTable + gPerformanceProtocolGuid +- gEdkiiFpdtExtendedFirmwarePerformanceGuid ## SOMETIMES_PRODUCES ## UNDEFINED # StatusCode Data + gZeroGuid ## SOMETIMES_CONSUMES ## GUID + gEdkiiSmmPerformanceMeasurementProtocolGuid ## PRODUCES ## UNDEFINED # Install protocol ++ gEfiFirmwarePerformanceGuid ## SOMETIMES_PRODUCES ## UNDEFINED # SmiHandlerRegister + + [Pcd] + gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask ## CONSUMES +diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec +index 5d9e2b8d3d..b139f1668c 100644 +--- a/MdeModulePkg/MdeModulePkg.dec ++++ b/MdeModulePkg/MdeModulePkg.dec +@@ -1822,9 +1822,9 @@ + gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosEntryPointProvideMethod|0x3|UINT32|0x00010069 + + ## This PCD specifies the additional pad size in FPDT Basic Boot Performance Table for +- # the extension FPDT boot records received after ReadyToBoot and before ExitBootService. ++ # the extension FPDT boot records received after EndOfDxe and before ExitBootService. + # @Prompt Pad size for extension FPDT boot records. +- gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x20000|UINT32|0x0001005F ++ gEfiMdeModulePkgTokenSpaceGuid.PcdExtFpdtBootRecordPadSize|0x30000|UINT32|0x0001005F + + ## Indicates if ConIn device are connected on demand.

+ # TRUE - ConIn device are not connected during BDS and ReadKeyStroke/ReadKeyStrokeEx produced +diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c +index 61a7704b37..68755554ad 100644 +--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c ++++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c +@@ -5,7 +5,7 @@ + for Firmware Basic Boot Performance Record and other boot performance records, + and install FPDT to ACPI table. + +- Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.
++ Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent + + **/ +@@ -16,6 +16,7 @@ + #include + #include + #include ++#include + + #include + #include +@@ -32,6 +33,8 @@ + #include + #include + #include ++#include ++#include + + #define SMM_BOOT_RECORD_COMM_SIZE (OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + sizeof(SMM_BOOT_RECORD_COMMUNICATE)) + +@@ -278,11 +281,12 @@ InstallFirmwarePerformanceDataTable ( + VOID + ) + { +- EFI_STATUS Status; +- EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol; +- UINTN BootPerformanceDataSize; +- FIRMWARE_PERFORMANCE_VARIABLE PerformanceVariable; +- UINTN Size; ++ EFI_STATUS Status; ++ EFI_ACPI_TABLE_PROTOCOL *AcpiTableProtocol; ++ UINTN BootPerformanceDataSize; ++ FIRMWARE_PERFORMANCE_VARIABLE PerformanceVariable; ++ UINTN Size; ++ EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicyProtocol; + + // + // Get AcpiTable Protocol. +@@ -292,6 +296,14 @@ InstallFirmwarePerformanceDataTable ( + return Status; + } + ++ // ++ // Get VariablePolicy Protocol. ++ // ++ Status = gBS->LocateProtocol(&gEdkiiVariablePolicyProtocolGuid, NULL, (VOID **)&VariablePolicyProtocol); ++ if (EFI_ERROR (Status)) { ++ return Status; ++ } ++ + if (mReceivedAcpiBootPerformanceTable != NULL) { + mAcpiBootPerformanceTable = mReceivedAcpiBootPerformanceTable; + mAcpiBootPerformanceTable->BasicBoot.ResetEnd = mBootPerformanceTableTemplate.BasicBoot.ResetEnd; +@@ -369,6 +381,24 @@ InstallFirmwarePerformanceDataTable ( + &PerformanceVariable + ); + ++ // ++ // Lock the variable which stores the Performance Table pointers. ++ // ++ Status = RegisterBasicVariablePolicy ( ++ VariablePolicyProtocol, ++ &gEfiFirmwarePerformanceGuid, ++ EFI_FIRMWARE_PERFORMANCE_VARIABLE_NAME, ++ VARIABLE_POLICY_NO_MIN_SIZE, ++ VARIABLE_POLICY_NO_MAX_SIZE, ++ VARIABLE_POLICY_NO_MUST_ATTR, ++ VARIABLE_POLICY_NO_CANT_ATTR, ++ VARIABLE_POLICY_TYPE_LOCK_NOW ++ ); ++ if (EFI_ERROR(Status)) { ++ DEBUG((DEBUG_ERROR, "[FirmwarePerformanceDxe] Error when lock variable %s, Status = %r\n", EFI_FIRMWARE_PERFORMANCE_VARIABLE_NAME, Status)); ++ ASSERT_EFI_ERROR(Status); ++ } ++ + // + // Publish Firmware Performance Data Table. + // +@@ -501,18 +531,12 @@ FpdtStatusCodeListenerDxe ( + DEBUG ((EFI_D_INFO, "FPDT: Boot Performance - OsLoaderStartImageStart = %ld\n", mAcpiBootPerformanceTable->BasicBoot.OsLoaderStartImageStart)); + DEBUG ((EFI_D_INFO, "FPDT: Boot Performance - ExitBootServicesEntry = 0\n")); + DEBUG ((EFI_D_INFO, "FPDT: Boot Performance - ExitBootServicesExit = 0\n")); +- } else if (Value == (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT)) { +- if (mAcpiBootPerformanceTable == NULL) { +- // +- // ACPI Firmware Performance Data Table not installed yet, install it now. +- // +- InstallFirmwarePerformanceDataTable (); +- } + } else if (Data != NULL && CompareGuid (&Data->Type, &gEdkiiFpdtExtendedFirmwarePerformanceGuid)) { + // + // Get the Boot performance table and then install it to ACPI table. + // + CopyMem (&mReceivedAcpiBootPerformanceTable, Data + 1, Data->Size); ++ InstallFirmwarePerformanceDataTable (); + } else if (Data != NULL && CompareGuid (&Data->Type, &gEfiFirmwarePerformanceGuid)) { + DEBUG ((DEBUG_ERROR, "FpdtStatusCodeListenerDxe: Performance data reported through gEfiFirmwarePerformanceGuid will not be collected by FirmwarePerformanceDataTableDxe\n")); + Status = EFI_UNSUPPORTED; +@@ -526,6 +550,32 @@ FpdtStatusCodeListenerDxe ( + return Status; + } + ++/** ++ Notify function for event EndOfDxe. ++ ++ This is used to install ACPI Firmware Performance Data Table for basic boot records. ++ ++ @param[in] Event The Event that is being processed. ++ @param[in] Context The Event Context. ++ ++**/ ++VOID ++EFIAPI ++FpdtEndOfDxeEventNotify ( ++ IN EFI_EVENT Event, ++ IN VOID *Context ++ ) ++{ ++ // ++ // When performance is enabled, the FPDT will be installed when DxeCorePerformanceLib report the data to FimwarePerformanceDxe. ++ // This is used to install the FPDT for the basic boot recods when performance infrastructure is not enabled. ++ // ++ if ((PcdGet8(PcdPerformanceLibraryPropertyMask) & PERFORMANCE_LIBRARY_PROPERTY_MEASUREMENT_ENABLED) != 0) { ++ return; ++ } ++ ASSERT (mReceivedAcpiBootPerformanceTable == NULL); ++ InstallFirmwarePerformanceDataTable (); ++} + + /** + Notify function for event EVT_SIGNAL_EXIT_BOOT_SERVICES. This is used to record +@@ -596,6 +646,7 @@ FirmwarePerformanceDxeEntryPoint ( + FIRMWARE_SEC_PERFORMANCE *Performance; + VOID *Registration; + UINT64 OemTableId; ++ EFI_EVENT EndOfDxeEvent; + + CopyMem ( + mFirmwarePerformanceTableTemplate.Header.OemId, +@@ -620,6 +671,19 @@ FirmwarePerformanceDxeEntryPoint ( + Status = mRscHandlerProtocol->Register (FpdtStatusCodeListenerDxe, TPL_HIGH_LEVEL); + ASSERT_EFI_ERROR (Status); + ++ // ++ // Register the notify function to install FPDT at EndOfDxe. ++ // ++ Status = gBS->CreateEventEx ( ++ EVT_NOTIFY_SIGNAL, ++ TPL_NOTIFY, ++ FpdtEndOfDxeEventNotify, ++ NULL, ++ &gEfiEndOfDxeEventGroupGuid, ++ &EndOfDxeEvent ++ ); ++ ASSERT_EFI_ERROR (Status); ++ + // + // Register the notify function to update FPDT on ExitBootServices Event. + // +diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf +index 1debb0193e..0411a22e66 100644 +--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf ++++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf +@@ -5,7 +5,7 @@ + # for Firmware Basic Boot Performance Record and other boot performance records, + # and install FPDT to ACPI table. + # +-# Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
++# Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.
+ # SPDX-License-Identifier: BSD-2-Clause-Patent + # + ## +@@ -46,12 +46,14 @@ + HobLib + LockBoxLib + UefiLib ++ VariablePolicyHelperLib + + [Protocols] + gEfiAcpiTableProtocolGuid ## CONSUMES + gEfiRscHandlerProtocolGuid ## CONSUMES + gEfiVariableArchProtocolGuid ## CONSUMES + gEfiLockBoxProtocolGuid ## CONSUMES ++ gEdkiiVariablePolicyProtocolGuid ## CONSUMES + + [Guids] + gEfiEventExitBootServicesGuid ## CONSUMES ## Event +@@ -63,6 +65,7 @@ + gEfiFirmwarePerformanceGuid + gEdkiiFpdtExtendedFirmwarePerformanceGuid ## SOMETIMES_CONSUMES ## UNDEFINED # StatusCode Data + gFirmwarePerformanceS3PointerGuid ## PRODUCES ## UNDEFINED # SaveLockBox ++ gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event + + [Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeOsLoaderLoad ## CONSUMES +@@ -72,6 +75,7 @@ + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision ## CONSUMES ++ gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask ## CONSUMES + + [FeaturePcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwarePerformanceDataTableS3Support ## CONSUMES +diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c +index d6c6e7693e..dbd9fe1842 100644 +--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c ++++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.c +@@ -11,7 +11,7 @@ + + FpdtSmiHandler() will receive untrusted input and do basic validation. + +- Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
++ Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent + + **/ +@@ -29,21 +29,12 @@ + #include + #include + #include +-#include + #include +-#include + #include + +-SMM_BOOT_PERFORMANCE_TABLE *mSmmBootPerformanceTable = NULL; +- + EFI_SMM_RSC_HANDLER_PROTOCOL *mRscHandlerProtocol = NULL; + UINT64 mSuspendStartTime = 0; + BOOLEAN mS3SuspendLockBoxSaved = FALSE; +-UINT32 mBootRecordSize = 0; +-UINT8 *mBootRecordBuffer = NULL; +- +-SPIN_LOCK mSmmFpdtLock; +-BOOLEAN mSmramIsOutOfResource = FALSE; + + /** + Report status code listener for SMM. This is used to record the performance +@@ -85,21 +76,6 @@ FpdtStatusCodeListenerSmm ( + return EFI_UNSUPPORTED; + } + +- // +- // Collect one or more Boot records in boot time +- // +- if (Data != NULL && CompareGuid (&Data->Type, &gEdkiiFpdtExtendedFirmwarePerformanceGuid)) { +- AcquireSpinLock (&mSmmFpdtLock); +- // +- // Get the boot performance data. +- // +- CopyMem (&mSmmBootPerformanceTable, Data + 1, Data->Size); +- mBootRecordBuffer = ((UINT8 *) (mSmmBootPerformanceTable)) + sizeof (SMM_BOOT_PERFORMANCE_TABLE); +- +- ReleaseSpinLock (&mSmmFpdtLock); +- return EFI_SUCCESS; +- } +- + if (Data != NULL && CompareGuid (&Data->Type, &gEfiFirmwarePerformanceGuid)) { + DEBUG ((DEBUG_ERROR, "FpdtStatusCodeListenerSmm: Performance data reported through gEfiFirmwarePerformanceGuid will not be collected by FirmwarePerformanceDataTableSmm\n")); + return EFI_UNSUPPORTED; +@@ -154,118 +130,6 @@ FpdtStatusCodeListenerSmm ( + return EFI_SUCCESS; + } + +-/** +- Communication service SMI Handler entry. +- +- This SMI handler provides services for report SMM boot records. +- +- Caution: This function may receive untrusted input. +- Communicate buffer and buffer size are external input, so this function will do basic validation. +- +- @param[in] DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister(). +- @param[in] RegisterContext Points to an optional handler context which was specified when the +- handler was registered. +- @param[in, out] CommBuffer A pointer to a collection of data in memory that will +- be conveyed from a non-SMM environment into an SMM environment. +- @param[in, out] CommBufferSize The size of the CommBuffer. +- +- @retval EFI_SUCCESS The interrupt was handled and quiesced. No other handlers +- should still be called. +- @retval EFI_WARN_INTERRUPT_SOURCE_QUIESCED The interrupt has been quiesced but other handlers should +- still be called. +- @retval EFI_WARN_INTERRUPT_SOURCE_PENDING The interrupt is still pending and other handlers should still +- be called. +- @retval EFI_INTERRUPT_PENDING The interrupt could not be quiesced. +- +-**/ +-EFI_STATUS +-EFIAPI +-FpdtSmiHandler ( +- IN EFI_HANDLE DispatchHandle, +- IN CONST VOID *RegisterContext, +- IN OUT VOID *CommBuffer, +- IN OUT UINTN *CommBufferSize +- ) +-{ +- EFI_STATUS Status; +- SMM_BOOT_RECORD_COMMUNICATE *SmmCommData; +- UINTN BootRecordOffset; +- UINTN BootRecordSize; +- VOID *BootRecordData; +- UINTN TempCommBufferSize; +- +- // +- // If input is invalid, stop processing this SMI +- // +- if (CommBuffer == NULL || CommBufferSize == NULL) { +- return EFI_SUCCESS; +- } +- +- TempCommBufferSize = *CommBufferSize; +- +- if(TempCommBufferSize < sizeof (SMM_BOOT_RECORD_COMMUNICATE)) { +- return EFI_SUCCESS; +- } +- +- if (!SmmIsBufferOutsideSmmValid ((UINTN)CommBuffer, TempCommBufferSize)) { +- DEBUG ((EFI_D_ERROR, "FpdtSmiHandler: SMM communication data buffer in SMRAM or overflow!\n")); +- return EFI_SUCCESS; +- } +- +- SmmCommData = (SMM_BOOT_RECORD_COMMUNICATE*)CommBuffer; +- +- Status = EFI_SUCCESS; +- +- switch (SmmCommData->Function) { +- case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_SIZE : +- if (mSmmBootPerformanceTable != NULL) { +- mBootRecordSize = mSmmBootPerformanceTable->Header.Length - sizeof (SMM_BOOT_PERFORMANCE_TABLE); +- } +- SmmCommData->BootRecordSize = mBootRecordSize; +- break; +- +- case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA : +- Status = EFI_UNSUPPORTED; +- break; +- +- case SMM_FPDT_FUNCTION_GET_BOOT_RECORD_DATA_BY_OFFSET : +- BootRecordOffset = SmmCommData->BootRecordOffset; +- BootRecordData = SmmCommData->BootRecordData; +- BootRecordSize = SmmCommData->BootRecordSize; +- if (BootRecordData == NULL || BootRecordOffset >= mBootRecordSize) { +- Status = EFI_INVALID_PARAMETER; +- break; +- } +- +- // +- // Sanity check +- // +- if (BootRecordSize > mBootRecordSize - BootRecordOffset) { +- BootRecordSize = mBootRecordSize - BootRecordOffset; +- } +- SmmCommData->BootRecordSize = BootRecordSize; +- if (!SmmIsBufferOutsideSmmValid ((UINTN)BootRecordData, BootRecordSize)) { +- DEBUG ((EFI_D_ERROR, "FpdtSmiHandler: SMM Data buffer in SMRAM or overflow!\n")); +- Status = EFI_ACCESS_DENIED; +- break; +- } +- +- CopyMem ( +- (UINT8*)BootRecordData, +- mBootRecordBuffer + BootRecordOffset, +- BootRecordSize +- ); +- break; +- +- default: +- Status = EFI_UNSUPPORTED; +- } +- +- SmmCommData->ReturnStatus = Status; +- +- return EFI_SUCCESS; +-} +- + /** + The module Entry Point of the Firmware Performance Data Table SMM driver. + +@@ -284,12 +148,6 @@ FirmwarePerformanceSmmEntryPoint ( + ) + { + EFI_STATUS Status; +- EFI_HANDLE Handle; +- +- // +- // Initialize spin lock +- // +- InitializeSpinLock (&mSmmFpdtLock); + + // + // Get SMM Report Status Code Handler Protocol. +@@ -307,12 +165,5 @@ FirmwarePerformanceSmmEntryPoint ( + Status = mRscHandlerProtocol->Register (FpdtStatusCodeListenerSmm); + ASSERT_EFI_ERROR (Status); + +- // +- // Register SMI handler. +- // +- Handle = NULL; +- Status = gSmst->SmiHandlerRegister (FpdtSmiHandler, &gEfiFirmwarePerformanceGuid, &Handle); +- ASSERT_EFI_ERROR (Status); +- + return Status; + } +diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf +index 618cbd56ca..6be57553f0 100644 +--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf ++++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableSmm/FirmwarePerformanceSmm.inf +@@ -4,7 +4,7 @@ + # This module registers report status code listener to collect performance data + # for SMM boot performance records and S3 Suspend Performance Record. + # +-# Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
++# Copyright (c) 2011 - 2021, Intel Corporation. All rights reserved.
+ # SPDX-License-Identifier: BSD-2-Clause-Patent + # + ## +@@ -51,10 +51,8 @@ + + [Guids] + ## SOMETIMES_PRODUCES ## UNDEFINED # SaveLockBox +- ## PRODUCES ## UNDEFINED # SmiHandlerRegister + ## SOMETIMES_CONSUMES ## UNDEFINED # StatusCode Data + gEfiFirmwarePerformanceGuid +- gEdkiiFpdtExtendedFirmwarePerformanceGuid ## SOMETIMES_PRODUCES ## UNDEFINED # StatusCode Data + + [Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdProgressCodeS3SuspendStart ## CONSUMES +-- +2.27.0 + diff --git a/edk2.spec b/edk2.spec index f126508..815c961 100644 --- a/edk2.spec +++ b/edk2.spec @@ -5,7 +5,7 @@ Name: edk2 Version: %{stable_date} -Release: 9 +Release: 10 Summary: EFI Development Kit II License: BSD-2-Clause-Patent URL: https://github.com/tianocore/edk2 @@ -39,6 +39,7 @@ Patch0024: 0024-NetworkPkg-IScsiDxe-reformat-IScsiHexToBin-leading-c.patch Patch0025: 0025-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-hex-parsing.patch Patch0026: 0026-NetworkPkg-IScsiDxe-fix-IScsiHexToBin-buffer-overflo.patch Patch0027: 0027-NetworkPkg-IScsiDxe-check-IScsiHexToBin-return-value.patch +Patch0028: 0028-MdeModulePkg-FPDT-Lock-boot-performance-table-addres.patch BuildRequires: acpica-tools gcc gcc-c++ libuuid-devel python3 bc nasm python3-unversioned-command @@ -236,6 +237,9 @@ chmod +x %{buildroot}%{_bindir}/Rsa2048Sha256GenerateKeys %endif %changelog +* Wed Dec 1 2021 Jinhua Cao -202002-10 +- fix CVE-2021-28216 + * Wed Sep 22 2021 imxcc - 202002-9 - fix cve-2021-38575 -- Gitee