public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gerd Hoffmann" <kraxel@redhat.com>
To: devel@edk2.groups.io
Cc: Oliver Steffen <osteffen@redhat.com>,
	Konstantin Kostiuk <kkostiuk@redhat.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: [edk2-devel] [PATCH v2 3/5] OvmfPkg/VirtHstiDxe: add varstore flash check
Date: Fri, 19 Apr 2024 14:31:31 +0200	[thread overview]
Message-ID: <20240419123133.789238-4-kraxel@redhat.com> (raw)
In-Reply-To: <20240419123133.789238-1-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]
-=-=-=-=-=-=-=-=-=-=-=-



  parent reply	other threads:[~2024-04-19 12:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-19 12:31 [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add VirtHstiDxe driver Gerd Hoffmann
2024-04-19 12:31 ` [edk2-devel] [PATCH v2 1/5] " Gerd Hoffmann
2024-04-19 12:31 ` [edk2-devel] [PATCH v2 2/5] OvmfPkg: Add VirtHstiDxe to OVMF firmware build Gerd Hoffmann
2024-04-19 12:31 ` Gerd Hoffmann [this message]
2024-04-19 12:31 ` [edk2-devel] [PATCH v2 4/5] OvmfPkg/VirtHstiDxe: add code flash check Gerd Hoffmann
2024-04-19 12:31 ` [edk2-devel] [PATCH v2 5/5] OvmfPkg/VirtHstiDxe: add README.md Gerd Hoffmann
2024-04-21  2:46 ` [edk2-devel] [PATCH v2 0/5] OvmfPkg: Add VirtHstiDxe driver Yao, Jiewen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240419123133.789238-4-kraxel@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox