From ad8cc2ba61795b48177d7f9e97a402ae32b4e0bb Mon Sep 17 00:00:00 2001 From: Jon Maloy Date: Wed, 24 Jan 2024 10:36:38 -0500 Subject: [PATCH] * Wed Jan 24 2024 Jon Maloy - 20220126gitbb1bba3d77-9 - edk2-OvmfPkg-VirtNorFlashDxe-stop-accepting-gEfiVariableG.patch [RHEL-17587] - edk2-OvmfPkg-VirtNorFlashDxe-sanity-check-variables.patch [RHEL-17587] - Resolves: RHEL-17587 ([rhel8] guest fails to boot due to ASSERT error) --- ...rtNorFlashDxe-sanity-check-variables.patch | 216 ++++++++++++++++++ ...lashDxe-stop-accepting-gEfiVariableG.patch | 47 ++++ edk2.spec | 12 +- 3 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 edk2-OvmfPkg-VirtNorFlashDxe-sanity-check-variables.patch create mode 100644 edk2-OvmfPkg-VirtNorFlashDxe-stop-accepting-gEfiVariableG.patch diff --git a/edk2-OvmfPkg-VirtNorFlashDxe-sanity-check-variables.patch b/edk2-OvmfPkg-VirtNorFlashDxe-sanity-check-variables.patch new file mode 100644 index 0000000..f94ca10 --- /dev/null +++ b/edk2-OvmfPkg-VirtNorFlashDxe-sanity-check-variables.patch @@ -0,0 +1,216 @@ +From c4eef747624d41aaa09dc64ccafdb84bf1fe656e Mon Sep 17 00:00:00 2001 +From: Gerd Hoffmann +Date: Tue, 9 Jan 2024 12:29:02 +0100 +Subject: [PATCH 2/2] OvmfPkg/VirtNorFlashDxe: sanity-check variables + +RH-Author: Gerd Hoffmann +RH-MergeRequest: 42: OvmfPkg/VirtNorFlashDxe: sanity-check variables +RH-Jira: RHEL-17587 +RH-Acked-by: Laszlo Ersek +RH-Commit: [2/2] 56ff961f754d517c0e27ccf46a95b228efe7ab4b + +Extend the ValidateFvHeader function, additionally to the header checks +walk over the list of variables and sanity check them. + +In case we find inconsistencies indicating variable store corruption +return EFI_NOT_FOUND so the variable store will be re-initialized. + +Signed-off-by: Gerd Hoffmann +Message-Id: <20240109112902.30002-4-kraxel@redhat.com> +Reviewed-by: Laszlo Ersek +[lersek@redhat.com: fix StartId initialization/assignment coding style] +(cherry picked from commit 4a443f73fd67ca8caaf0a3e1a01f8231b330d2e0) +--- + .../Drivers/NorFlashDxe/NorFlashDxe.inf | 1 + + .../Drivers/NorFlashDxe/NorFlashFvb.c | 149 +++++++++++++++++- + 2 files changed, 145 insertions(+), 5 deletions(-) + +diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf +index f8d4c27031..10388880a1 100644 +--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf ++++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf +@@ -35,6 +35,7 @@ + DebugLib + HobLib + NorFlashPlatformLib ++ SafeIntLib + UefiLib + UefiDriverEntryPoint + UefiBootServicesTableLib +diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c +index 904605cbbc..2a166c94a6 100644 +--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c ++++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c +@@ -13,6 +13,7 @@ + #include + #include + #include ++#include + + #include + #include +@@ -166,11 +167,12 @@ ValidateFvHeader ( + IN NOR_FLASH_INSTANCE *Instance + ) + { +- UINT16 Checksum; +- EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader; +- VARIABLE_STORE_HEADER *VariableStoreHeader; +- UINTN VariableStoreLength; +- UINTN FvLength; ++ UINT16 Checksum; ++ CONST EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader; ++ CONST VARIABLE_STORE_HEADER *VariableStoreHeader; ++ UINTN VarOffset; ++ UINTN VariableStoreLength; ++ UINTN FvLength; + + FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER*)Instance->RegionBaseAddress; + +@@ -223,6 +225,143 @@ ValidateFvHeader ( + return EFI_NOT_FOUND; + } + ++ // ++ // check variables ++ // ++ DEBUG ((DEBUG_INFO, "%a: checking variables\n", __func__)); ++ VarOffset = sizeof (*VariableStoreHeader); ++ for ( ; ;) { ++ UINTN VarHeaderEnd; ++ UINTN VarNameEnd; ++ UINTN VarEnd; ++ UINTN VarPadding; ++ CONST AUTHENTICATED_VARIABLE_HEADER *VarHeader; ++ CONST CHAR16 *VarName; ++ CONST CHAR8 *VarState; ++ RETURN_STATUS Status; ++ ++ Status = SafeUintnAdd (VarOffset, sizeof (*VarHeader), &VarHeaderEnd); ++ if (RETURN_ERROR (Status)) { ++ DEBUG ((DEBUG_ERROR, "%a: integer overflow\n", __func__)); ++ return EFI_NOT_FOUND; ++ } ++ ++ if (VarHeaderEnd >= VariableStoreHeader->Size) { ++ if (VarOffset <= VariableStoreHeader->Size - sizeof (UINT16)) { ++ CONST UINT16 *StartId; ++ ++ StartId = (VOID *)((UINTN)VariableStoreHeader + VarOffset); ++ if (*StartId == 0x55aa) { ++ DEBUG ((DEBUG_ERROR, "%a: startid at invalid location\n", __func__)); ++ return EFI_NOT_FOUND; ++ } ++ } ++ ++ DEBUG ((DEBUG_INFO, "%a: end of var list (no space left)\n", __func__)); ++ break; ++ } ++ ++ VarHeader = (VOID *)((UINTN)VariableStoreHeader + VarOffset); ++ if (VarHeader->StartId != 0x55aa) { ++ DEBUG ((DEBUG_INFO, "%a: end of var list (no startid)\n", __func__)); ++ break; ++ } ++ ++ VarName = NULL; ++ switch (VarHeader->State) { ++ // usage: State = VAR_HEADER_VALID_ONLY ++ case VAR_HEADER_VALID_ONLY: ++ VarState = "header-ok"; ++ VarName = L""; ++ break; ++ ++ // usage: State = VAR_ADDED ++ case VAR_ADDED: ++ VarState = "ok"; ++ break; ++ ++ // usage: State &= VAR_IN_DELETED_TRANSITION ++ case VAR_ADDED &VAR_IN_DELETED_TRANSITION: ++ VarState = "del-in-transition"; ++ break; ++ ++ // usage: State &= VAR_DELETED ++ case VAR_ADDED &VAR_DELETED: ++ case VAR_ADDED &VAR_DELETED &VAR_IN_DELETED_TRANSITION: ++ VarState = "deleted"; ++ break; ++ ++ default: ++ DEBUG (( ++ DEBUG_ERROR, ++ "%a: invalid variable state: 0x%x\n", ++ __func__, ++ VarHeader->State ++ )); ++ return EFI_NOT_FOUND; ++ } ++ ++ Status = SafeUintnAdd (VarHeaderEnd, VarHeader->NameSize, &VarNameEnd); ++ if (RETURN_ERROR (Status)) { ++ DEBUG ((DEBUG_ERROR, "%a: integer overflow\n", __func__)); ++ return EFI_NOT_FOUND; ++ } ++ ++ Status = SafeUintnAdd (VarNameEnd, VarHeader->DataSize, &VarEnd); ++ if (RETURN_ERROR (Status)) { ++ DEBUG ((DEBUG_ERROR, "%a: integer overflow\n", __func__)); ++ return EFI_NOT_FOUND; ++ } ++ ++ if (VarEnd > VariableStoreHeader->Size) { ++ DEBUG (( ++ DEBUG_ERROR, ++ "%a: invalid variable size: 0x%Lx + 0x%Lx + 0x%x + 0x%x > 0x%x\n", ++ __func__, ++ (UINT64)VarOffset, ++ (UINT64)(sizeof (*VarHeader)), ++ VarHeader->NameSize, ++ VarHeader->DataSize, ++ VariableStoreHeader->Size ++ )); ++ return EFI_NOT_FOUND; ++ } ++ ++ if (((VarHeader->NameSize & 1) != 0) || ++ (VarHeader->NameSize < 4)) ++ { ++ DEBUG ((DEBUG_ERROR, "%a: invalid name size\n", __func__)); ++ return EFI_NOT_FOUND; ++ } ++ ++ if (VarName == NULL) { ++ VarName = (VOID *)((UINTN)VariableStoreHeader + VarHeaderEnd); ++ if (VarName[VarHeader->NameSize / 2 - 1] != L'\0') { ++ DEBUG ((DEBUG_ERROR, "%a: name is not null terminated\n", __func__)); ++ return EFI_NOT_FOUND; ++ } ++ } ++ ++ DEBUG (( ++ DEBUG_VERBOSE, ++ "%a: +0x%04Lx: name=0x%x data=0x%x guid=%g '%s' (%a)\n", ++ __func__, ++ (UINT64)VarOffset, ++ VarHeader->NameSize, ++ VarHeader->DataSize, ++ &VarHeader->VendorGuid, ++ VarName, ++ VarState ++ )); ++ ++ VarPadding = (4 - (VarEnd & 3)) & 3; ++ Status = SafeUintnAdd (VarEnd, VarPadding, &VarOffset); ++ if (RETURN_ERROR (Status)) { ++ DEBUG ((DEBUG_ERROR, "%a: integer overflow\n", __func__)); ++ return EFI_NOT_FOUND; ++ } ++ } ++ + return EFI_SUCCESS; + } + +-- +2.41.0 + diff --git a/edk2-OvmfPkg-VirtNorFlashDxe-stop-accepting-gEfiVariableG.patch b/edk2-OvmfPkg-VirtNorFlashDxe-stop-accepting-gEfiVariableG.patch new file mode 100644 index 0000000..a3b6933 --- /dev/null +++ b/edk2-OvmfPkg-VirtNorFlashDxe-stop-accepting-gEfiVariableG.patch @@ -0,0 +1,47 @@ +From abe5b633eaae333190fb742af3fa15968f02a92e Mon Sep 17 00:00:00 2001 +From: Gerd Hoffmann +Date: Tue, 9 Jan 2024 12:29:01 +0100 +Subject: [PATCH 1/2] OvmfPkg/VirtNorFlashDxe: stop accepting gEfiVariableGuid +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +RH-Author: Gerd Hoffmann +RH-MergeRequest: 42: OvmfPkg/VirtNorFlashDxe: sanity-check variables +RH-Jira: RHEL-17587 +RH-Acked-by: Laszlo Ersek +RH-Commit: [1/2] 790f895bd180bc2c4b957a7a3c7d07e9107dd74b + +Only accept gEfiAuthenticatedVariableGuid when checking the variable +store header in ValidateFvHeader(). + +The edk2 code base has been switched to use the authenticated varstore +format unconditionally (even in case secure boot is not used or +supported) a few years ago. + +Suggested-by: László Érsek +Signed-off-by: Gerd Hoffmann +Reviewed-by: Laszlo Ersek +Message-Id: <20240109112902.30002-3-kraxel@redhat.com> +(cherry picked from commit ae22b2f136bcbd27135a5f4dd76d3a68a172d00e) +--- + ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c | 3 +-- + 1 file changed, 1 insertion(+), 2 deletions(-) + +diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c +index db8eb595f4..904605cbbc 100644 +--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c ++++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c +@@ -210,8 +210,7 @@ ValidateFvHeader ( + VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)FwVolHeader + FwVolHeader->HeaderLength); + + // Check the Variable Store Guid +- if (!CompareGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid) && +- !CompareGuid (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid)) { ++ if (!CompareGuid (&VariableStoreHeader->Signature, &gEfiAuthenticatedVariableGuid)) { + DEBUG ((EFI_D_INFO, "%a: Variable Store Guid non-compatible\n", + __FUNCTION__)); + return EFI_NOT_FOUND; +-- +2.41.0 + diff --git a/edk2.spec b/edk2.spec index ddd23f6..e8ff8b9 100644 --- a/edk2.spec +++ b/edk2.spec @@ -7,7 +7,7 @@ ExclusiveArch: x86_64 aarch64 Name: edk2 Version: %{GITDATE}git%{GITCOMMIT} -Release: 8%{?dist} +Release: 9%{?dist} Summary: UEFI firmware for 64-bit virtual machines Group: Applications/Emulators License: BSD-2-Clause-Patent and OpenSSL and MIT @@ -60,6 +60,10 @@ Patch28: edk2-rh-openssl-add-crypto-bn-rsa_sup_mul.c-to-file-list.patch Patch29: edk2-SecurityPkg-DxeImageVerificationLib-Check-result-of-.patch # For bz#2150267 - ovmf must consider max cpu count not boot cpu count for apic mode [rhel-8] Patch30: edk2-UefiCpuPkg-MpInitLib-fix-apic-mode-for-cpu-hotplug.patch +# For RHEL-17587 - [rhel8] guest fails to boot due to ASSERT error +Patch31: edk2-OvmfPkg-VirtNorFlashDxe-stop-accepting-gEfiVariableG.patch +# For RHEL-17587 - [rhel8] guest fails to boot due to ASSERT error +Patch32: edk2-OvmfPkg-VirtNorFlashDxe-sanity-check-variables.patch # python3-devel and libuuid-devel are required for building tools. @@ -504,6 +508,12 @@ true %endif %changelog +* Wed Jan 24 2024 Jon Maloy - 20220126gitbb1bba3d77-9 +- edk2-OvmfPkg-VirtNorFlashDxe-stop-accepting-gEfiVariableG.patch [RHEL-17587] +- edk2-OvmfPkg-VirtNorFlashDxe-sanity-check-variables.patch [RHEL-17587] +- Resolves: RHEL-17587 + ([rhel8] guest fails to boot due to ASSERT error) + * Fri Jan 05 2024 Jon Maloy - 20220126gitbb1bba3d77-8 - edk2-Bumped-openssl-submodule-version-to-cf317b2bb227.patch [RHEL-7560] - Resolves: RHEL-7560