public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v2 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables
@ 2023-12-07  9:44 Gerd Hoffmann
  2023-12-07 16:16 ` Ard Biesheuvel
  2023-12-11 23:25 ` Laszlo Ersek
  0 siblings, 2 replies; 7+ messages in thread
From: Gerd Hoffmann @ 2023-12-07  9:44 UTC (permalink / raw)
  To: devel; +Cc: mike.maslenkin, Jiewen Yao, Ard Biesheuvel, oliver, Gerd Hoffmann

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]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-12-14 16:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-07  9:44 [edk2-devel] [PATCH v2 1/1] OvmfPkg/VirtNorFlashDxe: sanity-check variables Gerd Hoffmann
2023-12-07 16:16 ` Ard Biesheuvel
2023-12-08 12:04   ` Gerd Hoffmann
2023-12-11 23:37     ` Laszlo Ersek
2023-12-11 23:25 ` Laszlo Ersek
2023-12-14 15:31   ` Gerd Hoffmann
2023-12-14 16:18     ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox