Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c | 37 ++++++++++++++++++++++++++++++++++--- Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c | 61 +++++++++++++++++++++++++++++++++++++++++++++---------------- Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf | 3 ++- Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h | 26 ++++++++++++++++++++++++-- Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf | 8 +++++--- 6 files changed, 248 insertions(+), 29 deletions(-)
From: "Chiu, Chasel" <chasel.chiu@intel.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829
Fixed the bug that existing variable will not be locked when it is
identical with hob data by creating LockLargeVariable function, also
switched to VariablePolicyProtocol for locking variables.
Failing to lock variable could be security vulnerability, so the
LargeVariableWriteLib functions will return EFI_ABORTED if locking
was failed and SaveMemoryConfig driver will delete variable to
prevent from using unlocked variable.
This patch also modified SaveMemoryConfig driver to be unloaded after
execution because it does not produce any service protocol. To achieve
this goal the DxeRuntimeVariableWriteLib should close registered
ExitBootService events in its DESTRUCTOR.
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Eric Dong <eric.dong@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>
---Patch V4: 1.Removed CpuDeadLoop and delete variable if Lock failed in SaveMemoryConfig. 2. LargeVariableWrite will not delete variable if Lock failed, let caller to do it.
Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c | 37 ++++++++++++++++++++++++++++++++++---
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c | 61 +++++++++++++++++++++++++++++++++++++++++++++----------------
Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf | 3 ++-
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h | 26 ++++++++++++++++++++++++--
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf | 8 +++++---
6 files changed, 248 insertions(+), 29 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
index 820585f676..fb51edd5cb 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
@@ -2,13 +2,14 @@
This is the driver that locates the MemoryConfigurationData HOB, if it
exists, and saves the data to nvRAM.
-Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
#include <Base.h>
#include <Uefi.h>
+#include <Library/BaseLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiRuntimeServicesTableLib.h>
#include <Library/HobLib.h>
@@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/BaseMemoryLib.h>
#include <Library/LargeVariableReadLib.h>
#include <Library/LargeVariableWriteLib.h>
+#include <Library/VariableWriteLib.h>
#include <Guid/FspNonVolatileStorageHob2.h>
/**
@@ -86,6 +88,23 @@ SaveMemoryConfigEntryPoint (
Status = GetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, &BufferSize, VariableData);
if (!EFI_ERROR (Status) && (BufferSize == DataSize) && (0 == CompareMem (HobData, VariableData, DataSize))) {
DataIsIdentical = TRUE;
+ //
+ // No need to update Variable, only lock it.
+ //
+ Status = LockLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid);
+ if (EFI_ERROR (Status)) {
+ //
+ // Fail to lock variable is security vulnerability and should not happen.
+ //
+ ASSERT_EFI_ERROR (Status);
+ //
+ // When building without ASSERT_EFI_ERROR hang, delete the varialbe so it will not be consumed.
+ //
+ DEBUG ((DEBUG_ERROR, "Delete variable!\n"));
+ DataSize = 0;
+ Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, FALSE, DataSize, HobData);
+ ASSERT_EFI_ERROR (Status);
+ }
}
FreePool (VariableData);
}
@@ -95,6 +114,18 @@ SaveMemoryConfigEntryPoint (
if (!DataIsIdentical) {
Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, TRUE, DataSize, HobData);
+ if (Status == EFI_ABORTED) {
+ //
+ // Fail to lock variable! This should not happen.
+ //
+ ASSERT_EFI_ERROR (Status);
+ //
+ // When building without ASSERT_EFI_ERROR hang, delete the varialbe so it will not be consumed.
+ //
+ DEBUG ((DEBUG_ERROR, "Delete variable!\n"));
+ DataSize = 0;
+ Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, FALSE, DataSize, HobData);
+ }
ASSERT_EFI_ERROR (Status);
DEBUG ((DEBUG_INFO, "Saved size of FSP / MRC Training Data: 0x%x\n", DataSize));
} else {
@@ -106,7 +137,7 @@ SaveMemoryConfigEntryPoint (
}
//
- // This driver cannot be unloaded because DxeRuntimeVariableWriteLib constructor will register ExitBootServices callback.
+ // This driver does not produce any protocol services, so always unload it.
//
- return EFI_SUCCESS;
+ return EFI_REQUEST_UNLOAD_IMAGE;
}
diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
index e4b97ef1df..cb8e2e7f0a 100644
--- a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
+++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
@@ -10,7 +10,7 @@
integer number will be added to the end of the variable name. This number
will be incremented for each variable as needed to store the entire data set.
- Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -245,7 +245,7 @@ Done:
@retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
@retval EFI_WRITE_PROTECTED The variable in question is read-only.
@retval EFI_WRITE_PROTECTED The variable in question cannot be deleted.
-
+ @retval EFI_ABORTED LockVariable was requested but failed.
@retval EFI_NOT_FOUND The variable trying to be updated or deleted was not found.
**/
@@ -323,6 +323,13 @@ SetLargeVariable (
}
if (LockVariable) {
Status = VarLibVariableRequestToLock (VariableName, VendorGuid);
+ if (EFI_ERROR (Status)) {
+ //
+ // Do not delete Variable when failed to lock. Caller is responsible to do this.
+ //
+ DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error locking variable: Status = %r\n", Status));
+ Status = EFI_ABORTED;
+ }
}
} else {
//
@@ -402,7 +409,7 @@ SetLargeVariable (
DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error writting variable: Status = %r\n", Status));
goto Done;
}
- VariablesSaved = Index;
+ VariablesSaved ++;
BytesRemaining -= SizeToSave;
OffsetPtr += SizeToSave;
} // End of for loop
@@ -420,6 +427,10 @@ SetLargeVariable (
Status = VarLibVariableRequestToLock (TempVariableName, VendorGuid);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error locking variable: Status = %r\n", Status));
+ //
+ // Do not delete Variable when failed to lock. Caller is responsible to do this.
+ //
+ Status = EFI_ABORTED;
VariablesSaved = 0;
goto Done;
}
@@ -442,9 +453,132 @@ Done:
0,
NULL
);
- DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error deleting variable: Status = %r\n", Status2));
+ if (EFI_ERROR (Status2)) {
+ DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error deleting variable: Status = %r\n", Status2));
+ }
}
}
DEBUG ((DEBUG_ERROR, "SetLargeVariable: Status = %r\n", Status));
return Status;
}
+
+/**
+ Locks the existing large variable.
+
+ @param[in] VariableName A Null-terminated string that is the name of the vendor's variable.
+ Each VariableName is unique for each VendorGuid. VariableName must
+ contain 1 or more characters. If VariableName is an empty string,
+ then EFI_INVALID_PARAMETER is returned.
+ @param[in] VendorGuid A unique identifier for the vendor.
+ @retval EFI_SUCCESS The firmware has successfully locked the variable.
+ @retval EFI_INVALID_PARAMETER An invalid combination of variable name and GUID was supplied
+ @retval EFI_OUT_OF_RESOURCES The VariableName is longer than 1018 characters
+ @retval EFI_UNSUPPORTED The service for locking variable is not ready.
+ @retval EFI_NOT_FOUND The targeting variable for locking is not present.
+ @retval EFI_ABORTED Fail to lock variable.
+**/
+EFI_STATUS
+EFIAPI
+LockLargeVariable (
+ IN CHAR16 *VariableName,
+ IN EFI_GUID *VendorGuid
+ )
+{
+ CHAR16 TempVariableName[MAX_VARIABLE_NAME_SIZE];
+ UINT64 VariableSize;
+ EFI_STATUS Status;
+ UINTN Index;
+
+ //
+ // Check input parameters.
+ //
+ if (VariableName == NULL || VariableName[0] == 0 || VendorGuid == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Check the length of the variable name is short enough to allow an integer
+ // to be appended.
+ //
+ if (StrLen (VariableName) >= (MAX_VARIABLE_NAME_SIZE - MAX_VARIABLE_SPLIT_DIGITS)) {
+ DEBUG ((DEBUG_ERROR, "LockLargeVariable: Variable name too long\n"));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ if (!VarLibIsVariableRequestToLockSupported ()) {
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Check if it is single variable scenario.
+ //
+ VariableSize = 0;
+ Status = VarLibGetVariable (VariableName, VendorGuid, NULL, &VariableSize, NULL);
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // Lock single variable.
+ //
+ DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", VariableName, VendorGuid));
+ Status = VarLibVariableRequestToLock (VariableName, VendorGuid);
+ if (EFI_ERROR (Status)) {
+ //
+ // Do not delete Variable when failed to lock. Caller is responsible to do this.
+ //
+ DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n", Status));
+ return EFI_ABORTED;
+ }
+ return EFI_SUCCESS;
+ } else {
+ //
+ // Check if it is multiple variables scenario.
+ //
+ Index = 0;
+ ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
+ UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", VariableName, Index);
+ VariableSize = 0;
+ Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL, &VariableSize, NULL);
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ //
+ // Lock first variable and continue to rest of the variables.
+ //
+ DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", TempVariableName, VendorGuid));
+ Status = VarLibVariableRequestToLock (TempVariableName, VendorGuid);
+ if (EFI_ERROR (Status)) {
+ //
+ // Do not delete Variable when failed to lock. Caller is responsible to do this.
+ //
+ DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n", Status));
+ return EFI_ABORTED;
+ }
+ for (Index = 1; Index < MAX_VARIABLE_SPLIT; Index++) {
+ ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
+ UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", VariableName, Index);
+
+ VariableSize = 0;
+ Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL, &VariableSize, NULL);
+ if (Status == EFI_BUFFER_TOO_SMALL) {
+ DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", TempVariableName, VendorGuid));
+ Status = VarLibVariableRequestToLock (TempVariableName, VendorGuid);
+ if (EFI_ERROR (Status)) {
+ //
+ // Do not delete Variable when failed to lock. Caller is responsible to do this.
+ //
+ DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n", Status));
+ return EFI_ABORTED;
+ }
+ } else if (Status == EFI_NOT_FOUND) {
+ //
+ // No more variables need to lock.
+ //
+ return EFI_SUCCESS;
+ }
+ } // End of for loop
+ }
+ }
+
+ //
+ // Here probably means variable not present.
+ //
+ return Status;
+
+}
diff --git a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
index 9ed59f8827..28730f858b 100644
--- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
+++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
@@ -10,7 +10,7 @@
Using this library allows code to be written in a generic manner that can be
used in DXE or SMM without modification.
- Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -18,14 +18,16 @@
#include <Uefi.h>
#include <Guid/EventGroup.h>
-#include <Protocol/VariableLock.h>
+#include <Library/VariablePolicyHelperLib.h>
#include <Library/UefiLib.h>
#include <Library/DebugLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiRuntimeServicesTableLib.h>
-STATIC EDKII_VARIABLE_LOCK_PROTOCOL *mVariableWriteLibVariableLock = NULL;
+STATIC EDKII_VARIABLE_POLICY_PROTOCOL *mVariableWriteLibVariablePolicy = NULL;
+EFI_EVENT mExitBootServiceEvent;
+EFI_EVENT mLegacyBootEvent;
/**
Sets the value of a variable.
@@ -144,7 +146,7 @@ VarLibIsVariableRequestToLockSupported (
VOID
)
{
- if (mVariableWriteLibVariableLock != NULL) {
+ if (mVariableWriteLibVariablePolicy != NULL) {
return TRUE;
} else {
return FALSE;
@@ -178,16 +180,45 @@ VarLibVariableRequestToLock (
{
EFI_STATUS Status = EFI_UNSUPPORTED;
- if (mVariableWriteLibVariableLock != NULL) {
- Status = mVariableWriteLibVariableLock->RequestToLock (
- mVariableWriteLibVariableLock,
- VariableName,
- VendorGuid
- );
+ if (mVariableWriteLibVariablePolicy != NULL) {
+ Status = RegisterBasicVariablePolicy (
+ mVariableWriteLibVariablePolicy,
+ (CONST EFI_GUID*) VendorGuid,
+ (CONST CHAR16 *) VariableName,
+ 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
+ );
}
return Status;
}
+/**
+ Close events when driver unloaded.
+
+ @param[in] ImageHandle A handle for the image that is initializing this driver
+ @param[in] SystemTable A pointer to the EFI system table
+
+ @retval EFI_SUCCESS The initialization finished successfully.
+**/
+EFI_STATUS
+EFIAPI
+DxeRuntimeVariableWriteLibDestructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ if (mExitBootServiceEvent != 0) {
+ gBS->CloseEvent (mExitBootServiceEvent);
+ }
+ if (mLegacyBootEvent != 0) {
+ gBS->CloseEvent (mLegacyBootEvent);
+ }
+ return EFI_SUCCESS;
+}
+
/**
Exit Boot Services Event notification handler.
@@ -202,7 +233,7 @@ DxeRuntimeVariableWriteLibOnExitBootServices (
IN VOID *Context
)
{
- mVariableWriteLibVariableLock = NULL;
+ mVariableWriteLibVariablePolicy = NULL;
}
/**
@@ -227,13 +258,11 @@ DxeRuntimeVariableWriteLibConstructor (
)
{
EFI_STATUS Status;
- EFI_EVENT ExitBootServiceEvent;
- EFI_EVENT LegacyBootEvent;
//
// Locate VariableLockProtocol.
//
- Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **)&mVariableWriteLibVariableLock);
+ Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL, (VOID **)&mVariableWriteLibVariablePolicy);
ASSERT_EFI_ERROR (Status);
//
@@ -245,7 +274,7 @@ DxeRuntimeVariableWriteLibConstructor (
DxeRuntimeVariableWriteLibOnExitBootServices,
NULL,
&gEfiEventExitBootServicesGuid,
- &ExitBootServiceEvent
+ &mExitBootServiceEvent
);
ASSERT_EFI_ERROR (Status);
@@ -257,7 +286,7 @@ DxeRuntimeVariableWriteLibConstructor (
TPL_NOTIFY,
DxeRuntimeVariableWriteLibOnExitBootServices,
NULL,
- &LegacyBootEvent
+ &mLegacyBootEvent
);
ASSERT_EFI_ERROR (Status);
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
index e2dbd2fb49..61e85a6586 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
@@ -1,7 +1,7 @@
### @file
# Component information file for SaveMemoryConfig module
#
-# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -25,6 +25,7 @@
BaseMemoryLib
LargeVariableReadLib
LargeVariableWriteLib
+ BaseLib
[Packages]
MdePkg/MdePkg.dec
diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
index c847d7f152..1b9370685a 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
@@ -16,7 +16,7 @@
is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
solution to this problem.
- Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -52,7 +52,7 @@
@retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
@retval EFI_WRITE_PROTECTED The variable in question is read-only.
@retval EFI_WRITE_PROTECTED The variable in question cannot be deleted.
-
+ @retval EFI_ABORTED LockVariable was requested but failed.
@retval EFI_NOT_FOUND The variable trying to be updated or deleted was not found.
**/
@@ -66,4 +66,26 @@ SetLargeVariable (
IN VOID *Data
);
+/**
+ Locks the existing large variable.
+
+ @param[in] VariableName A Null-terminated string that is the name of the vendor's variable.
+ Each VariableName is unique for each VendorGuid. VariableName must
+ contain 1 or more characters. If VariableName is an empty string,
+ then EFI_INVALID_PARAMETER is returned.
+ @param[in] VendorGuid A unique identifier for the vendor.
+ @retval EFI_SUCCESS The firmware has successfully locked the variable.
+ @retval EFI_INVALID_PARAMETER An invalid combination of variable name and GUID was supplied
+ @retval EFI_OUT_OF_RESOURCES The VariableName is longer than 1018 characters
+ @retval EFI_UNSUPPORTED The service for locking variable is not ready.
+ @retval EFI_NOT_FOUND The targeting variable for locking is not present.
+ @retval EFI_ABORTED Fail to lock variable.
+**/
+EFI_STATUS
+EFIAPI
+LockLargeVariable (
+ IN CHAR16 *VariableName,
+ IN EFI_GUID *VendorGuid
+ );
+
#endif // _LARGE_VARIABLE_WRITE_LIB_H_
diff --git a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
index 704a8ac7cc..f83090c847 100644
--- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
+++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
@@ -10,7 +10,7 @@
# Using this library allows code to be written in a generic manner that can be
# used in DXE or SMM without modification.
#
-# Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -24,6 +24,7 @@
MODULE_TYPE = DXE_RUNTIME_DRIVER
LIBRARY_CLASS = VariableWriteLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
CONSTRUCTOR = DxeRuntimeVariableWriteLibConstructor
+ DESTRUCTOR = DxeRuntimeVariableWriteLibDestructor
[Packages]
MdePkg/MdePkg.dec
@@ -37,13 +38,14 @@
UefiLib
UefiBootServicesTableLib
UefiRuntimeServicesTableLib
+ VariablePolicyHelperLib
[Guids]
gEfiEventExitBootServicesGuid ## CONSUMES ## Event
[Protocols]
gEfiVariableWriteArchProtocolGuid ## CONSUMES
- gEdkiiVariableLockProtocolGuid ## CONSUMES
+ gEdkiiVariablePolicyProtocolGuid ## CONSUMES
[Depex]
- gEfiVariableWriteArchProtocolGuid AND gEdkiiVariableLockProtocolGuid
+ gEfiVariableWriteArchProtocolGuid AND gEdkiiVariablePolicyProtocolGuid
--
2.28.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86723): https://edk2.groups.io/g/devel/message/86723
Mute This Topic: https://groups.io/mt/89204326/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Chasel,
There are a couple of typos and minor code style issues pointed out below. Please fix those during the push... no need to send another patch.
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
Thanks,
Nate
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu,
> Chasel
> Sent: Wednesday, February 16, 2022 10:21 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Oram,
> Isaac W <isaac.w.oram@intel.com>
> Subject: [edk2-devel] [edk2-platforms: PATCH v4]
> MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
>
> From: "Chiu, Chasel" <chasel.chiu@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829
>
> Fixed the bug that existing variable will not be locked when it is
> identical with hob data by creating LockLargeVariable function, also
> switched to VariablePolicyProtocol for locking variables.
>
> Failing to lock variable could be security vulnerability, so the
> LargeVariableWriteLib functions will return EFI_ABORTED if locking
> was failed and SaveMemoryConfig driver will delete variable to
> prevent from using unlocked variable.
>
> This patch also modified SaveMemoryConfig driver to be unloaded after
> execution because it does not produce any service protocol. To achieve
> this goal the DxeRuntimeVariableWriteLib should close registered
> ExitBootService events in its DESTRUCTOR.
>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>
> ---Patch V4: 1.Removed CpuDeadLoop and delete variable if Lock failed in SaveMemoryConfig. 2. LargeVariableWrite will not delete variable if Lock failed, let caller to do it.
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c | 37 ++++++++++++++++++++++++++++++++++---
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c | 61 +++++++++++++++++++++++++++++++++++++++++++++----------------
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf | 3 ++-
> Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h | 26 ++++++++++++++++++++++++--
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf | 8 +++++---
> 6 files changed, 248 insertions(+), 29 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
> index 820585f676..fb51edd5cb 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.c
> @@ -2,13 +2,14 @@
> This is the driver that locates the MemoryConfigurationData HOB, if it
> exists, and saves the data to nvRAM.
>
> -Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
>
> #include <Base.h>
> #include <Uefi.h>
> +#include <Library/BaseLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiRuntimeServicesTableLib.h>
> #include <Library/HobLib.h>
> @@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/BaseMemoryLib.h>
> #include <Library/LargeVariableReadLib.h>
> #include <Library/LargeVariableWriteLib.h>
> +#include <Library/VariableWriteLib.h>
> #include <Guid/FspNonVolatileStorageHob2.h>
>
> /**
> @@ -86,6 +88,23 @@ SaveMemoryConfigEntryPoint (
> Status = GetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, &BufferSize, VariableData);
> if (!EFI_ERROR (Status) && (BufferSize == DataSize) && (0 == CompareMem (HobData, VariableData, DataSize))) {
> DataIsIdentical = TRUE;
> + //
> + // No need to update Variable, only lock it.
> + //
> + Status = LockLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid);
> + if (EFI_ERROR (Status)) {
> + //
> + // Fail to lock variable is security vulnerability and should not happen.
> + //
> + ASSERT_EFI_ERROR (Status);
> + //
> + // When building without ASSERT_EFI_ERROR hang, delete the varialbe so it will not be consumed.
Typo here, "varialbe" should be "variable".
> + //
> + DEBUG ((DEBUG_ERROR, "Delete variable!\n"));
> + DataSize = 0;
> + Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, FALSE, DataSize, HobData);
> + ASSERT_EFI_ERROR (Status);
> + }
> }
> FreePool (VariableData);
> }
> @@ -95,6 +114,18 @@ SaveMemoryConfigEntryPoint (
>
> if (!DataIsIdentical) {
> Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, TRUE, DataSize, HobData);
> + if (Status == EFI_ABORTED) {
> + //
> + // Fail to lock variable! This should not happen.
> + //
> + ASSERT_EFI_ERROR (Status);
> + //
> + // When building without ASSERT_EFI_ERROR hang, delete the varialbe so it will not be consumed.
Typo here, "varialbe" should be "variable".
> + //
> + DEBUG ((DEBUG_ERROR, "Delete variable!\n"));
> + DataSize = 0;
> + Status = SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVariableGuid, FALSE, DataSize, HobData);
> + }
> ASSERT_EFI_ERROR (Status);
> DEBUG ((DEBUG_INFO, "Saved size of FSP / MRC Training Data: 0x%x\n", DataSize));
> } else {
> @@ -106,7 +137,7 @@ SaveMemoryConfigEntryPoint (
> }
>
> //
> - // This driver cannot be unloaded because DxeRuntimeVariableWriteLib constructor will register ExitBootServices callback.
> + // This driver does not produce any protocol services, so always unload it.
> //
> - return EFI_SUCCESS;
> + return EFI_REQUEST_UNLOAD_IMAGE;
> }
> diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
> index e4b97ef1df..cb8e2e7f0a 100644
> --- a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
> +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c
> @@ -10,7 +10,7 @@
> integer number will be added to the end of the variable name. This number
> will be incremented for each variable as needed to store the entire data set.
>
> - Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -245,7 +245,7 @@ Done:
> @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
> @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> @retval EFI_WRITE_PROTECTED The variable in question cannot be deleted.
> -
> + @retval EFI_ABORTED LockVariable was requested but failed.
> @retval EFI_NOT_FOUND The variable trying to be updated or deleted was not found.
>
> **/
> @@ -323,6 +323,13 @@ SetLargeVariable (
> }
> if (LockVariable) {
> Status = VarLibVariableRequestToLock (VariableName, VendorGuid);
> + if (EFI_ERROR (Status)) {
> + //
> + // Do not delete Variable when failed to lock. Caller is responsible to do this.
> + //
> + DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error locking variable: Status = %r\n", Status));
> + Status = EFI_ABORTED;
> + }
> }
> } else {
> //
> @@ -402,7 +409,7 @@ SetLargeVariable (
> DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error writting variable: Status = %r\n", Status));
> goto Done;
> }
> - VariablesSaved = Index;
> + VariablesSaved ++;
No space for unary operators. Should be VariableSaved++;
> BytesRemaining -= SizeToSave;
> OffsetPtr += SizeToSave;
> } // End of for loop
> @@ -420,6 +427,10 @@ SetLargeVariable (
> Status = VarLibVariableRequestToLock (TempVariableName, VendorGuid);
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error locking variable: Status = %r\n", Status));
> + //
> + // Do not delete Variable when failed to lock. Caller is responsible to do this.
> + //
> + Status = EFI_ABORTED;
> VariablesSaved = 0;
> goto Done;
> }
> @@ -442,9 +453,132 @@ Done:
> 0,
> NULL
> );
> - DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error deleting variable: Status = %r\n", Status2));
> + if (EFI_ERROR (Status2)) {
> + DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error deleting variable: Status = %r\n", Status2));
> + }
> }
> }
> DEBUG ((DEBUG_ERROR, "SetLargeVariable: Status = %r\n", Status));
> return Status;
> }
> +
> +/**
> + Locks the existing large variable.
> +
> + @param[in] VariableName A Null-terminated string that is the name of the vendor's variable.
> + Each VariableName is unique for each VendorGuid. VariableName must
> + contain 1 or more characters. If VariableName is an empty string,
> + then EFI_INVALID_PARAMETER is returned.
> + @param[in] VendorGuid A unique identifier for the vendor.
> + @retval EFI_SUCCESS The firmware has successfully locked the variable.
> + @retval EFI_INVALID_PARAMETER An invalid combination of variable name and GUID was supplied
> + @retval EFI_OUT_OF_RESOURCES The VariableName is longer than 1018 characters
> + @retval EFI_UNSUPPORTED The service for locking variable is not ready.
> + @retval EFI_NOT_FOUND The targeting variable for locking is not present.
> + @retval EFI_ABORTED Fail to lock variable.
> +**/
> +EFI_STATUS
> +EFIAPI
> +LockLargeVariable (
> + IN CHAR16 *VariableName,
> + IN EFI_GUID *VendorGuid
> + )
> +{
> + CHAR16 TempVariableName[MAX_VARIABLE_NAME_SIZE];
> + UINT64 VariableSize;
> + EFI_STATUS Status;
> + UINTN Index;
> +
> + //
> + // Check input parameters.
> + //
> + if (VariableName == NULL || VariableName[0] == 0 || VendorGuid == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + //
> + // Check the length of the variable name is short enough to allow an integer
> + // to be appended.
> + //
> + if (StrLen (VariableName) >= (MAX_VARIABLE_NAME_SIZE - MAX_VARIABLE_SPLIT_DIGITS)) {
> + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Variable name too long\n"));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + if (!VarLibIsVariableRequestToLockSupported ()) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + //
> + // Check if it is single variable scenario.
> + //
> + VariableSize = 0;
> + Status = VarLibGetVariable (VariableName, VendorGuid, NULL, &VariableSize, NULL);
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> + //
> + // Lock single variable.
> + //
> + DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", VariableName, VendorGuid));
> + Status = VarLibVariableRequestToLock (VariableName, VendorGuid);
> + if (EFI_ERROR (Status)) {
> + //
> + // Do not delete Variable when failed to lock. Caller is responsible to do this.
> + //
> + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n", Status));
> + return EFI_ABORTED;
> + }
> + return EFI_SUCCESS;
> + } else {
> + //
> + // Check if it is multiple variables scenario.
> + //
> + Index = 0;
> + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
> + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", VariableName, Index);
> + VariableSize = 0;
> + Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL, &VariableSize, NULL);
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> + //
> + // Lock first variable and continue to rest of the variables.
> + //
> + DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", TempVariableName, VendorGuid));
> + Status = VarLibVariableRequestToLock (TempVariableName, VendorGuid);
> + if (EFI_ERROR (Status)) {
> + //
> + // Do not delete Variable when failed to lock. Caller is responsible to do this.
> + //
> + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n", Status));
> + return EFI_ABORTED;
> + }
> + for (Index = 1; Index < MAX_VARIABLE_SPLIT; Index++) {
> + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
> + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", VariableName, Index);
> +
> + VariableSize = 0;
> + Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL, &VariableSize, NULL);
> + if (Status == EFI_BUFFER_TOO_SMALL) {
> + DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", TempVariableName, VendorGuid));
> + Status = VarLibVariableRequestToLock (TempVariableName, VendorGuid);
> + if (EFI_ERROR (Status)) {
> + //
> + // Do not delete Variable when failed to lock. Caller is responsible to do this.
> + //
> + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n", Status));
> + return EFI_ABORTED;
> + }
> + } else if (Status == EFI_NOT_FOUND) {
> + //
> + // No more variables need to lock.
> + //
> + return EFI_SUCCESS;
> + }
> + } // End of for loop
> + }
> + }
> +
> + //
> + // Here probably means variable not present.
> + //
> + return Status;
> +
> +}
> diff --git a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
> index 9ed59f8827..28730f858b 100644
> --- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
> +++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
> @@ -10,7 +10,7 @@
> Using this library allows code to be written in a generic manner that can be
> used in DXE or SMM without modification.
>
> - Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -18,14 +18,16 @@
> #include <Uefi.h>
>
> #include <Guid/EventGroup.h>
> -#include <Protocol/VariableLock.h>
> +#include <Library/VariablePolicyHelperLib.h>
>
> #include <Library/UefiLib.h>
> #include <Library/DebugLib.h>
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiRuntimeServicesTableLib.h>
>
> -STATIC EDKII_VARIABLE_LOCK_PROTOCOL *mVariableWriteLibVariableLock = NULL;
> +STATIC EDKII_VARIABLE_POLICY_PROTOCOL *mVariableWriteLibVariablePolicy = NULL;
> +EFI_EVENT mExitBootServiceEvent;
> +EFI_EVENT mLegacyBootEvent;
>
> /**
> Sets the value of a variable.
> @@ -144,7 +146,7 @@ VarLibIsVariableRequestToLockSupported (
> VOID
> )
> {
> - if (mVariableWriteLibVariableLock != NULL) {
> + if (mVariableWriteLibVariablePolicy != NULL) {
> return TRUE;
> } else {
> return FALSE;
> @@ -178,16 +180,45 @@ VarLibVariableRequestToLock (
> {
> EFI_STATUS Status = EFI_UNSUPPORTED;
>
> - if (mVariableWriteLibVariableLock != NULL) {
> - Status = mVariableWriteLibVariableLock->RequestToLock (
> - mVariableWriteLibVariableLock,
> - VariableName,
> - VendorGuid
> - );
> + if (mVariableWriteLibVariablePolicy != NULL) {
> + Status = RegisterBasicVariablePolicy (
> + mVariableWriteLibVariablePolicy,
> + (CONST EFI_GUID*) VendorGuid,
> + (CONST CHAR16 *) VariableName,
> + 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
> + );
> }
> return Status;
> }
>
> +/**
> + Close events when driver unloaded.
> +
> + @param[in] ImageHandle A handle for the image that is initializing this driver
> + @param[in] SystemTable A pointer to the EFI system table
> +
> + @retval EFI_SUCCESS The initialization finished successfully.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeRuntimeVariableWriteLibDestructor (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + if (mExitBootServiceEvent != 0) {
> + gBS->CloseEvent (mExitBootServiceEvent);
> + }
> + if (mLegacyBootEvent != 0) {
> + gBS->CloseEvent (mLegacyBootEvent);
> + }
> + return EFI_SUCCESS;
> +}
> +
> /**
> Exit Boot Services Event notification handler.
>
> @@ -202,7 +233,7 @@ DxeRuntimeVariableWriteLibOnExitBootServices (
> IN VOID *Context
> )
> {
> - mVariableWriteLibVariableLock = NULL;
> + mVariableWriteLibVariablePolicy = NULL;
> }
>
> /**
> @@ -227,13 +258,11 @@ DxeRuntimeVariableWriteLibConstructor (
> )
> {
> EFI_STATUS Status;
> - EFI_EVENT ExitBootServiceEvent;
> - EFI_EVENT LegacyBootEvent;
>
> //
> // Locate VariableLockProtocol.
> //
> - Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL, (VOID **)&mVariableWriteLibVariableLock);
> + Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NULL, (VOID **)&mVariableWriteLibVariablePolicy);
> ASSERT_EFI_ERROR (Status);
>
> //
> @@ -245,7 +274,7 @@ DxeRuntimeVariableWriteLibConstructor (
> DxeRuntimeVariableWriteLibOnExitBootServices,
> NULL,
> &gEfiEventExitBootServicesGuid,
> - &ExitBootServiceEvent
> + &mExitBootServiceEvent
> );
> ASSERT_EFI_ERROR (Status);
>
> @@ -257,7 +286,7 @@ DxeRuntimeVariableWriteLibConstructor (
> TPL_NOTIFY,
> DxeRuntimeVariableWriteLibOnExitBootServices,
> NULL,
> - &LegacyBootEvent
> + &mLegacyBootEvent
> );
> ASSERT_EFI_ERROR (Status);
>
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
> index e2dbd2fb49..61e85a6586 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConfig.inf
> @@ -1,7 +1,7 @@
> ### @file
> # Component information file for SaveMemoryConfig module
> #
> -# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -25,6 +25,7 @@
> BaseMemoryLib
> LargeVariableReadLib
> LargeVariableWriteLib
> + BaseLib
>
> [Packages]
> MdePkg/MdePkg.dec
> diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> index c847d7f152..1b9370685a 100644
> --- a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> +++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> @@ -16,7 +16,7 @@
> is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
> solution to this problem.
>
> - Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -52,7 +52,7 @@
> @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a hardware error.
> @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> @retval EFI_WRITE_PROTECTED The variable in question cannot be deleted.
> -
> + @retval EFI_ABORTED LockVariable was requested but failed.
> @retval EFI_NOT_FOUND The variable trying to be updated or deleted was not found.
>
> **/
> @@ -66,4 +66,26 @@ SetLargeVariable (
> IN VOID *Data
> );
>
> +/**
> + Locks the existing large variable.
> +
> + @param[in] VariableName A Null-terminated string that is the name of the vendor's variable.
> + Each VariableName is unique for each VendorGuid. VariableName must
> + contain 1 or more characters. If VariableName is an empty string,
> + then EFI_INVALID_PARAMETER is returned.
> + @param[in] VendorGuid A unique identifier for the vendor.
> + @retval EFI_SUCCESS The firmware has successfully locked the variable.
> + @retval EFI_INVALID_PARAMETER An invalid combination of variable name and GUID was supplied
> + @retval EFI_OUT_OF_RESOURCES The VariableName is longer than 1018 characters
> + @retval EFI_UNSUPPORTED The service for locking variable is not ready.
> + @retval EFI_NOT_FOUND The targeting variable for locking is not present.
> + @retval EFI_ABORTED Fail to lock variable.
> +**/
> +EFI_STATUS
> +EFIAPI
> +LockLargeVariable (
> + IN CHAR16 *VariableName,
> + IN EFI_GUID *VendorGuid
> + );
> +
> #endif // _LARGE_VARIABLE_WRITE_LIB_H_
> diff --git a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
> index 704a8ac7cc..f83090c847 100644
> --- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
> @@ -10,7 +10,7 @@
> # Using this library allows code to be written in a generic manner that can be
> # used in DXE or SMM without modification.
> #
> -# Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -24,6 +24,7 @@
> MODULE_TYPE = DXE_RUNTIME_DRIVER
> LIBRARY_CLASS = VariableWriteLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
> CONSTRUCTOR = DxeRuntimeVariableWriteLibConstructor
> + DESTRUCTOR = DxeRuntimeVariableWriteLibDestructor
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -37,13 +38,14 @@
> UefiLib
> UefiBootServicesTableLib
> UefiRuntimeServicesTableLib
> + VariablePolicyHelperLib
>
> [Guids]
> gEfiEventExitBootServicesGuid ## CONSUMES ## Event
>
> [Protocols]
> gEfiVariableWriteArchProtocolGuid ## CONSUMES
> - gEdkiiVariableLockProtocolGuid ## CONSUMES
> + gEdkiiVariablePolicyProtocolGuid ## CONSUMES
Please re-align the columns here
>
> [Depex]
> - gEfiVariableWriteArchProtocolGuid AND gEdkiiVariableLockProtocolGuid
> + gEfiVariableWriteArchProtocolGuid AND gEdkiiVariablePolicyProtocolGuid
> --
> 2.28.0.windows.1
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#86723): https://edk2.groups.io/g/devel/message/86723
> Mute This Topic: https://groups.io/mt/89204326/1767664
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [nathaniel.l.desimone@intel.com] -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86736): https://edk2.groups.io/g/devel/message/86736
Mute This Topic: https://groups.io/mt/89204326/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Thanks Nate, Isaac for good catch! I will correct them before pushing.
> -----Original Message-----
> From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Sent: Friday, February 18, 2022 7:22 AM
> To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@intel.com>
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric
> <eric.dong@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>
> Subject: RE: [edk2-devel] [edk2-platforms: PATCH v4]
> MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
>
> Hi Chasel,
>
> There are a couple of typos and minor code style issues pointed out below.
> Please fix those during the push... no need to send another patch.
>
> Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
>
> Thanks,
> Nate
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu,
> > Chasel
> > Sent: Wednesday, February 16, 2022 10:21 PM
> > To: devel@edk2.groups.io
> > Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desimone@intel.com>; Gao, Liming
> > <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>; Oram,
> > Isaac W <isaac.w.oram@intel.com>
> > Subject: [edk2-devel] [edk2-platforms: PATCH v4]
> > MinPlatformPkg/SaveMemoryConfig: Variable may not be locked.
> >
> > From: "Chiu, Chasel" <chasel.chiu@intel.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3829
> >
> > Fixed the bug that existing variable will not be locked when it is
> > identical with hob data by creating LockLargeVariable function, also
> > switched to VariablePolicyProtocol for locking variables.
> >
> > Failing to lock variable could be security vulnerability, so the
> > LargeVariableWriteLib functions will return EFI_ABORTED if locking was
> > failed and SaveMemoryConfig driver will delete variable to prevent
> > from using unlocked variable.
> >
> > This patch also modified SaveMemoryConfig driver to be unloaded after
> > execution because it does not produce any service protocol. To achieve
> > this goal the DxeRuntimeVariableWriteLib should close registered
> > ExitBootService events in its DESTRUCTOR.
> >
> > Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> > Reviewed-by: Isaac Oram <isaac.w.oram@intel.com> ---Patch V4:
> > 1.Removed CpuDeadLoop and delete variable if Lock failed in
> SaveMemoryConfig. 2. LargeVariableWrite will not delete variable if Lock failed,
> let caller to do it.
> >
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> onfig.c | 37 ++++++++++++++++++++++++++++++++++---
> >
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWri
> teLib.c | 142
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++----
> >
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunti
> meVariableWriteLib.c | 61
> +++++++++++++++++++++++++++++++++++++++++++++----------------
> >
> Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryC
> onfig.inf | 3 ++-
> > Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> | 26 ++++++++++++++++++++++++--
> >
> Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunti
> meVariableWriteLib.inf | 8 +++++---
> > 6 files changed, 248 insertions(+), 29 deletions(-)
> >
> > diff --git
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.c
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.c
> > index 820585f676..fb51edd5cb 100644
> > ---
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.c
> > +++
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMe
> > +++ moryConfig.c
> > @@ -2,13 +2,14 @@
> > This is the driver that locates the MemoryConfigurationData HOB, if it
> > exists, and saves the data to nvRAM.
> >
> > -Copyright (c) 2017 - 2021, Intel Corporation. All rights
> > reserved.<BR>
> > +Copyright (c) 2017 - 2022, Intel Corporation. All rights
> > +reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> >
> > #include <Base.h>
> > #include <Uefi.h>
> > +#include <Library/BaseLib.h>
> > #include <Library/UefiBootServicesTableLib.h>
> > #include <Library/UefiRuntimeServicesTableLib.h>
> > #include <Library/HobLib.h>
> > @@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > #include <Library/BaseMemoryLib.h> #include
> > <Library/LargeVariableReadLib.h> #include
> > <Library/LargeVariableWriteLib.h>
> > +#include <Library/VariableWriteLib.h>
> > #include <Guid/FspNonVolatileStorageHob2.h>
> >
> > /**
> > @@ -86,6 +88,23 @@ SaveMemoryConfigEntryPoint (
> > Status = GetLargeVariable (L"FspNvsBuffer",
> &gFspNvsBufferVariableGuid, &BufferSize, VariableData);
> > if (!EFI_ERROR (Status) && (BufferSize == DataSize) && (0 ==
> CompareMem (HobData, VariableData, DataSize))) {
> > DataIsIdentical = TRUE;
> > + //
> > + // No need to update Variable, only lock it.
> > + //
> > + Status = LockLargeVariable (L"FspNvsBuffer",
> &gFspNvsBufferVariableGuid);
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Fail to lock variable is security vulnerability and should not happen.
> > + //
> > + ASSERT_EFI_ERROR (Status);
> > + //
> > + // When building without ASSERT_EFI_ERROR hang, delete the
> varialbe so it will not be consumed.
>
> Typo here, "varialbe" should be "variable".
>
> > + //
> > + DEBUG ((DEBUG_ERROR, "Delete variable!\n"));
> > + DataSize = 0;
> > + Status = SetLargeVariable (L"FspNvsBuffer",
> &gFspNvsBufferVariableGuid, FALSE, DataSize, HobData);
> > + ASSERT_EFI_ERROR (Status);
> > + }
> > }
> > FreePool (VariableData);
> > }
> > @@ -95,6 +114,18 @@ SaveMemoryConfigEntryPoint (
> >
> > if (!DataIsIdentical) {
> > Status = SetLargeVariable (L"FspNvsBuffer",
> > &gFspNvsBufferVariableGuid, TRUE, DataSize, HobData);
> > + if (Status == EFI_ABORTED) {
> > + //
> > + // Fail to lock variable! This should not happen.
> > + //
> > + ASSERT_EFI_ERROR (Status);
> > + //
> > + // When building without ASSERT_EFI_ERROR hang, delete the varialbe
> so it will not be consumed.
>
> Typo here, "varialbe" should be "variable".
>
> > + //
> > + DEBUG ((DEBUG_ERROR, "Delete variable!\n"));
> > + DataSize = 0;
> > + Status = SetLargeVariable (L"FspNvsBuffer",
> &gFspNvsBufferVariableGuid, FALSE, DataSize, HobData);
> > + }
> > ASSERT_EFI_ERROR (Status);
> > DEBUG ((DEBUG_INFO, "Saved size of FSP / MRC Training Data: 0x%x\n",
> DataSize));
> > } else {
> > @@ -106,7 +137,7 @@ SaveMemoryConfigEntryPoint (
> > }
> >
> > //
> > - // This driver cannot be unloaded because DxeRuntimeVariableWriteLib
> constructor will register ExitBootServices callback.
> > + // This driver does not produce any protocol services, so always unload it.
> > //
> > - return EFI_SUCCESS;
> > + return EFI_REQUEST_UNLOAD_IMAGE;
> > }
> > diff --git
> > a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVari
> > ableWriteLib.c
> > b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVari
> > ableWriteLib.c
> > index e4b97ef1df..cb8e2e7f0a 100644
> > ---
> > a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVari
> > ableWriteLib.c
> > +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/Large
> > +++ VariableWriteLib.c
> > @@ -10,7 +10,7 @@
> > integer number will be added to the end of the variable name. This number
> > will be incremented for each variable as needed to store the entire data set.
> >
> > - Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2021 - 2022, Intel Corporation. All rights
> > + reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -245,7 +245,7 @@ Done:
> > @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a
> hardware error.
> > @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> > @retval EFI_WRITE_PROTECTED The variable in question cannot be
> deleted.
> > -
> > + @retval EFI_ABORTED LockVariable was requested but failed.
> > @retval EFI_NOT_FOUND The variable trying to be updated or deleted
> was not found.
> >
> > **/
> > @@ -323,6 +323,13 @@ SetLargeVariable (
> > }
> > if (LockVariable) {
> > Status = VarLibVariableRequestToLock (VariableName,
> > VendorGuid);
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Do not delete Variable when failed to lock. Caller is responsible to do
> this.
> > + //
> > + DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error locking variable:
> Status = %r\n", Status));
> > + Status = EFI_ABORTED;
> > + }
> > }
> > } else {
> > //
> > @@ -402,7 +409,7 @@ SetLargeVariable (
> > DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error writting variable:
> Status = %r\n", Status));
> > goto Done;
> > }
> > - VariablesSaved = Index;
> > + VariablesSaved ++;
>
> No space for unary operators. Should be VariableSaved++;
>
> > BytesRemaining -= SizeToSave;
> > OffsetPtr += SizeToSave;
> > } // End of for loop
> > @@ -420,6 +427,10 @@ SetLargeVariable (
> > Status = VarLibVariableRequestToLock (TempVariableName, VendorGuid);
> > if (EFI_ERROR (Status)) {
> > DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error locking
> > variable: Status = %r\n", Status));
> > + //
> > + // Do not delete Variable when failed to lock. Caller is responsible to do
> this.
> > + //
> > + Status = EFI_ABORTED;
> > VariablesSaved = 0;
> > goto Done;
> > }
> > @@ -442,9 +453,132 @@ Done:
> > 0,
> > NULL
> > );
> > - DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error deleting variable: Status
> = %r\n", Status2));
> > + if (EFI_ERROR (Status2)) {
> > + DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error deleting variable:
> Status = %r\n", Status2));
> > + }
> > }
> > }
> > DEBUG ((DEBUG_ERROR, "SetLargeVariable: Status = %r\n", Status));
> > return Status;
> > }
> > +
> > +/**
> > + Locks the existing large variable.
> > +
> > + @param[in] VariableName A Null-terminated string that is the name of
> the vendor's variable.
> > + Each VariableName is unique for each VendorGuid.
> VariableName must
> > + contain 1 or more characters. If VariableName is an empty
> string,
> > + then EFI_INVALID_PARAMETER is returned.
> > + @param[in] VendorGuid A unique identifier for the vendor.
> > + @retval EFI_SUCCESS The firmware has successfully locked the
> variable.
> > + @retval EFI_INVALID_PARAMETER An invalid combination of variable name
> and GUID was supplied
> > + @retval EFI_OUT_OF_RESOURCES The VariableName is longer than 1018
> characters
> > + @retval EFI_UNSUPPORTED The service for locking variable is not ready.
> > + @retval EFI_NOT_FOUND The targeting variable for locking is not
> present.
> > + @retval EFI_ABORTED Fail to lock variable.
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +LockLargeVariable (
> > + IN CHAR16 *VariableName,
> > + IN EFI_GUID *VendorGuid
> > + )
> > +{
> > + CHAR16 TempVariableName[MAX_VARIABLE_NAME_SIZE];
> > + UINT64 VariableSize;
> > + EFI_STATUS Status;
> > + UINTN Index;
> > +
> > + //
> > + // Check input parameters.
> > + //
> > + if (VariableName == NULL || VariableName[0] == 0 || VendorGuid == NULL)
> {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + //
> > + // Check the length of the variable name is short enough to allow
> > + an integer // to be appended.
> > + //
> > + if (StrLen (VariableName) >= (MAX_VARIABLE_NAME_SIZE -
> MAX_VARIABLE_SPLIT_DIGITS)) {
> > + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Variable name too long\n"));
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + if (!VarLibIsVariableRequestToLockSupported ()) {
> > + return EFI_UNSUPPORTED;
> > + }
> > +
> > + //
> > + // Check if it is single variable scenario.
> > + //
> > + VariableSize = 0;
> > + Status = VarLibGetVariable (VariableName, VendorGuid, NULL,
> > + &VariableSize, NULL); if (Status == EFI_BUFFER_TOO_SMALL) {
> > + //
> > + // Lock single variable.
> > + //
> > + DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", VariableName,
> VendorGuid));
> > + Status = VarLibVariableRequestToLock (VariableName, VendorGuid);
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Do not delete Variable when failed to lock. Caller is responsible to do
> this.
> > + //
> > + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n",
> Status));
> > + return EFI_ABORTED;
> > + }
> > + return EFI_SUCCESS;
> > + } else {
> > + //
> > + // Check if it is multiple variables scenario.
> > + //
> > + Index = 0;
> > + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
> > + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d",
> VariableName, Index);
> > + VariableSize = 0;
> > + Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL,
> &VariableSize, NULL);
> > + if (Status == EFI_BUFFER_TOO_SMALL) {
> > + //
> > + // Lock first variable and continue to rest of the variables.
> > + //
> > + DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", TempVariableName,
> VendorGuid));
> > + Status = VarLibVariableRequestToLock (TempVariableName, VendorGuid);
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Do not delete Variable when failed to lock. Caller is responsible to do
> this.
> > + //
> > + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n",
> Status));
> > + return EFI_ABORTED;
> > + }
> > + for (Index = 1; Index < MAX_VARIABLE_SPLIT; Index++) {
> > + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE);
> > + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE,
> > + L"%s%d", VariableName, Index);
> > +
> > + VariableSize = 0;
> > + Status = VarLibGetVariable (TempVariableName, VendorGuid, NULL,
> &VariableSize, NULL);
> > + if (Status == EFI_BUFFER_TOO_SMALL) {
> > + DEBUG ((DEBUG_INFO, "Locking %s, Guid = %g\n", TempVariableName,
> VendorGuid));
> > + Status = VarLibVariableRequestToLock (TempVariableName,
> VendorGuid);
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // Do not delete Variable when failed to lock. Caller is responsible to
> do this.
> > + //
> > + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus = %r\n",
> Status));
> > + return EFI_ABORTED;
> > + }
> > + } else if (Status == EFI_NOT_FOUND) {
> > + //
> > + // No more variables need to lock.
> > + //
> > + return EFI_SUCCESS;
> > + }
> > + } // End of for loop
> > + }
> > + }
> > +
> > + //
> > + // Here probably means variable not present.
> > + //
> > + return Status;
> > +
> > +}
> > diff --git
> > a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/Dxe
> > RuntimeVariableWriteLib.c
> > b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/Dxe
> > RuntimeVariableWriteLib.c
> > index 9ed59f8827..28730f858b 100644
> > ---
> > a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/Dxe
> > RuntimeVariableWriteLib.c
> > +++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib
> > +++ /DxeRuntimeVariableWriteLib.c
> > @@ -10,7 +10,7 @@
> > Using this library allows code to be written in a generic manner that can be
> > used in DXE or SMM without modification.
> >
> > - Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2021 - 2022, Intel Corporation. All rights
> > + reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -18,14 +18,16 @@
> > #include <Uefi.h>
> >
> > #include <Guid/EventGroup.h>
> > -#include <Protocol/VariableLock.h>
> > +#include <Library/VariablePolicyHelperLib.h>
> >
> > #include <Library/UefiLib.h>
> > #include <Library/DebugLib.h>
> > #include <Library/UefiBootServicesTableLib.h>
> > #include <Library/UefiRuntimeServicesTableLib.h>
> >
> > -STATIC EDKII_VARIABLE_LOCK_PROTOCOL *mVariableWriteLibVariableLock
> =
> > NULL;
> > +STATIC EDKII_VARIABLE_POLICY_PROTOCOL
> *mVariableWriteLibVariablePolicy = NULL;
> > +EFI_EVENT mExitBootServiceEvent;
> > +EFI_EVENT mLegacyBootEvent;
> >
> > /**
> > Sets the value of a variable.
> > @@ -144,7 +146,7 @@ VarLibIsVariableRequestToLockSupported (
> > VOID
> > )
> > {
> > - if (mVariableWriteLibVariableLock != NULL) {
> > + if (mVariableWriteLibVariablePolicy != NULL) {
> > return TRUE;
> > } else {
> > return FALSE;
> > @@ -178,16 +180,45 @@ VarLibVariableRequestToLock ( {
> > EFI_STATUS Status = EFI_UNSUPPORTED;
> >
> > - if (mVariableWriteLibVariableLock != NULL) {
> > - Status = mVariableWriteLibVariableLock->RequestToLock (
> > - mVariableWriteLibVariableLock,
> > - VariableName,
> > - VendorGuid
> > - );
> > + if (mVariableWriteLibVariablePolicy != NULL) {
> > + Status = RegisterBasicVariablePolicy (
> > + mVariableWriteLibVariablePolicy,
> > + (CONST EFI_GUID*) VendorGuid,
> > + (CONST CHAR16 *) VariableName,
> > + 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
> > + );
> > }
> > return Status;
> > }
> >
> > +/**
> > + Close events when driver unloaded.
> > +
> > + @param[in] ImageHandle A handle for the image that is initializing
> > + this driver @param[in] SystemTable A pointer to the EFI system
> > + table
> > +
> > + @retval EFI_SUCCESS The initialization finished successfully.
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +DxeRuntimeVariableWriteLibDestructor (
> > + IN EFI_HANDLE ImageHandle,
> > + IN EFI_SYSTEM_TABLE *SystemTable
> > + )
> > +{
> > + if (mExitBootServiceEvent != 0) {
> > + gBS->CloseEvent (mExitBootServiceEvent);
> > + }
> > + if (mLegacyBootEvent != 0) {
> > + gBS->CloseEvent (mLegacyBootEvent);
> > + }
> > + return EFI_SUCCESS;
> > +}
> > +
> > /**
> > Exit Boot Services Event notification handler.
> >
> > @@ -202,7 +233,7 @@ DxeRuntimeVariableWriteLibOnExitBootServices (
> > IN VOID *Context
> > )
> > {
> > - mVariableWriteLibVariableLock = NULL;
> > + mVariableWriteLibVariablePolicy = NULL;
> > }
> >
> > /**
> > @@ -227,13 +258,11 @@ DxeRuntimeVariableWriteLibConstructor (
> > )
> > {
> > EFI_STATUS Status;
> > - EFI_EVENT ExitBootServiceEvent;
> > - EFI_EVENT LegacyBootEvent;
> >
> > //
> > // Locate VariableLockProtocol.
> > //
> > - Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid,
> > NULL, (VOID **)&mVariableWriteLibVariableLock);
> > + Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid,
> > + NULL, (VOID **)&mVariableWriteLibVariablePolicy);
> > ASSERT_EFI_ERROR (Status);
> >
> > //
> > @@ -245,7 +274,7 @@ DxeRuntimeVariableWriteLibConstructor (
> > DxeRuntimeVariableWriteLibOnExitBootServices,
> > NULL,
> > &gEfiEventExitBootServicesGuid,
> > - &ExitBootServiceEvent
> > + &mExitBootServiceEvent
> > );
> > ASSERT_EFI_ERROR (Status);
> >
> > @@ -257,7 +286,7 @@ DxeRuntimeVariableWriteLibConstructor (
> > TPL_NOTIFY,
> > DxeRuntimeVariableWriteLibOnExitBootServices,
> > NULL,
> > - &LegacyBootEvent
> > + &mLegacyBootEvent
> > );
> > ASSERT_EFI_ERROR (Status);
> >
> > diff --git
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.inf
> >
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.inf
> > index e2dbd2fb49..61e85a6586 100644
> > ---
> >
> a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor
> y
> > Config.inf
> > +++
> b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMe
> > +++ moryConfig.inf
> > @@ -1,7 +1,7 @@
> > ### @file
> > # Component information file for SaveMemoryConfig module # -#
> > Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2017 - 2022, Intel Corporation. All rights
> > +reserved.<BR>
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -25,6 +25,7 @@
> > BaseMemoryLib
> > LargeVariableReadLib
> > LargeVariableWriteLib
> > + BaseLib
> >
> > [Packages]
> > MdePkg/MdePkg.dec
> > diff --git
> a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> > index c847d7f152..1b9370685a 100644
> > --- a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> > +++
> b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
> > @@ -16,7 +16,7 @@
> > is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
> > solution to this problem.
> >
> > - Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > **/
> > @@ -52,7 +52,7 @@
> > @retval EFI_DEVICE_ERROR The variable could not be retrieved due to a
> hardware error.
> > @retval EFI_WRITE_PROTECTED The variable in question is read-only.
> > @retval EFI_WRITE_PROTECTED The variable in question cannot be
> deleted.
> > -
> > + @retval EFI_ABORTED LockVariable was requested but failed.
> > @retval EFI_NOT_FOUND The variable trying to be updated or deleted
> was not found.
> >
> > **/
> > @@ -66,4 +66,26 @@ SetLargeVariable (
> > IN VOID *Data
> > );
> >
> > +/**
> > + Locks the existing large variable.
> > +
> > + @param[in] VariableName A Null-terminated string that is the name of
> the vendor's variable.
> > + Each VariableName is unique for each VendorGuid.
> VariableName must
> > + contain 1 or more characters. If VariableName is an empty
> string,
> > + then EFI_INVALID_PARAMETER is returned.
> > + @param[in] VendorGuid A unique identifier for the vendor.
> > + @retval EFI_SUCCESS The firmware has successfully locked the
> variable.
> > + @retval EFI_INVALID_PARAMETER An invalid combination of variable name
> and GUID was supplied
> > + @retval EFI_OUT_OF_RESOURCES The VariableName is longer than 1018
> characters
> > + @retval EFI_UNSUPPORTED The service for locking variable is not ready.
> > + @retval EFI_NOT_FOUND The targeting variable for locking is not
> present.
> > + @retval EFI_ABORTED Fail to lock variable.
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +LockLargeVariable (
> > + IN CHAR16 *VariableName,
> > + IN EFI_GUID *VendorGuid
> > + );
> > +
> > #endif // _LARGE_VARIABLE_WRITE_LIB_H_
> > diff --git
> a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRun
> timeVariableWriteLib.inf
> b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRun
> timeVariableWriteLib.inf
> > index 704a8ac7cc..f83090c847 100644
> > ---
> a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRun
> timeVariableWriteLib.inf
> > +++
> b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRun
> timeVariableWriteLib.inf
> > @@ -10,7 +10,7 @@
> > # Using this library allows code to be written in a generic manner that can be
> > # used in DXE or SMM without modification.
> > #
> > -# Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.<BR>
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > @@ -24,6 +24,7 @@
> > MODULE_TYPE = DXE_RUNTIME_DRIVER
> > LIBRARY_CLASS = VariableWriteLib|DXE_CORE DXE_DRIVER
> DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
> > CONSTRUCTOR = DxeRuntimeVariableWriteLibConstructor
> > + DESTRUCTOR = DxeRuntimeVariableWriteLibDestructor
> >
> > [Packages]
> > MdePkg/MdePkg.dec
> > @@ -37,13 +38,14 @@
> > UefiLib
> > UefiBootServicesTableLib
> > UefiRuntimeServicesTableLib
> > + VariablePolicyHelperLib
> >
> > [Guids]
> > gEfiEventExitBootServicesGuid ## CONSUMES ## Event
> >
> > [Protocols]
> > gEfiVariableWriteArchProtocolGuid ## CONSUMES
> > - gEdkiiVariableLockProtocolGuid ## CONSUMES
> > + gEdkiiVariablePolicyProtocolGuid ## CONSUMES
>
> Please re-align the columns here
>
> >
> > [Depex]
> > - gEfiVariableWriteArchProtocolGuid AND gEdkiiVariableLockProtocolGuid
> > + gEfiVariableWriteArchProtocolGuid AND gEdkiiVariablePolicyProtocolGuid
> > --
> > 2.28.0.windows.1
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#86723): https://edk2.groups.io/g/devel/message/86723
> > Mute This Topic: https://groups.io/mt/89204326/1767664
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [nathaniel.l.desimone@intel.com] -=-=-=-=-=-=
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86737): https://edk2.groups.io/g/devel/message/86737
Mute This Topic: https://groups.io/mt/89204326/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.