From ddc43e7a41fac5b1dc93b1d0bb1e71319acfba4e Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 22 Apr 2024 12:47:27 +0200 Subject: OvmfPkg/VirtHstiDxe: add varstore flash check 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 Cc: Jiewen Yao Cc: Konstantin Kostiuk Signed-off-by: Gerd Hoffmann Reviewed-by: Jiewen Yao --- OvmfPkg/VirtHstiDxe/Flash.c | 90 +++++++++++++++++++++++++++++++++++++ OvmfPkg/VirtHstiDxe/QemuQ35.c | 13 ++++++ OvmfPkg/VirtHstiDxe/VirtHstiDxe.h | 16 ++++++- OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf | 4 ++ 4 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 OvmfPkg/VirtHstiDxe/Flash.c diff --git a/OvmfPkg/VirtHstiDxe/Flash.c b/OvmfPkg/VirtHstiDxe/Flash.c new file mode 100644 index 0000000..e933567 --- /dev/null +++ b/OvmfPkg/VirtHstiDxe/Flash.c @@ -0,0 +1,90 @@ +/** @file + +SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include +#include + +#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 5eab4aa..2dcfa52 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 (PcdOvmfFlashNvStorageVariableBase))) { + case QEMU_FIRMWARE_FLASH_WRITABLE: + ErrorMsg = L"qemu vars pflash is not secure"; + break; + } + + VirtHstiTestResult (ErrorMsg, 0, VIRT_HSTI_BYTE0_SMM_SECURE_VARS_FLASH); + } } diff --git a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.h index cf0d77f..ceff41c 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/VirtHstiDxe.inf b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf index 8c63ff6..b6bdd1f 100644 --- a/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf +++ b/OvmfPkg/VirtHstiDxe/VirtHstiDxe.inf @@ -22,6 +22,7 @@ VirtHstiDxe.c QemuPC.c QemuQ35.c + Flash.c [Packages] MdePkg/MdePkg.dec @@ -46,5 +47,8 @@ [FeaturePcd] gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire +[Pcd] + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase + [Depex] TRUE -- cgit v1.1