rpms/edk2
7
0
Fork 0
Code Issues Pull Requests Releases Activity

* Wed Jan 24 2024 Jon Maloy <jmaloy@redhat.com> - 20220126gitbb1bba3d77-9

Browse Source 
- 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)
This commit is contained in:
Jon Maloy 2024-01-24 10:36:38 -05:00
parent 4d77ac6798
commit ad8cc2ba61
3 changed files with 274 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

216
edk2-OvmfPkg-VirtNorFlashDxe-sanity-check-variables.patch Normal file
View File

@ -0,0 +1,216 @@
From c4eef747624d41aaa09dc64ccafdb84bf1fe656e Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 9 Jan 2024 12:29:02 +0100
Subject: [PATCH 2/2] OvmfPkg/VirtNorFlashDxe: sanity-check variables
RH-Author: Gerd Hoffmann <None>
RH-MergeRequest: 42: OvmfPkg/VirtNorFlashDxe: sanity-check variables
RH-Jira: RHEL-17587
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
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 <kraxel@redhat.com>
Message-Id: <20240109112902.30002-4-kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
[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 <Library/UefiLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/SafeIntLib.h>
#include <Guid/VariableFormat.h>
#include <Guid/SystemNvDataGuid.h>
@@ -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"<unknown>";
+ 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

47
edk2-OvmfPkg-VirtNorFlashDxe-stop-accepting-gEfiVariableG.patch Normal file
View File

@ -0,0 +1,47 @@
From abe5b633eaae333190fb742af3fa15968f02a92e Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
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 <None>
RH-MergeRequest: 42: OvmfPkg/VirtNorFlashDxe: sanity-check variables
RH-Jira: RHEL-17587
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
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 <lersek@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
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

12
edk2.spec
View File

@ -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 <jmaloy@redhat.com> - 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 <jmaloy@redhat.com> - 20220126gitbb1bba3d77-8
- edk2-Bumped-openssl-submodule-version-to-cf317b2bb227.patch [RHEL-7560]
- Resolves: RHEL-7560