summaryrefslogtreecommitdiffstats
path: root/OvmfPkg
diff options
context:
space:
mode:
authorGerd Hoffmann <kraxel@redhat.com>2024-01-09 12:29:02 +0100
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2024-01-09 16:31:57 +0000
commit4a443f73fd67ca8caaf0a3e1a01f8231b330d2e0 (patch)
treeb6bf83b76b0bbde6a4d9a7a8f3f9453ddcba8d3a /OvmfPkg
parentae22b2f136bcbd27135a5f4dd76d3a68a172d00e (diff)
downloadedk2-4a443f73fd67ca8caaf0a3e1a01f8231b330d2e0.tar.gz
edk2-4a443f73fd67ca8caaf0a3e1a01f8231b330d2e0.tar.bz2
edk2-4a443f73fd67ca8caaf0a3e1a01f8231b330d2e0.zip
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]
Diffstat (limited to 'OvmfPkg')
-rw-r--r--OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf1
-rw-r--r--OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c149
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;
}