diff mbox

[edk2,v3,2/3] ArmPlatformPkg: enable use of authenticated variables in NorFlashDxe

Message ID 1430979400-24189-3-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel May 7, 2015, 6:16 a.m. UTC
The NorFlashDxe uses an explicit 'BEFORE xxx' Depex declaration to
ensure that it is dispatched before VariableRuntimeDxe, and uses the
file GUID of the latter as 'xxx' explicitly to accomplish that.

However, when enabling UEFI Secure Boot, this breaks down since the
authenticated VariableRuntimeDxe is a completely separate driver, with
a different GUID. Also, the hardcoded dependency on gEfiVariableGuid,
which is not used under UEFI Secure Boot, needs to be factored out in
order to allow this driver to be used.

So clone NorFlashDxe.inf into NorFlashAuthenticatedDxe.inf, and fix
up the dependencies so they refer to gEfiAuthenticatedVariableGuid and
SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/ArmPlatformPkg.dec                                     |  4 ++
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec                      |  4 --
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedDxe.inf       | 76 ++++++++++++++++++++
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedVariableDep.c | 19 +++++
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h                      |  2 +
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf                    |  1 +
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c                   |  4 +-
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashVariableDep.c              | 19 +++++
 8 files changed, 123 insertions(+), 6 deletions(-)

Comments

Ard Biesheuvel May 7, 2015, 7:27 a.m. UTC | #1
On 7 May 2015 at 08:55, Laszlo Ersek <lersek@redhat.com> wrote:
> On 05/07/15 08:16, Ard Biesheuvel wrote:
>> The NorFlashDxe uses an explicit 'BEFORE xxx' Depex declaration to
>> ensure that it is dispatched before VariableRuntimeDxe, and uses the
>> file GUID of the latter as 'xxx' explicitly to accomplish that.
>>
>> However, when enabling UEFI Secure Boot, this breaks down since the
>> authenticated VariableRuntimeDxe is a completely separate driver, with
>> a different GUID. Also, the hardcoded dependency on gEfiVariableGuid,
>> which is not used under UEFI Secure Boot, needs to be factored out in
>> order to allow this driver to be used.
>>
>> So clone NorFlashDxe.inf into NorFlashAuthenticatedDxe.inf, and fix
>> up the dependencies so they refer to gEfiAuthenticatedVariableGuid and
>> SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
>> instead.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmPlatformPkg/ArmPlatformPkg.dec                                     |  4 ++
>>  ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec                      |  4 --
>>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedDxe.inf       | 76 ++++++++++++++++++++
>>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedVariableDep.c | 19 +++++
>>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h                      |  2 +
>>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf                    |  1 +
>>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c                   |  4 +-
>>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashVariableDep.c              | 19 +++++
>>  8 files changed, 123 insertions(+), 6 deletions(-)
>
> snip
>
>> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
>> index c24680098f62..9b76bfa1df23 100644
>> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
>> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
>> @@ -152,6 +152,8 @@ struct _NOR_FLASH_INSTANCE {
>>    NOR_FLASH_DEVICE_PATH               DevicePath;
>>  };
>>
>> +CONST EFI_GUID* CONST                 mNorFlashVariableGuid;
>> +
>>  EFI_STATUS
>>  NorFlashReadCfiData (
>>    IN  UINTN                   DeviceBaseAddress,
>
> While this is valid C -- the above is a "tentative definition",
> discussed eg. in C89 6.7.2 p2, and C99 6.9.2 p2 -- it is more usual to
> add the storage-class specifier "extern" to such declarations.
>
> So, please do that, and then you can add my
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>

Thanks.

I have pushed these 3 patches (with this change and your R-b added) to:
https://git.linaro.org/leg/edk2.git/shortlog/refs/heads/linaro-topic-uefi-secure-boot

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
diff mbox

Patch

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 9364bb92c5f0..58328345bd06 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -36,6 +36,10 @@ 
   # Following Guid must match FILE_GUID in MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
   #
   gVariableRuntimeDxeFileGuid = { 0xcbd2e4d5, 0x7068, 0x4ff5, { 0xb4, 0x62, 0x98, 0x22, 0xb4, 0xad, 0x8d, 0x60 } }
+  #
+  # Following Guid must match FILE_GUID in SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf
+  #
+  gVariableAuthenticatedRuntimeDxeFileGuid = { 0x2226f30f, 0x3d5b, 0x402d,  {0x99, 0x36, 0xa9, 0x71, 0x84, 0xEB, 0x45, 0x16 } }
 
   ## Include/Guid/ArmGlobalVariableHob.h
   gArmGlobalVariableGuid      = { 0xc3253c90, 0xa24f, 0x4599, { 0xa6, 0x64, 0x1f, 0x88, 0x13, 0x77, 0x8f, 0xc9} }
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
index e8108bc34b56..fd59375d9baf 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
@@ -33,10 +33,6 @@ 
 
 [Guids.common]
   gArmVExpressTokenSpaceGuid    =  { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
-  #
-  # Following Guid must match FILE_GUID in MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
-  #
-  gVariableRuntimeDxeFileGuid = { 0xcbd2e4d5, 0x7068, 0x4ff5, { 0xb4, 0x62, 0x98, 0x22, 0xb4, 0xad, 0x8d, 0x60 } }
 
 [PcdsFeatureFlag.common]
 
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedDxe.inf
new file mode 100644
index 000000000000..ff8f048ecb21
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedDxe.inf
@@ -0,0 +1,76 @@ 
+#/** @file
+#
+#  Component description file for NorFlashAuthenticatedDxe module
+#
+#  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
+#  Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = NorFlashAuthenticatedDxe
+  FILE_GUID                      = 10B86CEA-F2FE-456A-B1C7-4F506CA46005
+  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = NorFlashInitialise
+
+[Sources.common]
+  NorFlashDxe.c
+  NorFlashFvbDxe.c
+  NorFlashBlockIoDxe.c
+  NorFlashAuthenticatedVariableDep.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  SecurityPkg/SecurityPkg.dec
+
+[LibraryClasses]
+  IoLib
+  BaseLib
+  DebugLib
+  HobLib
+  NorFlashPlatformLib
+  UefiLib
+  UefiDriverEntryPoint
+  UefiBootServicesTableLib
+  UefiRuntimeLib
+  DxeServicesTableLib
+
+[Guids]
+  gEfiSystemNvDataFvGuid
+  gEfiAuthenticatedVariableGuid
+  gEfiEventVirtualAddressChangeGuid
+
+[Protocols]
+  gEfiBlockIoProtocolGuid
+  gEfiDevicePathProtocolGuid
+  gEfiFirmwareVolumeBlockProtocolGuid
+  gEfiDiskIoProtocolGuid
+
+[Pcd.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+
+  gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked
+
+[Depex]
+  #
+  # NorFlashAuthenticatedDxe must be loaded before VariableAuthenticatedRuntimeDxe
+  # in case empty flash needs populating with default values
+  #
+  BEFORE gVariableAuthenticatedRuntimeDxeFileGuid
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedVariableDep.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedVariableDep.c
new file mode 100644
index 000000000000..2ea8ead85d9b
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashAuthenticatedVariableDep.c
@@ -0,0 +1,19 @@ 
+/** @file  NorFlashAuthenticatedVariableDep.c
+
+  Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+
+#include <Include/Guid/AuthenticatedVariableFormat.h>
+
+CONST EFI_GUID* CONST mNorFlashVariableGuid = &gEfiAuthenticatedVariableGuid;
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
index c24680098f62..9b76bfa1df23 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
@@ -152,6 +152,8 @@  struct _NOR_FLASH_INSTANCE {
   NOR_FLASH_DEVICE_PATH               DevicePath;
 };
 
+CONST EFI_GUID* CONST                 mNorFlashVariableGuid;
+
 EFI_STATUS
 NorFlashReadCfiData (
   IN  UINTN                   DeviceBaseAddress,
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index a161c0399e52..563d7573e7a2 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -26,6 +26,7 @@ 
   NorFlashDxe.c
   NorFlashFvbDxe.c
   NorFlashBlockIoDxe.c
+  NorFlashVariableDep.c
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index 4f56bae33022..3ed3bb945ff6 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -111,7 +111,7 @@  InitializeFvAndVariableStoreHeaders (
   // VARIABLE_STORE_HEADER
   //
   VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)Headers + FirmwareVolumeHeader->HeaderLength);
-  CopyGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid);
+  CopyGuid (&VariableStoreHeader->Signature, mNorFlashVariableGuid);
   VariableStoreHeader->Size = PcdGet32(PcdFlashNvStorageVariableSize) - FirmwareVolumeHeader->HeaderLength;
   VariableStoreHeader->Format            = VARIABLE_STORE_FORMATTED;
   VariableStoreHeader->State             = VARIABLE_STORE_HEALTHY;
@@ -178,7 +178,7 @@  ValidateFvHeader (
   VariableStoreHeader = (VARIABLE_STORE_HEADER*)((UINTN)FwVolHeader + FwVolHeader->HeaderLength);
 
   // Check the Variable Store Guid
-  if( CompareGuid (&VariableStoreHeader->Signature, &gEfiVariableGuid) == FALSE ) {
+  if (!CompareGuid (&VariableStoreHeader->Signature, mNorFlashVariableGuid)) {
     DEBUG ((EFI_D_ERROR, "ValidateFvHeader: Variable Store Guid non-compatible\n"));
     return EFI_NOT_FOUND;
   }
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashVariableDep.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashVariableDep.c
new file mode 100644
index 000000000000..4d52296ce1a0
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashVariableDep.c
@@ -0,0 +1,19 @@ 
+/** @file  NorFlashVariableDep.c
+
+  Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+
+#include <Include/Guid/VariableFormat.h>
+
+CONST EFI_GUID* CONST mNorFlashVariableGuid = &gEfiVariableGuid;