Bump. This specific patch needs Reviews.

- Bret


From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Bret Barkelew via groups.io <bret=corthon.com@groups.io>
Sent: Tuesday, June 2, 2020 11:58 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Jian J Wang <jian.j.wang@intel.com>; Hao A Wu <hao.a.wu@intel.com>; liming.gao <liming.gao@intel.com>
Subject: [EXTERNAL] [edk2-devel] [PATCH v5 12/14] MdeModulePkg: Change TCG MOR variables to use VariablePolicy
 
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc4837d136ec548d67adc08d807a3e192%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747730722710&amp;sdata=NAVaoXnmIgu2F0YobNn7oC5XNGudCoalxrB3nPZDl98%3D&amp;reserved=0

These were previously using VarLock, which is
being deprecated.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Bret Barkelew <brbarkel@microsoft.com>
Signed-off-by: Bret Barkelew <brbarkel@microsoft.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c          | 52 ++++++++++++++------
 MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c          | 52 +++++++++++++++-----
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf   |  2 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf |  1 +
 4 files changed, 82 insertions(+), 25 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
index e7accf4ed806..b85f08c48c11 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
@@ -5,6 +5,7 @@
   MOR lock control unsupported.

 

 Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>

+Copyright (c) Microsoft Corporation.

 SPDX-License-Identifier: BSD-2-Clause-Patent

 

 **/

@@ -17,7 +18,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/BaseMemoryLib.h>

 #include "Variable.h"

 

-extern EDKII_VARIABLE_LOCK_PROTOCOL     mVariableLock;

+#include <Protocol/VariablePolicy.h>

+#include <Library/VariablePolicyHelperLib.h>

 

 /**

   This service is an MOR/MorLock checker handler for the SetVariable().

@@ -77,11 +79,6 @@ MorLockInit (
     NULL                                        // Data

     );

 

-  //

-  // Need set this variable to be read-only to prevent other module set it.

-  //

-  VariableLockRequestToLock (&mVariableLock, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, &gEfiMemoryOverwriteRequestControlLockGuid);

-

   //

   // The MOR variable can effectively improve platform security only when the

   // MorLock variable protects the MOR variable. In turn MorLock cannot be made

@@ -99,11 +96,6 @@ MorLockInit (
     0,                                      // DataSize

     NULL                                    // Data

     );

-  VariableLockRequestToLock (

-    &mVariableLock,

-    MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,

-    &gEfiMemoryOverwriteControlDataGuid

-    );

 

   return EFI_SUCCESS;

 }

@@ -118,7 +110,39 @@ MorLockInitAtEndOfDxe (
   VOID

   )

 {

-  //

-  // Do nothing.

-  //

+  EFI_STATUS                        Status;

+  EDKII_VARIABLE_POLICY_PROTOCOL    *VariablePolicy;

+

+  // First, we obviously need to locate the VariablePolicy protocol.

+  Status = gBS->LocateProtocol( &gEdkiiVariablePolicyProtocolGuid, NULL, (VOID**)&VariablePolicy );

+  if (EFI_ERROR( Status )) {

+    DEBUG(( DEBUG_ERROR, "%a - Could not locate VariablePolicy protocol! %r\n", __FUNCTION__, Status ));

+    return;

+  }

+

+  // If we're successful, go ahead and set the policies to protect the target variables.

+  Status = RegisterBasicVariablePolicy( VariablePolicy,

+                                        &gEfiMemoryOverwriteRequestControlLockGuid,

+                                        MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME,

+                                        VARIABLE_POLICY_NO_MIN_SIZE,

+                                        VARIABLE_POLICY_NO_MAX_SIZE,

+                                        VARIABLE_POLICY_NO_MUST_ATTR,

+                                        VARIABLE_POLICY_NO_CANT_ATTR,

+                                        VARIABLE_POLICY_TYPE_LOCK_NOW );

+  if (EFI_ERROR( Status )) {

+    DEBUG(( DEBUG_ERROR, "%a - Could not lock variable %s! %r\n", __FUNCTION__, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, Status ));

+  }

+  Status = RegisterBasicVariablePolicy( VariablePolicy,

+                                        &gEfiMemoryOverwriteControlDataGuid,

+                                        MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,

+                                        VARIABLE_POLICY_NO_MIN_SIZE,

+                                        VARIABLE_POLICY_NO_MAX_SIZE,

+                                        VARIABLE_POLICY_NO_MUST_ATTR,

+                                        VARIABLE_POLICY_NO_CANT_ATTR,

+                                        VARIABLE_POLICY_TYPE_LOCK_NOW );

+  if (EFI_ERROR( Status )) {

+    DEBUG(( DEBUG_ERROR, "%a - Could not lock variable %s! %r\n", __FUNCTION__, MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, Status ));

+  }

+

+  return;

 }

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
index 085f82035f4b..ee37942a6b0c 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
@@ -19,7 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "Variable.h"

 

 #include <Protocol/VariablePolicy.h>

-

+#include <Library/VariablePolicyHelperLib.h>

 #include <Library/VariablePolicyLib.h>

 

 typedef struct {

@@ -422,6 +422,8 @@ MorLockInitAtEndOfDxe (
 {

   UINTN      MorSize;

   EFI_STATUS MorStatus;

+  EFI_STATUS              Status;

+  VARIABLE_POLICY_ENTRY   *NewPolicy;

 

   if (!mMorLockInitializationRequired) {

     //

@@ -494,11 +496,25 @@ MorLockInitAtEndOfDxe (
   // The MOR variable is absent; the platform firmware does not support it.

   // Lock the variable so that no other module may create it.

   //

-  VariableLockRequestToLock (

-    NULL,                                   // This

-    MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,

-    &gEfiMemoryOverwriteControlDataGuid

-    );

+  NewPolicy = NULL;

+  Status = CreateBasicVariablePolicy( &gEfiMemoryOverwriteControlDataGuid,

+                                      MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,

+                                      VARIABLE_POLICY_NO_MIN_SIZE,

+                                      VARIABLE_POLICY_NO_MAX_SIZE,

+                                      VARIABLE_POLICY_NO_MUST_ATTR,

+                                      VARIABLE_POLICY_NO_CANT_ATTR,

+                                      VARIABLE_POLICY_TYPE_LOCK_NOW,

+                                      &NewPolicy );

+  if (!EFI_ERROR( Status )) {

+    Status = RegisterVariablePolicy( NewPolicy );

+  }

+  if (EFI_ERROR( Status )) {

+    DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTION__, MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, Status ));

+    ASSERT_EFI_ERROR( Status );

+  }

+  if (NewPolicy != NULL) {

+    FreePool( NewPolicy );

+  }

 

   //

   // Delete the MOR Control Lock variable too (should it exists for some

@@ -514,9 +530,23 @@ MorLockInitAtEndOfDxe (
     );

   mMorLockPassThru = FALSE;

 

-  VariableLockRequestToLock (

-    NULL,                                       // This

-    MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME,

-    &gEfiMemoryOverwriteRequestControlLockGuid

-    );

+  NewPolicy = NULL;

+  Status = CreateBasicVariablePolicy( &gEfiMemoryOverwriteRequestControlLockGuid,

+                                      MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME,

+                                      VARIABLE_POLICY_NO_MIN_SIZE,

+                                      VARIABLE_POLICY_NO_MAX_SIZE,

+                                      VARIABLE_POLICY_NO_MUST_ATTR,

+                                      VARIABLE_POLICY_NO_CANT_ATTR,

+                                      VARIABLE_POLICY_TYPE_LOCK_NOW,

+                                      &NewPolicy );

+  if (!EFI_ERROR( Status )) {

+    Status = RegisterVariablePolicy( NewPolicy );

+  }

+  if (EFI_ERROR( Status )) {

+    DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTION__, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, Status ));

+    ASSERT_EFI_ERROR( Status );

+  }

+  if (NewPolicy != NULL) {

+    FreePool( NewPolicy );

+  }

 }

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index 48ac167906f7..8debc560e6dc 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -71,6 +71,7 @@ [LibraryClasses]
   AuthVariableLib

   VarCheckLib

   VariablePolicyLib

+  VariablePolicyHelperLib

 

 [Protocols]

   gEfiFirmwareVolumeBlockProtocolGuid           ## CONSUMES

@@ -80,6 +81,7 @@ [Protocols]
   gEfiVariableWriteArchProtocolGuid             ## PRODUCES

   gEfiVariableArchProtocolGuid                  ## PRODUCES

   gEdkiiVariableLockProtocolGuid                ## PRODUCES

+  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES

   gEdkiiVarCheckProtocolGuid                    ## PRODUCES

 

 [Guids]

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
index d8f480be27cc..62f2f9252f43 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
@@ -76,6 +76,7 @@ [LibraryClasses]
   SynchronizationLib

   VarCheckLib

   VariablePolicyLib

+  VariablePolicyHelperLib

 

 [Protocols]

   gEfiSmmFirmwareVolumeBlockProtocolGuid        ## CONSUMES

--
2.26.2.windows.1.8.g01c50adf56.20200515075929


-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#60649): https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F60649&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc4837d136ec548d67adc08d807a3e192%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747730722710&amp;sdata=7J764sVxeEu973uDLn7KBpquLZp7j9A0QzxlCyeaOFM%3D&amp;reserved=0
Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F74646438%2F1852292&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc4837d136ec548d67adc08d807a3e192%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747730722710&amp;sdata=I51sthKt1j%2FdJExzKr2jwopeDgdDimIBPmOBU55KxoM%3D&amp;reserved=0
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc4837d136ec548d67adc08d807a3e192%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747730732700&amp;sdata=XQ8938isA26a%2BDE1oLinYkb49yAq2BDCPnBhyiECgLg%3D&amp;reserved=0  [bret.barkelew@microsoft.com]
-=-=-=-=-=-=