From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Xie, Yuanhao" <yuanhao.xie@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Liming Gao <gaoliming@byosoft.com.cn>, "Ni, Ray" <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH 2/3] MdeModulePkg: Refactors SmmLockBox.c.
Date: Thu, 9 May 2024 03:41:58 +0000 [thread overview]
Message-ID: <MN0PR11MB61582A4CEF45C996E9080237FEE62@MN0PR11MB6158.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20240507060910.1687-3-yuanhao.xie@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> -----Original Message-----
> From: Xie, Yuanhao <yuanhao.xie@intel.com>
> Sent: Tuesday, May 7, 2024 2:09 PM
> To: devel@edk2.groups.io
> Cc: Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin
> <jiaxin.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Xie, Yuanhao
> <yuanhao.xie@intel.com>
> 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 <gaoliming@byosoft.com.cn>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Signed-off-by: Yuanhao Xie <yuanhao.xie@intel.com>
> ---
> 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.<BR>
> +Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -31,360 +31,24 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Protocol/LockBox.h>
> #include <Guid/SmmLockBox.h>
>
> -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 UINT64 Length
> )
> {
> - 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 (&TempLockBoxParameterSave, 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 (
> - &TempLockBoxParameterSave.Guid,
> - (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
> - )
> -{
> - EFI_STATUS Status;
> - EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES
> TempLockBoxParameterSetAttributes;
> -
> - //
> - // Sanity check
> - //
> - if (mLocked) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Locked!\n"));
> - LockBoxParameterSetAttributes->Header.ReturnStatus =
> (UINT64)EFI_ACCESS_DENIED;
> - return;
> - }
> -
> - CopyMem (&TempLockBoxParameterSetAttributes,
> LockBoxParameterSetAttributes, sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES));
> -
> - //
> - // Update data
> - //
> - Status = SetLockBoxAttributes (
> - &TempLockBoxParameterSetAttributes.Guid,
> - TempLockBoxParameterSetAttributes.Attributes
> - );
> - LockBoxParameterSetAttributes->Header.ReturnStatus = (UINT64)Status;
> - return;
> -}
> -
> -/**
> - Dispatch function for SMM lock box update.
> -
> - Caution: This function may receive untrusted input.
> - Restore buffer and length are external input, so this function will validate
> - it is in SMRAM.
> -
> - @param LockBoxParameterUpdate parameter of lock box update
> -**/
> -VOID
> -SmmLockBoxUpdate (
> - IN EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *LockBoxParameterUpdate
> - )
> -{
> - EFI_STATUS Status;
> - EFI_SMM_LOCK_BOX_PARAMETER_UPDATE
> TempLockBoxParameterUpdate;
> -
> - //
> - // Sanity check
> - //
> - if (mLocked) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Locked!\n"));
> - LockBoxParameterUpdate->Header.ReturnStatus =
> (UINT64)EFI_ACCESS_DENIED;
> - return;
> - }
> -
> - CopyMem (&TempLockBoxParameterUpdate, LockBoxParameterUpdate,
> sizeof (EFI_SMM_LOCK_BOX_PARAMETER_UPDATE));
> -
> - //
> - // Sanity check
> - //
> - if (!SmmIsBufferOutsideSmmValid
> ((UINTN)TempLockBoxParameterUpdate.Buffer,
> (UINTN)TempLockBoxParameterUpdate.Length)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Update address in SMRAM or
> buffer overflow!\n"));
> - LockBoxParameterUpdate->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
> UpdateLockBox().
> - //
> - SpeculationBarrier ();
> -
> - //
> - // Update data
> - //
> - Status = UpdateLockBox (
> - &TempLockBoxParameterUpdate.Guid,
> - (UINTN)TempLockBoxParameterUpdate.Offset,
> - (VOID *)(UINTN)TempLockBoxParameterUpdate.Buffer,
> - (UINTN)TempLockBoxParameterUpdate.Length
> - );
> - LockBoxParameterUpdate->Header.ReturnStatus = (UINT64)Status;
> - return;
> -}
> -
> -/**
> - Dispatch function for SMM lock box restore.
> -
> - Caution: This function may receive untrusted input.
> - Restore buffer and length are external input, so this function will validate
> - it is in SMRAM.
> -
> - @param LockBoxParameterRestore parameter of lock box restore
> -**/
> -VOID
> -SmmLockBoxRestore (
> - IN EFI_SMM_LOCK_BOX_PARAMETER_RESTORE *LockBoxParameterRestore
> - )
> -{
> - EFI_STATUS Status;
> - EFI_SMM_LOCK_BOX_PARAMETER_RESTORE
> TempLockBoxParameterRestore;
> -
> - CopyMem (&TempLockBoxParameterRestore, LockBoxParameterRestore,
> sizeof (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE));
> -
> - //
> - // Sanity check
> - //
> - if (!SmmIsBufferOutsideSmmValid
> ((UINTN)TempLockBoxParameterRestore.Buffer,
> (UINTN)TempLockBoxParameterRestore.Length)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Restore address in SMRAM or
> buffer overflow!\n"));
> - LockBoxParameterRestore->Header.ReturnStatus =
> (UINT64)EFI_ACCESS_DENIED;
> - return;
> - }
> -
> - //
> - // Restore data
> - //
> - if ((TempLockBoxParameterRestore.Length == 0) &&
> (TempLockBoxParameterRestore.Buffer == 0)) {
> - Status = RestoreLockBox (
> - &TempLockBoxParameterRestore.Guid,
> - NULL,
> - NULL
> - );
> - } else {
> - Status = RestoreLockBox (
> - &TempLockBoxParameterRestore.Guid,
> - (VOID *)(UINTN)TempLockBoxParameterRestore.Buffer,
> - (UINTN *)&TempLockBoxParameterRestore.Length
> - );
> - if ((Status == EFI_BUFFER_TOO_SMALL) || (Status == EFI_SUCCESS)) {
> - //
> - // Return the actual Length value.
> - //
> - LockBoxParameterRestore->Length =
> TempLockBoxParameterRestore.Length;
> - }
> - }
> -
> - LockBoxParameterRestore->Header.ReturnStatus = (UINT64)Status;
> - return;
> -}
> -
> -/**
> - Dispatch function for SMM lock box restore all in place.
> -
> - @param LockBoxParameterRestoreAllInPlace parameter of lock box restore
> all in place
> -**/
> -VOID
> -SmmLockBoxRestoreAllInPlace (
> - IN EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE
> *LockBoxParameterRestoreAllInPlace
> - )
> -{
> - EFI_STATUS Status;
> -
> - Status = RestoreAllLockBoxInPlace ();
> - LockBoxParameterRestoreAllInPlace->Header.ReturnStatus =
> (UINT64)Status;
> - return;
> -}
> -
> -/**
> - Dispatch function for a Software SMI handler.
> -
> - Caution: This function may receive untrusted input.
> - Communicate buffer and buffer size are external input, so this function will
> do basic validation.
> -
> - @param DispatchHandle The unique handle assigned to this handler by
> SmiHandlerRegister().
> - @param Context Points to an optional handler context which was
> specified when the
> - handler was registered.
> - @param CommBuffer A pointer to a collection of data in memory that will
> - be conveyed from a non-SMM environment into an SMM
> environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @retval EFI_SUCCESS Command is handled successfully.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -SmmLockBoxHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context OPTIONAL,
> - IN OUT VOID *CommBuffer OPTIONAL,
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - )
> -{
> - EFI_SMM_LOCK_BOX_PARAMETER_HEADER *LockBoxParameterHeader;
> - UINTN TempCommBufferSize;
> -
> - DEBUG ((DEBUG_INFO, "SmmLockBox SmmLockBoxHandler Enter\n"));
> -
> - //
> - // If input is invalid, stop processing this SMI
> - //
> - if ((CommBuffer == NULL) || (CommBufferSize == NULL)) {
> - return EFI_SUCCESS;
> - }
> -
> - TempCommBufferSize = *CommBufferSize;
> -
> - //
> - // Sanity check
> - //
> - if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size invalid!\n"));
> - return EFI_SUCCESS;
> - }
> -
> - if (!SmmIsBufferOutsideSmmValid ((UINTN)CommBuffer,
> TempCommBufferSize)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer in SMRAM or
> overflow!\n"));
> - return EFI_SUCCESS;
> - }
> -
> - LockBoxParameterHeader = (EFI_SMM_LOCK_BOX_PARAMETER_HEADER
> *)((UINTN)CommBuffer);
> -
> - LockBoxParameterHeader->ReturnStatus = (UINT64)-1;
> -
> - DEBUG ((DEBUG_INFO, "SmmLockBox LockBoxParameterHeader - %x\n",
> (UINTN)LockBoxParameterHeader));
> -
> - DEBUG ((DEBUG_INFO, "SmmLockBox Command - %x\n",
> (UINTN)LockBoxParameterHeader->Command));
> -
> - switch (LockBoxParameterHeader->Command) {
> - case EFI_SMM_LOCK_BOX_COMMAND_SAVE:
> - if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for SAVE
> invalid!\n"));
> - break;
> - }
> -
> - SmmLockBoxSave ((EFI_SMM_LOCK_BOX_PARAMETER_SAVE
> *)(UINTN)LockBoxParameterHeader);
> - break;
> - case EFI_SMM_LOCK_BOX_COMMAND_UPDATE:
> - if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for
> UPDATE invalid!\n"));
> - break;
> - }
> -
> - SmmLockBoxUpdate ((EFI_SMM_LOCK_BOX_PARAMETER_UPDATE
> *)(UINTN)LockBoxParameterHeader);
> - break;
> - case EFI_SMM_LOCK_BOX_COMMAND_RESTORE:
> - if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for
> RESTORE invalid!\n"));
> - break;
> - }
> -
> - SmmLockBoxRestore ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE
> *)(UINTN)LockBoxParameterHeader);
> - break;
> - case EFI_SMM_LOCK_BOX_COMMAND_SET_ATTRIBUTES:
> - if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for
> SET_ATTRIBUTES invalid!\n"));
> - break;
> - }
> -
> - SmmLockBoxSetAttributes
> ((EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES
> *)(UINTN)LockBoxParameterHeader);
> - break;
> - case EFI_SMM_LOCK_BOX_COMMAND_RESTORE_ALL_IN_PLACE:
> - if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) {
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for
> RESTORE_ALL_IN_PLACE invalid!\n"));
> - break;
> - }
> -
> - SmmLockBoxRestoreAllInPlace
> ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE
> *)(UINTN)LockBoxParameterHeader);
> - break;
> - default:
> - DEBUG ((DEBUG_ERROR, "SmmLockBox Command invalid!\n"));
> - break;
> - }
> -
> - LockBoxParameterHeader->Command = (UINT32)-1;
> -
> - DEBUG ((DEBUG_INFO, "SmmLockBox SmmLockBoxHandler Exit\n"));
> -
> - return EFI_SUCCESS;
> -}
> -
> -/**
> - Smm Ready To Lock event notification handler.
> -
> - It sets a flag indicating that SMRAM has been locked.
> -
> - @param[in] Protocol Points to the protocol's unique identifier.
> - @param[in] Interface Points to the interface instance.
> - @param[in] Handle The handle on which the interface was installed.
> -
> - @retval EFI_SUCCESS Notification handler runs successfully.
> - **/
> -EFI_STATUS
> -EFIAPI
> -SmmReadyToLockEventNotify (
> - IN CONST EFI_GUID *Protocol,
> - IN VOID *Interface,
> - IN EFI_HANDLE Handle
> - )
> -{
> - mLocked = TRUE;
> - return EFI_SUCCESS;
> + return SmmIsBufferOutsideSmmValid (Buffer, Length);
> }
>
> /**
> @@ -438,6 +102,5 @@ SmmLockBoxEntryPoint (
> NULL
> );
> ASSERT_EFI_ERROR (Status);
> -
> return Status;
> }
> diff --git
> a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
> b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
> index 5081b2d7f2..f279706e90 100644
> --- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
> +++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.inf
> @@ -6,7 +6,7 @@
> # This external input must be validated carefully to avoid security issue like
> # buffer overflow, integer overflow.
> #
> -# Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -30,6 +30,8 @@
>
> [Sources]
> SmmLockBox.c
> + SmmLockBoxCommon.c
> + SmmLockBoxCommon.h
>
> [Packages]
> MdePkg/MdePkg.dec
> diff --git
> a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.c
> b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.c
> new file mode 100644
> index 0000000000..5c6eae05af
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.c
> @@ -0,0 +1,384 @@
> +/** @file
> + LockBox SMM/MM driver.
> +
> + Caution: This module requires additional review when modified.
> + This driver will have external input - communicate buffer in SMM mode.
> + This external input must be validated carefully to avoid security issue like
> + buffer overflow, integer overflow.
> +
> + SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(),
> SmmLockBoxSave()
> + will receive untrusted input and do basic validation.
> +
> +Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
> +
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiSmm.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/LockBoxLib.h>
> +
> +#include <Protocol/SmmReadyToLock.h>
> +#include <Protocol/SmmCommunication.h>
> +#include <Protocol/LockBox.h>
> +#include <Guid/SmmLockBox.h>
> +#include "SmmLockBoxCommon.h"
> +
> +BOOLEAN mLocked = FALSE;
> +
> +/**
> + Dispatch function for SMM lock box save.
> +
> + Caution: This function may receive untrusted input.
> + Restore buffer and length are external input, so this function will validate
> + it is in SMRAM.
> +
> + @param LockBoxParameterSave parameter of lock box save
> +**/
> +VOID
> +SmmLockBoxSave (
> + IN EFI_SMM_LOCK_BOX_PARAMETER_SAVE *LockBoxParameterSave
> + )
> +{
> + 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 (&TempLockBoxParameterSave, LockBoxParameterSave, sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_SAVE));
> +
> + //
> + // Sanity check
> + //
> + if (!IsBufferOutsideMmValid ((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 (
> + &TempLockBoxParameterSave.Guid,
> + (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
> + )
> +{
> + EFI_STATUS Status;
> + EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES
> TempLockBoxParameterSetAttributes;
> +
> + //
> + // Sanity check
> + //
> + if (mLocked) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Locked!\n"));
> + LockBoxParameterSetAttributes->Header.ReturnStatus =
> (UINT64)EFI_ACCESS_DENIED;
> + return;
> + }
> +
> + CopyMem (&TempLockBoxParameterSetAttributes,
> LockBoxParameterSetAttributes, sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES));
> +
> + //
> + // Update data
> + //
> + Status = SetLockBoxAttributes (
> + &TempLockBoxParameterSetAttributes.Guid,
> + TempLockBoxParameterSetAttributes.Attributes
> + );
> + LockBoxParameterSetAttributes->Header.ReturnStatus = (UINT64)Status;
> + return;
> +}
> +
> +/**
> + Dispatch function for SMM lock box update.
> +
> + Caution: This function may receive untrusted input.
> + Restore buffer and length are external input, so this function will validate
> + it is in SMRAM.
> +
> + @param LockBoxParameterUpdate parameter of lock box update
> +**/
> +VOID
> +SmmLockBoxUpdate (
> + IN EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *LockBoxParameterUpdate
> + )
> +{
> + EFI_STATUS Status;
> + EFI_SMM_LOCK_BOX_PARAMETER_UPDATE
> TempLockBoxParameterUpdate;
> +
> + //
> + // Sanity check
> + //
> + if (mLocked) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Locked!\n"));
> + LockBoxParameterUpdate->Header.ReturnStatus =
> (UINT64)EFI_ACCESS_DENIED;
> + return;
> + }
> +
> + CopyMem (&TempLockBoxParameterUpdate, LockBoxParameterUpdate,
> sizeof (EFI_SMM_LOCK_BOX_PARAMETER_UPDATE));
> +
> + //
> + // Sanity check
> + //
> + if (!IsBufferOutsideMmValid
> ((UINTN)TempLockBoxParameterUpdate.Buffer,
> (UINTN)TempLockBoxParameterUpdate.Length)) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Update address in SMRAM or
> buffer overflow!\n"));
> + LockBoxParameterUpdate->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
> UpdateLockBox().
> + //
> + SpeculationBarrier ();
> +
> + //
> + // Update data
> + //
> + Status = UpdateLockBox (
> + &TempLockBoxParameterUpdate.Guid,
> + (UINTN)TempLockBoxParameterUpdate.Offset,
> + (VOID *)(UINTN)TempLockBoxParameterUpdate.Buffer,
> + (UINTN)TempLockBoxParameterUpdate.Length
> + );
> + LockBoxParameterUpdate->Header.ReturnStatus = (UINT64)Status;
> + return;
> +}
> +
> +/**
> + Dispatch function for SMM lock box restore.
> +
> + Caution: This function may receive untrusted input.
> + Restore buffer and length are external input, so this function will validate
> + it is in SMRAM.
> +
> + @param LockBoxParameterRestore parameter of lock box restore
> +**/
> +VOID
> +SmmLockBoxRestore (
> + IN EFI_SMM_LOCK_BOX_PARAMETER_RESTORE
> *LockBoxParameterRestore
> + )
> +{
> + EFI_STATUS Status;
> + EFI_SMM_LOCK_BOX_PARAMETER_RESTORE
> TempLockBoxParameterRestore;
> +
> + CopyMem (&TempLockBoxParameterRestore, LockBoxParameterRestore,
> sizeof (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE));
> +
> + //
> + // Sanity check
> + //
> + if (!IsBufferOutsideMmValid
> ((UINTN)TempLockBoxParameterRestore.Buffer,
> (UINTN)TempLockBoxParameterRestore.Length)) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Restore address in SMRAM or
> buffer overflow!\n"));
> + LockBoxParameterRestore->Header.ReturnStatus =
> (UINT64)EFI_ACCESS_DENIED;
> + return;
> + }
> +
> + //
> + // Restore data
> + //
> + if ((TempLockBoxParameterRestore.Length == 0) &&
> (TempLockBoxParameterRestore.Buffer == 0)) {
> + Status = RestoreLockBox (
> + &TempLockBoxParameterRestore.Guid,
> + NULL,
> + NULL
> + );
> + } else {
> + Status = RestoreLockBox (
> + &TempLockBoxParameterRestore.Guid,
> + (VOID *)(UINTN)TempLockBoxParameterRestore.Buffer,
> + (UINTN *)&TempLockBoxParameterRestore.Length
> + );
> + if ((Status == EFI_BUFFER_TOO_SMALL) || (Status == EFI_SUCCESS)) {
> + //
> + // Return the actual Length value.
> + //
> + LockBoxParameterRestore->Length =
> TempLockBoxParameterRestore.Length;
> + }
> + }
> +
> + LockBoxParameterRestore->Header.ReturnStatus = (UINT64)Status;
> + return;
> +}
> +
> +/**
> + Dispatch function for SMM lock box restore all in place.
> +
> + @param LockBoxParameterRestoreAllInPlace parameter of lock box restore
> all in place
> +**/
> +VOID
> +SmmLockBoxRestoreAllInPlace (
> + IN EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE
> *LockBoxParameterRestoreAllInPlace
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = RestoreAllLockBoxInPlace ();
> + LockBoxParameterRestoreAllInPlace->Header.ReturnStatus =
> (UINT64)Status;
> + return;
> +}
> +
> +/**
> + Dispatch function for a Software SMI handler.
> +
> + Caution: This function may receive untrusted input.
> + Communicate buffer and buffer size are external input, so this function will
> do basic validation.
> +
> + @param DispatchHandle The unique handle assigned to this handler by
> SmiHandlerRegister().
> + @param Context Points to an optional handler context which was
> specified when the
> + handler was registered.
> + @param CommBuffer A pointer to a collection of data in memory that will
> + be conveyed from a non-SMM environment into an SMM
> environment.
> + @param CommBufferSize The size of the CommBuffer.
> +
> + @retval EFI_SUCCESS Command is handled successfully.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmLockBoxHandler (
> + IN EFI_HANDLE DispatchHandle,
> + IN CONST VOID *Context OPTIONAL,
> + IN OUT VOID *CommBuffer OPTIONAL,
> + IN OUT UINTN *CommBufferSize OPTIONAL
> + )
> +{
> + EFI_SMM_LOCK_BOX_PARAMETER_HEADER *LockBoxParameterHeader;
> + UINTN TempCommBufferSize;
> +
> + DEBUG ((DEBUG_INFO, "SmmLockBox SmmLockBoxHandler Enter\n"));
> +
> + //
> + // If input is invalid, stop processing this SMI
> + //
> + if ((CommBuffer == NULL) || (CommBufferSize == NULL)) {
> + return EFI_SUCCESS;
> + }
> +
> + TempCommBufferSize = *CommBufferSize;
> +
> + //
> + // Sanity check
> + //
> + if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_HEADER)) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size
> invalid!\n"));
> + return EFI_SUCCESS;
> + }
> +
> + if (!IsBufferOutsideMmValid ((UINTN)CommBuffer, TempCommBufferSize))
> {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer in SMRAM or
> overflow!\n"));
> + return EFI_SUCCESS;
> + }
> +
> + LockBoxParameterHeader = (EFI_SMM_LOCK_BOX_PARAMETER_HEADER
> *)((UINTN)CommBuffer);
> +
> + LockBoxParameterHeader->ReturnStatus = (UINT64)-1;
> +
> + DEBUG ((DEBUG_INFO, "SmmLockBox LockBoxParameterHeader - %x\n",
> (UINTN)LockBoxParameterHeader));
> +
> + DEBUG ((DEBUG_INFO, "SmmLockBox Command - %x\n",
> (UINTN)LockBoxParameterHeader->Command));
> +
> + switch (LockBoxParameterHeader->Command) {
> + case EFI_SMM_LOCK_BOX_COMMAND_SAVE:
> + if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_SAVE)) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for SAVE
> invalid!\n"));
> + break;
> + }
> +
> + SmmLockBoxSave ((EFI_SMM_LOCK_BOX_PARAMETER_SAVE
> *)(UINTN)LockBoxParameterHeader);
> + break;
> + case EFI_SMM_LOCK_BOX_COMMAND_UPDATE:
> + if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_UPDATE)) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for
> UPDATE invalid!\n"));
> + break;
> + }
> +
> + SmmLockBoxUpdate ((EFI_SMM_LOCK_BOX_PARAMETER_UPDATE
> *)(UINTN)LockBoxParameterHeader);
> + break;
> + case EFI_SMM_LOCK_BOX_COMMAND_RESTORE:
> + if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE)) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for
> RESTORE invalid!\n"));
> + break;
> + }
> +
> + SmmLockBoxRestore ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE
> *)(UINTN)LockBoxParameterHeader);
> + break;
> + case EFI_SMM_LOCK_BOX_COMMAND_SET_ATTRIBUTES:
> + if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES)) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for
> SET_ATTRIBUTES invalid!\n"));
> + break;
> + }
> +
> + SmmLockBoxSetAttributes
> ((EFI_SMM_LOCK_BOX_PARAMETER_SET_ATTRIBUTES
> *)(UINTN)LockBoxParameterHeader);
> + break;
> + case EFI_SMM_LOCK_BOX_COMMAND_RESTORE_ALL_IN_PLACE:
> + if (TempCommBufferSize < sizeof
> (EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE)) {
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Command Buffer Size for
> RESTORE_ALL_IN_PLACE invalid!\n"));
> + break;
> + }
> +
> + SmmLockBoxRestoreAllInPlace
> ((EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE
> *)(UINTN)LockBoxParameterHeader);
> + break;
> + default:
> + DEBUG ((DEBUG_ERROR, "SmmLockBox Command invalid!\n"));
> + break;
> + }
> +
> + LockBoxParameterHeader->Command = (UINT32)-1;
> +
> + DEBUG ((DEBUG_INFO, "SmmLockBox SmmLockBoxHandler Exit\n"));
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Smm Ready To Lock event notification handler.
> +
> + It sets a flag indicating that SMRAM has been locked.
> +
> + @param[in] Protocol Points to the protocol's unique identifier.
> + @param[in] Interface Points to the interface instance.
> + @param[in] Handle The handle on which the interface was installed.
> +
> + @retval EFI_SUCCESS Notification handler runs successfully.
> + **/
> +EFI_STATUS
> +EFIAPI
> +SmmReadyToLockEventNotify (
> + IN CONST EFI_GUID *Protocol,
> + IN VOID *Interface,
> + IN EFI_HANDLE Handle
> + )
> +{
> + mLocked = TRUE;
> + return EFI_SUCCESS;
> +}
> diff --git
> a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.h
> b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.h
> new file mode 100644
> index 0000000000..2205c4fb3b
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBoxCommon.h
> @@ -0,0 +1,148 @@
> +/** @file
> + LockBox SMM/MM driver.
> +
> + Caution: This module requires additional review when modified.
> + This driver will have external input - communicate buffer in SMM mode.
> + This external input must be validated carefully to avoid security issue like
> + buffer overflow, integer overflow.
> +
> + SmmLockBoxHandler(), SmmLockBoxRestore(), SmmLockBoxUpdate(),
> SmmLockBoxSave()
> + will receive untrusted input and do basic validation.
> +
> +Copyright (c) 2010 - 2024, Intel Corporation. All rights reserved.<BR>
> +
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiSmm.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/LockBoxLib.h>
> +
> +#include <Protocol/SmmReadyToLock.h>
> +#include <Protocol/SmmCommunication.h>
> +#include <Protocol/LockBox.h>
> +#include <Guid/SmmLockBox.h>
> +
> +/**
> + This function is an abstraction layer for implementation specific Mm buffer
> validation routine.
> +
> + @param Buffer The buffer start address to be checked.
> + @param Length The buffer length to be checked.
> +
> + @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.
> +**/
> +BOOLEAN
> +IsBufferOutsideMmValid (
> + IN EFI_PHYSICAL_ADDRESS Buffer,
> + IN UINT64 Length
> + );
> +
> +/**
> + Dispatch function for SMM lock box save.
> +
> + Caution: This function may receive untrusted input.
> + Restore buffer and length are external input, so this function will validate
> + it is in SMRAM.
> +
> + @param LockBoxParameterSave parameter of lock box save
> +**/
> +VOID
> +SmmLockBoxSave (
> + IN EFI_SMM_LOCK_BOX_PARAMETER_SAVE *LockBoxParameterSave
> + );
> +
> +/**
> + 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
> + );
> +
> +/**
> + Dispatch function for SMM lock box update.
> +
> + Caution: This function may receive untrusted input.
> + Restore buffer and length are external input, so this function will validate
> + it is in SMRAM.
> +
> + @param LockBoxParameterUpdate parameter of lock box update
> +**/
> +VOID
> +SmmLockBoxUpdate (
> + IN EFI_SMM_LOCK_BOX_PARAMETER_UPDATE *LockBoxParameterUpdate
> + );
> +
> +/**
> + Dispatch function for SMM lock box restore.
> +
> + Caution: This function may receive untrusted input.
> + Restore buffer and length are external input, so this function will validate
> + it is in SMRAM.
> +
> + @param LockBoxParameterRestore parameter of lock box restore
> +**/
> +VOID
> +SmmLockBoxRestore (
> + IN EFI_SMM_LOCK_BOX_PARAMETER_RESTORE
> *LockBoxParameterRestore
> + );
> +
> +/**
> + Dispatch function for SMM lock box restore all in place.
> +
> + @param LockBoxParameterRestoreAllInPlace parameter of lock box restore
> all in place
> +**/
> +VOID
> +SmmLockBoxRestoreAllInPlace (
> + IN EFI_SMM_LOCK_BOX_PARAMETER_RESTORE_ALL_IN_PLACE
> *LockBoxParameterRestoreAllInPlace
> + );
> +
> +/**
> + Dispatch function for a Software SMI handler.
> +
> + Caution: This function may receive untrusted input.
> + Communicate buffer and buffer size are external input, so this function will
> do basic validation.
> +
> + @param DispatchHandle The unique handle assigned to this handler by
> SmiHandlerRegister().
> + @param Context Points to an optional handler context which was
> specified when the
> + handler was registered.
> + @param CommBuffer A pointer to a collection of data in memory that will
> + be conveyed from a non-SMM environment into an SMM
> environment.
> + @param CommBufferSize The size of the CommBuffer.
> +
> + @retval EFI_SUCCESS Command is handled successfully.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmLockBoxHandler (
> + IN EFI_HANDLE DispatchHandle,
> + IN CONST VOID *Context OPTIONAL,
> + IN OUT VOID *CommBuffer OPTIONAL,
> + IN OUT UINTN *CommBufferSize OPTIONAL
> + );
> +
> +/**
> + Smm Ready To Lock event notification handler.
> +
> + It sets a flag indicating that SMRAM has been locked.
> +
> + @param[in] Protocol Points to the protocol's unique identifier.
> + @param[in] Interface Points to the interface instance.
> + @param[in] Handle The handle on which the interface was installed.
> +
> + @retval EFI_SUCCESS Notification handler runs successfully.
> + **/
> +EFI_STATUS
> +EFIAPI
> +SmmReadyToLockEventNotify (
> + IN CONST EFI_GUID *Protocol,
> + IN VOID *Interface,
> + IN EFI_HANDLE Handle
> + );
> --
> 2.39.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118709): https://edk2.groups.io/g/devel/message/118709
Mute This Topic: https://groups.io/mt/105955700/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2024-05-09 3:42 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 6:09 [edk2-devel] [PATCH 0/3] Add Standalone MM Lockbox Driver Yuanhao Xie
2024-05-07 6:09 ` [edk2-devel] [PATCH 1/3] StandaloneMmPkg: Add LockBox Dependency DXE Driver Yuanhao Xie
2024-05-08 2:46 ` Ni, Ray
2024-05-09 3:42 ` Wu, Jiaxin
2024-05-07 6:09 ` [edk2-devel] [PATCH 2/3] MdeModulePkg: Refactors SmmLockBox.c Yuanhao Xie
2024-05-08 2:50 ` Ni, Ray
2024-05-09 3:41 ` Wu, Jiaxin [this message]
2024-05-07 6:09 ` [edk2-devel] [PATCH 3/3] MdeModulePkg: Add Standalone MM Lockbox Driver Yuanhao Xie
2024-05-08 2:53 ` Ni, Ray
2024-05-09 3:42 ` Wu, Jiaxin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN0PR11MB61582A4CEF45C996E9080237FEE62@MN0PR11MB6158.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox