Re: [edk2-devel] [PATCH 2/3] MdeModulePkg: Refactors SmmLockBox.c.
Reviewed-by: Jiaxin Wu
> -Original Message-
> From: Xie, Yuanhao
> Sent: Tuesday, May 7, 2024 2:09 PM
> To: devel@edk2.groups.io
> Cc: Liming Gao ; Wu, Jiaxin
> ; Ni, Ray ; Xie, Yuanhao
>
> Subject: [PATCH 2/3] MdeModulePkg: Refactors SmmLockBox.c.
>
> The Lockbox Driver allows sensitive data to be securely stored in a
> designated area, thus protected against unauthorized access.
>
> This patch does not introduce any functional modifications.
> It refactors the existing logic into a common component to facilitates
> the integration of the Standalone MM Lockbox Driver in an upcoming patch
>
> Cc: Liming Gao
> Cc: Jiaxin Wu
> Cc: Ray Ni
>
> Signed-off-by: Yuanhao Xie
> ---
> MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c | 361
> ---
> --
> --
> --
> MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf | 4
> +++-
> MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.c |
> 384
> ++
> ++
> ++
> ++
> ++
> ++
>
> MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.h |
> 148
> ++
> ++
>
> 4 files changed, 547 insertions(+), 350 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> index c1e15c596b..2774979c34 100644
> --- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> +++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> @@ -9,7 +9,7 @@
>SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(),
> SmmLockBoxSave()
>will receive untrusted input and do basic validation.
>
> -Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -31,360 +31,24 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include
> #include
>
> -BOOLEAN mLocked = FALSE;
> +#include "SmmLockBoxCommon.h"
>
> /**
> - Dispatch function for SMM lock box save.
> + This function is an abstraction layer for implementation specific Mm buffer
> validation routine.
>
> - Caution: This function may receive untrusted input.
> - Restore buffer and length are external input, so this function will
> validate
> - it is in SMRAM.
> + @param Buffer The buffer start address to be checked.
> + @param Length The buffer length to be checked.
>
> - @param LockBoxParameterSave parameter of lock box save
> + @retval TRUE This buffer is valid per processor architecture and not
> overlap
> with SMRAM.
> + @retval FALSE This buffer is not valid per processor architecture or
> overlap
> with SMRAM.
> **/
> -VOID
> -SmmLockBoxSave (
> - IN EFI_SMM_LOCK_BOX_PARAMETER_SAVE *LockBoxParameterSave
> +BOOLEAN
> +IsBufferOutsideMmValid (
> + IN EFI_PHYSICAL_ADDRESS Buffer,
> + IN UINT64Length
>)
> {
> - EFI_STATUS Status;
> - EFI_SMM_LOCK_BOX_PARAMETER_SAVE TempLockBoxParameterSave;
> -
> - //
> - // Sanity check
> - //
> - if (mLocked) {
> -DEBUG ((DEBUG_ERROR, "SmmLockBox Locked!\n"));
> -LockBoxParameterSave->Header.ReturnStatus =
> (UINT64)EFI_ACCESS_DENIED;
> -return;
> - }
> -
> - CopyMem (, LockBoxParameterSave, sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_SAVE));
> -
> - //
> - // Sanity check
> - //
> - if (!SmmIsBufferOutsideSmmValid
> ((UINTN)TempLockBoxParameterSave.Buffer,
> (UINTN)TempLockBoxParameterSave.Length)) {
> -DEBUG ((DEBUG_ERROR, "SmmLockBox Save address in SMRAM or buffer
> overflow!\n"));
> -LockBoxParameterSave->Header.ReturnStatus =
> (UINT64)EFI_ACCESS_DENIED;
> -return;
> - }
> -
> - //
> - // The SpeculationBarrier() call here is to ensure the above range check
> for
> - // the CommBuffer have been completed before calling into SaveLockBox().
> - //
> - SpeculationBarrier ();
> -
> - //
> - // Save data
> - //
> - Status = SaveLockBox (
> - ,
> - (VOID *)(UINTN)TempLockBoxParameterSave.Buffer,
> - (UINTN)TempLockBoxParameterSave.Length
> - );
> - LockBoxParameterSave->Header.ReturnStatus = (UINT64)Status;
> -
Re: [edk2-devel] [PATCH 2/3] MdeModulePkg: Refactors SmmLockBox.c.
Reviewed-by: Ray Ni
Thanks,
Ray
From: Xie, Yuanhao
Sent: Tuesday, May 7, 2024 14:09
To: devel@edk2.groups.io
Cc: Liming Gao ; Wu, Jiaxin ;
Ni, Ray ; Xie, Yuanhao
Subject: [PATCH 2/3] MdeModulePkg: Refactors SmmLockBox.c.
The Lockbox Driver allows sensitive data to be securely stored in a
designated area, thus protected against unauthorized access.
This patch does not introduce any functional modifications.
It refactors the existing logic into a common component to facilitates
the integration of the Standalone MM Lockbox Driver in an upcoming patch
Cc: Liming Gao
Cc: Jiaxin Wu
Cc: Ray Ni
Signed-off-by: Yuanhao Xie
---
MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c | 361
-
MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf | 4 +++-
MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.c | 384
MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.h | 148
4 files changed, 547 insertions(+), 350 deletions(-)
diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
index c1e15c596b..2774979c34 100644
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
@@ -9,7 +9,7 @@
SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(),
SmmLockBoxSave()
will receive untrusted input and do basic validation.
-Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -31,360 +31,24 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include
#include
-BOOLEAN mLocked = FALSE;
+#include "SmmLockBoxCommon.h"
/**
- Dispatch function for SMM lock box save.
+ This function is an abstraction layer for implementation specific Mm buffer
validation routine.
- Caution: This function may receive untrusted input.
- Restore buffer and length are external input, so this function will validate
- it is in SMRAM.
+ @param Buffer The buffer start address to be checked.
+ @param Length The buffer length to be checked.
- @param LockBoxParameterSave parameter of lock box save
+ @retval TRUE This buffer is valid per processor architecture and not
overlap with SMRAM.
+ @retval FALSE This buffer is not valid per processor architecture or overlap
with SMRAM.
**/
-VOID
-SmmLockBoxSave (
- IN EFI_SMM_LOCK_BOX_PARAMETER_SAVE *LockBoxParameterSave
+BOOLEAN
+IsBufferOutsideMmValid (
+ IN EFI_PHYSICAL_ADDRESS Buffer,
+ IN UINT64Length
)
{
- EFI_STATUS Status;
- EFI_SMM_LOCK_BOX_PARAMETER_SAVE TempLockBoxParameterSave;
-
- //
- // Sanity check
- //
- if (mLocked) {
-DEBUG ((DEBUG_ERROR, "SmmLockBox Locked!\n"));
-LockBoxParameterSave->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;
-return;
- }
-
- CopyMem (, LockBoxParameterSave, sizeof
(EFI_SMM_LOCK_BOX_PARAMETER_SAVE));
-
- //
- // Sanity check
- //
- if (!SmmIsBufferOutsideSmmValid ((UINTN)TempLockBoxParameterSave.Buffer,
(UINTN)TempLockBoxParameterSave.Length)) {
-DEBUG ((DEBUG_ERROR, "SmmLockBox Save address in SMRAM or buffer
overflow!\n"));
-LockBoxParameterSave->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;
-return;
- }
-
- //
- // The SpeculationBarrier() call here is to ensure the above range check for
- // the CommBuffer have been completed before calling into SaveLockBox().
- //
- SpeculationBarrier ();
-
- //
- // Save data
- //
- Status = SaveLockBox (
- ,
- (VOID *)(UINTN)TempLockBoxParameterSave.Buffer,
- (UINTN)TempLockBoxParameterSave.Length
- );
- LockBoxParameterSave->Header.ReturnStatus = (UINT64)Status;
- return;
-}
-
-/**
- Dispatch function for SMM lock box set attributes.
-
- @param LockBoxParameterSetAttributes parameter of lock box set attributes
-**/
-VOID
-SmmLockBoxSetAttributes (
- IN EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES *LockBoxParameterSetAttributes
-
