You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
211 lines
6.8 KiB
211 lines
6.8 KiB
From d557e973e4a400325f68014e463201a5b48c1547 Mon Sep 17 00:00:00 2001
|
|
From: Gerd Hoffmann <kraxel@redhat.com>
|
|
Date: Tue, 9 Jan 2024 12:29:02 +0100
|
|
Subject: [PATCH 3/3] OvmfPkg/VirtNorFlashDxe: sanity-check variables
|
|
|
|
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)
|
|
---
|
|
OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf | 1 +
|
|
OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c | 149 +++++++++++++++++++-
|
|
2 files changed, 145 insertions(+), 5 deletions(-)
|
|
|
|
diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
|
|
index 2a3d4a218e..f549400280 100644
|
|
--- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
|
|
+++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
|
|
@@ -34,6 +34,7 @@
|
|
DxeServicesTableLib
|
|
HobLib
|
|
IoLib
|
|
+ SafeIntLib
|
|
UefiBootServicesTableLib
|
|
UefiDriverEntryPoint
|
|
UefiLib
|
|
diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
|
|
index 9a614ae4b2..8fcd999ac6 100644
|
|
--- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
|
|
+++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
|
|
@@ -12,6 +12,7 @@
|
|
#include <Library/BaseMemoryLib.h>
|
|
#include <Library/MemoryAllocationLib.h>
|
|
#include <Library/PcdLib.h>
|
|
+#include <Library/SafeIntLib.h>
|
|
#include <Library/UefiLib.h>
|
|
|
|
#include <Guid/NvVarStoreFormatted.h>
|
|
@@ -185,11 +186,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;
|
|
|
|
@@ -258,6 +260,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.39.3
|
|
|