On 05/15/13 11:03, Gary Ching-Pang Lin wrote:
> A variable store length check was introduced since r14252. After
> applying the patch, OVMF died in
>
> SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
>
> ASSERT(MAX (PcdGet32 (PcdMaxVariableSize), PcdGet32 
> (PcdMaxHardwareErrorVariableSize)) < VariableStoreLength);
>
> The check is reasonable. However, in OvmfPkg/OvmfPkgX64.dsc:
>
> !if $(SECURE_BOOT_ENABLE) == TRUE
>   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x10000
> !else
>   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400
> !endif

## The maximum size of single common variable, that is non-HwErr type
#  varible.


>   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x8000

## The maximum size of single hardware error record variable.
# In IA32/X64 platforms, this value should be larger than 1KB.
# In IA64 platforms, this value should be larger than 128KB.


>   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xc000

## The size of volatile buffer. This buffer is used to store VOLATILE
#  attribute variable.


>   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0xc000

## Size of the NV variable range. Note that this value should less than
#  or equal to PcdFlashNvStorageFtwSpareSize
#  The root cause is that variable driver will use FTW protocol to
#  reclaim variable region.
#  If the length of variable region is larger than FTW spare size, it
#  means the whole variable region can not be reflushed through the
#  manner of fault tolerant write.


>   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x2000

## Size of the FTW working block range.


>   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x10000

## Size of the FTW spare block range. Note that this value should larger
#  than PcdFlashNvStorageVariableSize
#  The root cause is that variable driver will use FTW protocol to
#  reclaim variable region.
#  If the length of variable region is larger than FTW spare size, it
#  means the whole variable region can not be reflushed through the
#  manner of fault tolerant write.


> When Secure Boot is enabled, PcdFlashNvStorageVariableSize is much
> smaller than PcdMaxVariableSize, not to mention VariableStoreLength
> which is derived from PcdFlashNvStorageVariableSize, so the check
> always fails.
>
> Any suggestion about the variable size?

PcdMaxVariableSize was separated *and* raised for secure boot in svn
r13093. Apparently secure boot needs 64 times as much. I guess this
immediately violated some logic constraint (being greater than
PcdFlashNvStorageVariableSize -- "The max variable or hardware error
variable size should be < variable store size"), only that constraint
wasn't enforced until r14252.

... I wonder if this has been all along in the background of boot option
variables not working at all in the secure boot build of OVMF.

Apparently we're subject to at least the following inequalities:

  PcdMaxVariableSize              + HeaderLength < 
PcdFlashNvStorageVariableSize <= PcdFlashNvStorageFtwSpareSize
  PcdMaxHardwareErrorVariableSize + HeaderLength < 
PcdFlashNvStorageVariableSize <= PcdFlashNvStorageFtwSpareSize

Assuming PcdMaxVariableSize was set to 0x10000 in r13093, 64 times the
non-secure boot value, for a good reason, we should leave it intact and
raise the other variables instead.

Based on "OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.c" I'm under the
impression that HeaderLength is EMU_FV_HEADER_LENGTH here, which should
equal sizeof(EFI_FIRMWARE_VOLUME_HEADER) +
sizeof(EFI_FV_BLOCK_MAP_ENTRY).

Assuming packed structures (... which could be a stretch for
EFI_FIRMWARE_VOLUME_HEADER...),

  sizeof(EFI_FV_BLOCK_MAP_ENTRY)     == 8
  sizeof(EFI_FIRMWARE_VOLUME_HEADER) == 64
  EMU_FV_HEADER_LENGTH               == 72


... Does the attached patch work for you? It enables me to run the
SECURE_BOOT_ENABLE build. I could even test secure booting Windows 8 (a
preview build):
<http://www.linux-kvm.org/page/OVMF#Confirmation_of_secure_boot_in_Windows_8_Consumer_Preview_Build_8250>.

(

Unfortunately the enrolled keys don't seem to survive a VM reboot, so
there's no change in that -- this problem likely isn't related to NvVar
storage sizes.

Also I failed to secure boot Fedora 19
<http://www.linux-kvm.org/page/OVMF#Confirmation_of_secure_boot_in_Fedora_18>,
which I guess might still relate to this thread (also started by you):
<http://thread.gmane.org/gmane.comp.bios.tianocore.devel/2329>.

)

Thanks,
Laszlo
From 2998b767ac0891c3e770a59f94baafa6f22cf120 Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <ler...@redhat.com>
Date: Wed, 15 May 2013 14:00:05 +0200
Subject: [PATCH] OvmfPkg: raise NvVar storage sizes

The following inequalities must hold for MdeModulePkg PCDs related to
NvVar storage:

  PcdMaxVariableSize + HeaderLength
  < PcdFlashNvStorageVariableSize

  PcdMaxHardwareErrorVariableSize + HeaderLength
  < PcdFlashNvStorageVariableSize

  PcdFlashNvStorageVariableSize <= PcdFlashNvStorageFtwSpareSize

(In our case HeaderLength should be EMU_FV_HEADER_LENGTH, < 0x100.)

In the SECURE_BOOT_ENABLE build PcdMaxVariableSize breaks the first
inequality, which triggers an assert introduced in svn r14252. Raise the
right hand sides of the inequalities to restore them:

  PcdFlashNvStorageVariableSize := 0x12000
  PcdFlashNvStorageFtwSpareSize := 0x14000

In addition, synchronize

  PcdVariableStoreSize := PcdFlashNvStorageVariableSize

otherwise

  ASSERT(VariableStoreHeader->Size == VariableStoreLength)

fires in VariableCommonInitialize(), at
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c:2761.

"NvVars" files from before the patch seem to continue working.

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    |    6 +++---
 OvmfPkg/OvmfPkgIa32X64.dsc |    6 +++---
 OvmfPkg/OvmfPkgX64.dsc     |    6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index c99cee3..da89868 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -277,10 +277,10 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400
 !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x8000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xc000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0xc000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x12000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x12000
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x2000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x10000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x14000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 9173aae..8b0eb9c 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -282,10 +282,10 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400
 !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x8000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xc000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0xc000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x12000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x12000
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x2000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x10000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x14000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 05f8aaa..b6f4ba7 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -282,10 +282,10 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x400
 !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x8000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xc000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0xc000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x12000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x12000
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x2000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x10000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x14000
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
 
-- 
1.7.1

------------------------------------------------------------------------------
AlienVault Unified Security Management (USM) platform delivers complete
security visibility with the essential security capabilities. Easily and
efficiently configure, manage, and operate all of your security controls
from a single console and one unified framework. Download a free trial.
http://p.sf.net/sfu/alienvault_d2d
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to