[edk2-devel] [PATCH v2 3/5] OvmfPkg/VirtHstiDxe: add varstore flash check

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

Detects qemu config issue: vars pflash is not in secure mode (write
access restricted to smm).  Applies to Q35 with SMM only.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf |  4 ++
 OvmfPkg/VirtHstiDxe/VirtHstiDxe.h   | 16 ++++-
 OvmfPkg/VirtHstiDxe/Flash.c         | 90 +++++++++++++++++++++++++++++
 OvmfPkg/VirtHstiDxe/QemuQ35.c       | 13 +++++
 4 files changed, 122 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/VirtHstiDxe/Flash.c

diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
index 8c63ff6a8953..9cb2ed1f0c64 100644
--- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf
@@ -22,6 +22,7 @@ [Sources]
   VirtHstiDxe.c
   QemuPC.c
   QemuQ35.c
+  Flash.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -46,5 +47,8 @@ [Guids]
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
 
+[Pcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress
+
 [Depex]
   TRUE
diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
index cf0d77fc3af9..ceff41c03711 100644
--- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
+++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h
@@ -6,7 +6,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #define VIRT_HSTI_SECURITY_FEATURE_SIZE  2
 
-#define VIRT_HSTI_BYTE0_SMM_SMRAM_LOCK  BIT0
+#define VIRT_HSTI_BYTE0_SMM_SMRAM_LOCK         BIT0
+#define VIRT_HSTI_BYTE0_SMM_SECURE_VARS_FLASH  BIT1
 
 typedef struct {
   // ADAPTER_INFO_PLATFORM_SECURITY
@@ -65,3 +66,16 @@ VOID
 VirtHstiQemuPCVerify (
   VOID
   );
+
+/* Flash.c */
+
+#define QEMU_FIRMWARE_FLASH_UNKNOWN    0
+#define QEMU_FIRMWARE_FLASH_IS_ROM     1
+#define QEMU_FIRMWARE_FLASH_IS_RAM     2
+#define QEMU_FIRMWARE_FLASH_READ_ONLY  3
+#define QEMU_FIRMWARE_FLASH_WRITABLE   4
+
+UINT32
+VirtHstiQemuFirmwareFlashCheck (
+  UINT32  Address
+  );
diff --git a/OvmfPkg/VirtHstiDxe/Flash.c b/OvmfPkg/VirtHstiDxe/Flash.c
new file mode 100644
index 000000000000..e93356793f8c
--- /dev/null
+++ b/OvmfPkg/VirtHstiDxe/Flash.c
@@ -0,0 +1,90 @@
+/** @file
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+
+#include "VirtHstiDxe.h"
+
+#define WRITE_BYTE_CMD           0x10
+#define BLOCK_ERASE_CMD          0x20
+#define CLEAR_STATUS_CMD         0x50
+#define READ_STATUS_CMD          0x70
+#define READ_DEVID_CMD           0x90
+#define BLOCK_ERASE_CONFIRM_CMD  0xd0
+#define READ_ARRAY_CMD           0xff
+#define CLEARED_ARRAY_STATUS     0x00
+
+/* based on QemuFlashDetected (QemuFlashFvbServicesRuntimeDxe) */
+UINT32
+VirtHstiQemuFirmwareFlashCheck (
+  UINT32  Address
+  )
+{
+  volatile UINT8  *Ptr;
+
+  UINTN  Offset;
+  UINT8  OriginalUint8;
+  UINT8  ProbeUint8;
+
+  for (Offset = 0; Offset < EFI_PAGE_SIZE; Offset++) {
+    Ptr        = (UINT8 *)(UINTN)(Address + Offset);
+    ProbeUint8 = *Ptr;
+    if ((ProbeUint8 != CLEAR_STATUS_CMD) &&
+        (ProbeUint8 != READ_STATUS_CMD) &&
+        (ProbeUint8 != CLEARED_ARRAY_STATUS))
+    {
+      break;
+    }
+  }
+
+  if (Offset >= EFI_PAGE_SIZE) {
+    DEBUG ((DEBUG_INFO, "%a: check failed\n", __func__));
+    return QEMU_FIRMWARE_FLASH_UNKNOWN;
+  }
+
+  OriginalUint8 = *Ptr;
+  *Ptr          = CLEAR_STATUS_CMD;
+  ProbeUint8    = *Ptr;
+  if ((OriginalUint8 != CLEAR_STATUS_CMD) &&
+      (ProbeUint8 == CLEAR_STATUS_CMD))
+  {
+    *Ptr = OriginalUint8;
+    DEBUG ((DEBUG_INFO, "%a: %p behaves as RAM\n", __func__, Ptr));
+    return QEMU_FIRMWARE_FLASH_IS_RAM;
+  }
+
+  *Ptr       = READ_STATUS_CMD;
+  ProbeUint8 = *Ptr;
+  if (ProbeUint8 == OriginalUint8) {
+    DEBUG ((DEBUG_INFO, "%a: %p behaves as ROM\n", __func__, Ptr));
+    return QEMU_FIRMWARE_FLASH_IS_ROM;
+  }
+
+  if (ProbeUint8 == READ_STATUS_CMD) {
+    *Ptr = OriginalUint8;
+    DEBUG ((DEBUG_INFO, "%a: %p behaves as RAM\n", __func__, Ptr));
+    return QEMU_FIRMWARE_FLASH_IS_RAM;
+  }
+
+  if (ProbeUint8 == CLEARED_ARRAY_STATUS) {
+    *Ptr       = WRITE_BYTE_CMD;
+    *Ptr       = OriginalUint8;
+    *Ptr       = READ_STATUS_CMD;
+    ProbeUint8 = *Ptr;
+    *Ptr       = READ_ARRAY_CMD;
+    if (ProbeUint8 & 0x10 /* programming error */) {
+      DEBUG ((DEBUG_INFO, "%a: %p behaves as FLASH, write-protected\n", __func__, Ptr));
+      return QEMU_FIRMWARE_FLASH_READ_ONLY;
+    } else {
+      DEBUG ((DEBUG_INFO, "%a: %p behaves as FLASH, writable\n", __func__, Ptr));
+      return QEMU_FIRMWARE_FLASH_WRITABLE;
+    }
+  }
+
+  DEBUG ((DEBUG_INFO, "%a: check failed\n", __func__));
+  return QEMU_FIRMWARE_FLASH_UNKNOWN;
+}
diff --git a/OvmfPkg/VirtHstiDxe/QemuQ35.c b/OvmfPkg/VirtHstiDxe/QemuQ35.c
index 5eab4aab29d1..33753027060b 100644
--- a/OvmfPkg/VirtHstiDxe/QemuQ35.c
+++ b/OvmfPkg/VirtHstiDxe/QemuQ35.c
@@ -29,6 +29,7 @@ VirtHstiQemuQ35Init (
 {
   if (FeaturePcdGet (PcdSmmSmramRequire)) {
     VirtHstiSetSupported (&mHstiQ35, 0, VIRT_HSTI_BYTE0_SMM_SMRAM_LOCK);
+    VirtHstiSetSupported (&mHstiQ35, 0, VIRT_HSTI_BYTE0_SMM_SECURE_VARS_FLASH);
   }
 
   return &mHstiQ35;
@@ -55,4 +56,16 @@ VirtHstiQemuQ35Verify (
 
     VirtHstiTestResult (ErrorMsg, 0, VIRT_HSTI_BYTE0_SMM_SMRAM_LOCK);
   }
+
+  if (VirtHstiIsSupported (&mHstiQ35, 0, VIRT_HSTI_BYTE0_SMM_SECURE_VARS_FLASH)) {
+    CHAR16  *ErrorMsg = NULL;
+
+    switch (VirtHstiQemuFirmwareFlashCheck (PcdGet32 (PcdOvmfFdBaseAddress))) {
+      case QEMU_FIRMWARE_FLASH_WRITABLE:
+        ErrorMsg = L"qemu vars pflash is not secure";
+        break;
+    }
+
+    VirtHstiTestResult (ErrorMsg, 0, VIRT_HSTI_BYTE0_SMM_SECURE_VARS_FLASH);
+  }
 }
-- 
2.44.0



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