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 <kra...@redhat.com>
---
OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf | 1 +
OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c | 147 +++++++++++++++++++-
2 files changed, 143 insertions(+), 5 deletions(-)
diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
index 2a3d4a218e61..f549400280a1 100644
--- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
+++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashDxe.inf
@@ -34,6 +34,7 @@ [LibraryClasses]
DxeServicesTableLib
HobLib
IoLib
+ SafeIntLib
UefiBootServicesTableLib
UefiDriverEntryPoint
UefiLib
diff --git a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
index 9a614ae4b24d..ca2e40743dfd 100644
--- a/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
+++ b/OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c
@@ -12,6 +12,7 @@
#include <Library/BaseMemoryLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/PcdLib.h>
+#include <Library/SafeIntLib.h>
#include <Library/UefiLib.h>
#include <Guid/NvVarStoreFormatted.h>
@@ -185,11 +186,12 @@ ValidateFvHeader (
IN NOR_FLASH_INSTANCE *Instance
)
{
- UINT16 Checksum;
- EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
- VARIABLE_STORE_HEADER *VariableStoreHeader;
- UINTN VariableStoreLength;
- UINTN FvLength;
+ UINT16 Checksum;
+ CONST EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
+ CONST VARIABLE_STORE_HEADER *VariableStoreHeader;
+ UINTN VarOffset;
+ UINTN VariableStoreLength;
+ UINTN FvLength;
FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)Instance->RegionBaseAddress;
@@ -258,6 +260,141 @@ ValidateFvHeader (
return EFI_NOT_FOUND;
}
+ //
+ // check variables
+ //
+ DEBUG ((DEBUG_INFO, "%a: checking variables\n", __func__));
+ VarOffset = sizeof (*VariableStoreHeader);
+ for ( ; ;) {
+ UINTN VarHeaderEnd;
+ UINTN VarNameEnd;
+ UINTN VarEnd;
+ UINTN VarPadding;
+ CONST AUTHENTICATED_VARIABLE_HEADER *VarHeader;
+ CONST CHAR16 *VarName;
+ CONST CHAR8 *VarState;
+ RETURN_STATUS Status;
+
+ Status = SafeUintnAdd (VarOffset, sizeof (*VarHeader), &VarHeaderEnd);
+ if (RETURN_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: integer overflow\n", __func__));
+ return EFI_NOT_FOUND;
+ }
+
+ if (VarHeaderEnd >= VariableStoreHeader->Size) {
+ if (VarOffset <= VariableStoreHeader->Size - sizeof (UINT16)) {
+ CONST UINT16 *StartId = (VOID *)((UINTN)VariableStoreHeader +
VarOffset);
+ if (*StartId == 0x55aa) {
+ DEBUG ((DEBUG_ERROR, "%a: startid at invalid location\n", __func__));
+ return EFI_NOT_FOUND;
+ }
+ }
+
+ DEBUG ((DEBUG_INFO, "%a: end of var list (no space left)\n", __func__));
+ break;
+ }
+
+ VarHeader = (VOID *)((UINTN)VariableStoreHeader + VarOffset);
+ if (VarHeader->StartId != 0x55aa) {
+ DEBUG ((DEBUG_INFO, "%a: end of var list (no startid)\n", __func__));
+ break;
+ }
+
+ VarName = NULL;
+ switch (VarHeader->State) {
+ // usage: State = VAR_HEADER_VALID_ONLY
+ case VAR_HEADER_VALID_ONLY:
+ VarState = "header-ok";
+ VarName = L"<unknown>";
+ break;
+
+ // usage: State = VAR_ADDED
+ case VAR_ADDED:
+ VarState = "ok";
+ break;
+
+ // usage: State &= VAR_IN_DELETED_TRANSITION
+ case VAR_ADDED &VAR_IN_DELETED_TRANSITION:
+ VarState = "del-in-transition";
+ break;
+
+ // usage: State &= VAR_DELETED
+ 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;
+ }
+
+ Status = SafeUintnAdd (VarHeaderEnd, VarHeader->NameSize, &VarNameEnd);
+ if (RETURN_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: integer overflow\n", __func__));
+ return EFI_NOT_FOUND;
+ }
+
+ Status = SafeUintnAdd (VarNameEnd, VarHeader->DataSize, &VarEnd);
+ if (RETURN_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: integer overflow\n", __func__));
+ return EFI_NOT_FOUND;
+ }
+
+ if (VarEnd > VariableStoreHeader->Size) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: invalid variable size: 0x%Lx + 0x%Lx + 0x%x + 0x%x > 0x%x\n",
+ __func__,
+ (UINT64)VarOffset,
+ (UINT64)(sizeof (*VarHeader)),
+ VarHeader->NameSize,
+ VarHeader->DataSize,
+ VariableStoreHeader->Size
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ if (((VarHeader->NameSize & 1) != 0) ||
+ (VarHeader->NameSize < 4))
+ {
+ DEBUG ((DEBUG_ERROR, "%a: invalid name size\n", __func__));
+ return EFI_NOT_FOUND;
+ }
+
+ if (VarName == NULL) {
+ VarName = (VOID *)((UINTN)VariableStoreHeader + VarHeaderEnd);
+ if (VarName[VarHeader->NameSize / 2 - 1] != L'\0') {
+ DEBUG ((DEBUG_ERROR, "%a: name is not null terminated\n", __func__));
+ return EFI_NOT_FOUND;
+ }
+ }
+
+ DEBUG ((
+ DEBUG_VERBOSE,
+ "%a: +0x%04Lx: name=0x%x data=0x%x guid=%g '%s' (%a)\n",
+ __func__,
+ (UINT64)VarOffset,
+ VarHeader->NameSize,
+ VarHeader->DataSize,
+ &VarHeader->VendorGuid,
+ VarName,
+ VarState
+ ));
+
+ VarPadding = (4 - (VarEnd & 3)) & 3;
+ Status = SafeUintnAdd (VarEnd, VarPadding, &VarOffset);
+ if (RETURN_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: integer overflow\n", __func__));
+ return EFI_NOT_FOUND;
+ }
+ }
+
return EFI_SUCCESS;
}
--
2.43.0
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113441): https://edk2.groups.io/g/devel/message/113441
Mute This Topic: https://groups.io/mt/103617812/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
