[edk2-devel] [PATCH v2 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables

["Gerd Hoffmann" <kraxel@redhat.com>]

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>
---
 OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c | 82 +++++++++++++++++++++--
 1 file changed, 77 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
index 5ee98e9b595a..1bfb14495abd 100644
--- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
+++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
@@ -185,11 +185,16 @@ ValidateFvHeader (
   IN  NOR_FLASH_INSTANCE  *Instance
   )
 {
-  UINT16                      Checksum;
-  EFI_FIRMWARE_VOLUME_HEADER  *FwVolHeader;
-  VARIABLE_STORE_HEADER       *VariableStoreHeader;
-  UINTN                       VariableStoreLength;
-  UINTN                       FvLength;
+  UINT16                         Checksum;
+  EFI_FIRMWARE_VOLUME_HEADER     *FwVolHeader;
+  VARIABLE_STORE_HEADER          *VariableStoreHeader;
+  UINTN                          VarOffset;
+  AUTHENTICATED_VARIABLE_HEADER  *VarHeader;
+  UINTN                          VarSize;
+  CHAR16                         *VarName;
+  CHAR8                          *VarState;
+  UINTN                          VariableStoreLength;
+  UINTN                          FvLength;
 
   FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)Instance->RegionBaseAddress;
 
@@ -260,6 +265,73 @@ ValidateFvHeader (
     return EFI_NOT_FOUND;
   }
 
+  // check variables
+  DEBUG ((DEBUG_INFO, "%a: checking variables\n", __func__));
+  VarOffset = sizeof (*VariableStoreHeader);
+  while (VarOffset + sizeof (*VarHeader) < VariableStoreHeader->Size) {
+    VarHeader = (VOID *)((UINTN)VariableStoreHeader + VarOffset);
+    if (VarHeader->StartId != 0x55aa) {
+      DEBUG ((DEBUG_INFO, "%a: end of var list\n", __func__));
+      break;
+    }
+
+    VarSize = sizeof (*VarHeader) + VarHeader->NameSize + VarHeader->DataSize;
+    if (VarOffset + VarSize > VariableStoreHeader->Size) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: invalid variable size: 0x%x + 0x%x + 0x%x + 0x%x > 0x%x\n",
+        __func__,
+        VarOffset,
+        sizeof (*VarHeader),
+        VarHeader->NameSize,
+        VarHeader->DataSize,
+        VariableStoreHeader->Size
+        ));
+      return EFI_NOT_FOUND;
+    }
+
+    VarName = (VOID *)((UINTN)VariableStoreHeader + VarOffset
+                       + sizeof (*VarHeader));
+    switch (VarHeader->State) {
+      case VAR_HEADER_VALID_ONLY:
+        VarState = "header-ok";
+        VarName  = L"<unknown>";
+        break;
+      case VAR_ADDED:
+        VarState = "ok";
+        break;
+      case VAR_ADDED &VAR_IN_DELETED_TRANSITION:
+        VarState = "del-in-transition";
+        break;
+      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;
+    }
+
+    DEBUG ((
+      DEBUG_VERBOSE,
+      "%a: +0x%04x: name=0x%x data=0x%x '%s' (%a)\n",
+      __func__,
+      VarOffset,
+      VarHeader->NameSize,
+      VarHeader->DataSize,
+      VarName,
+      VarState
+      ));
+
+    VarSize    = (VarSize + 3) & ~3; // align
+    VarOffset += VarSize;
+  }
+
   return EFI_SUCCESS;
 }
 
-- 
2.43.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112177): https://edk2.groups.io/g/devel/message/112177
Mute This Topic: https://groups.io/mt/103031342/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-