From: "Liming Gao" <liming.gao@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"michael.kubacki@outlook.com" <michael.kubacki@outlook.com>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>, "Wu, Hao A" <hao.a.wu@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 04/12] MdeModulePkg: Define the VarCheckPolicyLib and SMM interface
Date: Tue, 12 May 2020 13:37:41 +0000 [thread overview]
Message-ID: <BN6PR11MB39727188BC284A8BDF09D4E380BE0@BN6PR11MB3972.namprd11.prod.outlook.com> (raw)
In-Reply-To: <8ff3a619-8c0a-e7e3-9755-fed6adf34675@redhat.com>
> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, May 12, 2020 8:27 PM
> To: devel@edk2.groups.io; michael.kubacki@outlook.com
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 04/12] MdeModulePkg: Define the VarCheckPolicyLib and SMM interface
>
> very superficial comments:
>
> On 05/12/20 08:46, Michael Kubacki wrote:
> > From: Bret Barkelew <brbarkel@microsoft.com>
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2522
> >
> > VariablePolicy is an updated interface to
> > replace VarLock and VarCheckProtocol.
> >
> > This is an instance of a VarCheckLib that is backed by the
> > VariablePolicyLib business logic. It also publishes the SMM
> > calling interface for messages from the DXE protocol.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> > ---
> > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c | 318 ++++++++++++++++++++
> > MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h | 54 ++++
> > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf | 44 +++
> > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni | 12 +
> > MdeModulePkg/MdeModulePkg.dec | 4 +
> > MdeModulePkg/MdeModulePkg.dsc | 2 +
> > 6 files changed, 434 insertions(+)
> >
> > diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> > new file mode 100644
> > index 000000000000..d5775d7dd068
> > --- /dev/null
> > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c
> > @@ -0,0 +1,318 @@
> > +/** @file -- VarCheckPolicyLib.c
> > +This is an instance of a VarCheck lib that leverages the business logic behind
> > +the VariablePolicy code to make its decisions.
> > +
> > +Copyright (c) Microsoft Corporation.
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <Library/VarCheckLib.h>
> > +#include <Library/BaseLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/SafeIntLib.h>
> > +#include <Library/MmServicesTableLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +
> > +#include <Protocol/MmCommunication.h>
> > +
> > +#include <Protocol/VariablePolicy.h>
> > +#include <Library/VariablePolicyLib.h>
> > +
> > +#include <Guid/VarCheckPolicyMmi.h>
> > +
> > +//================================================
> > +// As a VarCheck library, we're linked into the VariableServices
> > +// and may not be able to call them indirectly. To get around this,
> > +// use the internal GetVariable function to query the variable store.
> > +//================================================
> > +EFI_STATUS
> > +EFIAPI
> > +VariableServiceGetVariable (
> > + IN CHAR16 *VariableName,
> > + IN EFI_GUID *VendorGuid,
> > + OUT UINT32 *Attributes OPTIONAL,
> > + IN OUT UINTN *DataSize,
> > + OUT VOID *Data
> > + );
> > +
> > +
> > +/**
> > + MM Communication Handler to recieve commands from the DXE protocol for
> > + Variable Policies. This communication channel is used to register new policies
> > + and poll and toggle the enforcement of variable policies.
> > +
> > + @param[in] DispatchHandle All parameters standard to MM communications convention.
> > + @param[in] RegisterContex All parameters standard to MM communications convention.
> > + @param[in,out] CommBuffer All parameters standard to MM communications convention.
> > + @param[in,out] CommBufferSize All parameters standard to MM communications convention.
> > +
> > + @retval EFI_SUCCESS
> > + @retval EFI_INVALID_PARAMETER CommBuffer or CommBufferSize is null pointer.
> > + @retval EFI_INVALID_PARAMETER CommBuffer size is wrong.
> > + @retval EFI_INVALID_PARAMETER Revision or signature don't match.
> > +
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +EFIAPI
> > +VarCheckPolicyLibMmiHandler (
> > + IN EFI_HANDLE DispatchHandle,
> > + IN CONST VOID *RegisterContext,
> > + IN OUT VOID *CommBuffer,
> > + IN OUT UINTN *CommBufferSize
> > + )
> > +{
> > + EFI_STATUS Status = EFI_SUCCESS;
> > + EFI_STATUS SubCommandStatus;
> > + VAR_CHECK_POLICY_COMM_HEADER *PolicyCommmHeader;
> > + VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS *IsEnabledParams;
> > + VAR_CHECK_POLICY_COMM_DUMP_PARAMS *DumpParams;
> > + UINT8 *DumpInputBuffer;
> > + UINT8 *DumpOutputBuffer;
> > + UINTN DumpTotalPages;
> > + VARIABLE_POLICY_ENTRY *PolicyEntry;
> > + UINTN ExpectedSize;
> > + // Pagination Cache Variables
>
> (1) This comment style is not idiomatic in edk2. I think we use empty
> "//" lines before and after.
>
> > + static UINT8 *PaginationCache = NULL;
> > + static UINTN PaginationCacheSize = 0;
> > + static UINT32 CurrentPaginationCommand = 0;
> > +
> > + //
> > + // Validate some input parameters.
> > + //
> > + // If either of the pointers are NULL, we can't proceed.
> > + if (CommBuffer == NULL || CommBufferSize == NULL) {
> > + DEBUG(( DEBUG_INFO, "%a - Invalid comm buffer pointers!\n", __FUNCTION__ ));
>
> (2) Non-idiomatic parenthesis style.
>
> > + return EFI_INVALID_PARAMETER;
> > + }
> > + // If the size does not meet a minimum threshold, we cannot proceed.
> > + ExpectedSize = sizeof(VAR_CHECK_POLICY_COMM_HEADER);
> > + if (*CommBufferSize < ExpectedSize) {
> > + DEBUG(( DEBUG_INFO, "%a - Bad comm buffer size! %d < %d\n", __FUNCTION__, *CommBufferSize, ExpectedSize ));
> > + return EFI_INVALID_PARAMETER;
> > + }
> > + // Check the revision and the signature of the comm header.
> > + PolicyCommmHeader = CommBuffer;
> > + if (PolicyCommmHeader->Signature != VAR_CHECK_POLICY_COMM_SIG ||
> > + PolicyCommmHeader->Revision != VAR_CHECK_POLICY_COMM_REVISION) {
> > + DEBUG(( DEBUG_INFO, "%a - Signature or revision are incorrect!\n", __FUNCTION__ ));
> > + // We have verified the buffer is not null and have enough size to hold Result field.
> > + PolicyCommmHeader->Result = EFI_INVALID_PARAMETER;
> > + return EFI_SUCCESS;
> > + }
> > +
> > + // If we're in the middle of a paginated dump and any other command is sent,
> > + // pagination cache must be cleared.
> > + if (PaginationCache != NULL && PolicyCommmHeader->Command != CurrentPaginationCommand) {
> > + FreePool (PaginationCache);
> > + PaginationCache = NULL;
> > + PaginationCacheSize = 0;
> > + CurrentPaginationCommand = 0;
> > + }
> > +
> > + //
> > + // Now we can process the command as it was sent.
> > + //
> > + PolicyCommmHeader->Result = EFI_ABORTED; // Set a default return for incomplete commands.
> > + switch(PolicyCommmHeader->Command) {
> > + case VAR_CHECK_POLICY_COMMAND_DISABLE:
> > + PolicyCommmHeader->Result = DisableVariablePolicy();
> > + break;
> > +
> > + case VAR_CHECK_POLICY_COMMAND_IS_ENABLED:
> > + // Make sure that we're dealing with a reasonable size.
> > + // This add should be safe because these are fixed sizes so far.
> > + ExpectedSize += sizeof(VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS);
> > + if (*CommBufferSize < ExpectedSize) {
> > + DEBUG(( DEBUG_INFO, "%a - Bad comm buffer size! %d < %d\n", __FUNCTION__, *CommBufferSize, ExpectedSize ));
> > + PolicyCommmHeader->Result = EFI_INVALID_PARAMETER;
> > + break;
> > + }
> > +
> > + // Now that we know we've got a valid size, we can fill in the rest of the data.
> > + IsEnabledParams = (VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS*)((UINT8*)CommBuffer +
> sizeof(VAR_CHECK_POLICY_COMM_HEADER));
> > + IsEnabledParams->State = IsVariablePolicyEnabled();
> > + PolicyCommmHeader->Result = EFI_SUCCESS;
> > + break;
> > +
> > + case VAR_CHECK_POLICY_COMMAND_REGISTER:
> > + // Make sure that we're dealing with a reasonable size.
> > + // This add should be safe because these are fixed sizes so far.
> > + ExpectedSize += sizeof(VARIABLE_POLICY_ENTRY);
> > + if (*CommBufferSize < ExpectedSize) {
> > + DEBUG(( DEBUG_INFO, "%a - Bad comm buffer size! %d < %d\n", __FUNCTION__, *CommBufferSize, ExpectedSize ));
> > + PolicyCommmHeader->Result = EFI_INVALID_PARAMETER;
> > + break;
> > + }
> > +
> > + // At the very least, we can assume that we're working with a valid policy entry.
> > + // Time to compare its internal size.
> > + PolicyEntry = (VARIABLE_POLICY_ENTRY*)((UINT8*)CommBuffer + sizeof(VAR_CHECK_POLICY_COMM_HEADER));
> > + if (PolicyEntry->Version != VARIABLE_POLICY_ENTRY_REVISION ||
> > + PolicyEntry->Size < sizeof(VARIABLE_POLICY_ENTRY) ||
> > + EFI_ERROR(SafeUintnAdd(sizeof(VAR_CHECK_POLICY_COMM_HEADER), PolicyEntry->Size, &ExpectedSize)) ||
> > + *CommBufferSize < ExpectedSize) {
> > + DEBUG(( DEBUG_INFO, "%a - Bad policy entry contents!\n", __FUNCTION__ ));
> > + PolicyCommmHeader->Result = EFI_INVALID_PARAMETER;
> > + break;
> > + }
> > +
> > + PolicyCommmHeader->Result = RegisterVariablePolicy( PolicyEntry );
> > + break;
> > +
> > + case VAR_CHECK_POLICY_COMMAND_DUMP:
> > + // Make sure that we're dealing with a reasonable size.
> > + // This add should be safe because these are fixed sizes so far.
> > + ExpectedSize += sizeof(VAR_CHECK_POLICY_COMM_DUMP_PARAMS) + VAR_CHECK_POLICY_MM_DUMP_BUFFER_SIZE;
> > + if (*CommBufferSize < ExpectedSize) {
> > + DEBUG(( DEBUG_INFO, "%a - Bad comm buffer size! %d < %d\n", __FUNCTION__, *CommBufferSize, ExpectedSize ));
> > + PolicyCommmHeader->Result = EFI_INVALID_PARAMETER;
> > + break;
> > + }
> > +
> > + // Now that we know we've got a valid size, we can fill in the rest of the data.
> > + DumpParams = (VAR_CHECK_POLICY_COMM_DUMP_PARAMS*)(PolicyCommmHeader + 1);
> > +
> > + // If we're requesting the first page, initialize the cache and get the sizes.
> > + if (DumpParams->PageRequested == 0) {
> > + if (PaginationCache != NULL) {
> > + FreePool (PaginationCache);
> > + PaginationCache = NULL;
> > + }
> > +
> > + // Determine what the required size is going to be.
> > + DumpParams->TotalSize = 0;
> > + DumpParams->PageSize = 0;
> > + DumpParams->HasMore = FALSE;
> > + SubCommandStatus = DumpVariablePolicy (NULL, &DumpParams->TotalSize);
> > + if (SubCommandStatus == EFI_BUFFER_TOO_SMALL && DumpParams->TotalSize > 0) {
> > + CurrentPaginationCommand = VAR_CHECK_POLICY_COMMAND_DUMP;
> > + PaginationCacheSize = DumpParams->TotalSize;
> > + PaginationCache = AllocatePool (PaginationCacheSize);
> > + if (PaginationCache == NULL) {
> > + SubCommandStatus = EFI_OUT_OF_RESOURCES;
> > + }
> > + }
> > +
> > + // If we've allocated our pagination cache, we're good to cache.
> > + if (PaginationCache != NULL) {
> > + SubCommandStatus = DumpVariablePolicy (PaginationCache, &DumpParams->TotalSize);
> > + }
> > +
> > + // Populate the remaining fields and we can boogie.
> > + if (!EFI_ERROR (SubCommandStatus) && PaginationCache != NULL) {
> > + DumpParams->HasMore = TRUE;
> > + }
> > + }
> > + else if (PaginationCache != NULL) {
>
> (3) The edk2 coding style doesn't break "else"s to new lines.
>
> > + DumpParams->TotalSize = (UINT32)PaginationCacheSize;
> > + DumpParams->PageSize = VAR_CHECK_POLICY_MM_DUMP_BUFFER_SIZE;
> > + DumpOutputBuffer = (UINT8*)(DumpParams + 1);
> > +
> > + // Make sure that we don't over-index the cache.
> > + DumpTotalPages = PaginationCacheSize / DumpParams->PageSize;
> > + if (PaginationCacheSize % DumpParams->PageSize) DumpTotalPages++;
> > + if (DumpParams->PageRequested > DumpTotalPages) {
> > + SubCommandStatus = EFI_INVALID_PARAMETER;
> > + }
> > + else {
> > + // Figure out how far into the page cache we need to go for our next page.
> > + // We know the blind subtraction won't be bad because we already checked for page 0.
> > + DumpInputBuffer = &PaginationCache[DumpParams->PageSize * (DumpParams->PageRequested - 1)];
> > + // If we're getting the last page, adjust the PageSize.
> > + if (DumpParams->PageRequested == DumpTotalPages) {
> > + DumpParams->PageSize = PaginationCacheSize % DumpParams->PageSize;
> > + }
> > + CopyMem (DumpOutputBuffer, DumpInputBuffer, DumpParams->PageSize);
> > + // If we just got the last page, settle up the cache.
> > + if (DumpParams->PageRequested == DumpTotalPages) {
> > + DumpParams->HasMore = FALSE;
> > + FreePool (PaginationCache);
> > + PaginationCache = NULL;
> > + PaginationCacheSize = 0;
> > + CurrentPaginationCommand = 0;
> > + }
> > + // Otherwise, we could do more here.
> > + else {
> > + DumpParams->HasMore = TRUE;
> > + }
> > +
> > + // If we made it this far, we're basically good.
> > + SubCommandStatus = EFI_SUCCESS;
> > + }
> > + }
> > + // If we've requested any other page than 0 and the cache is empty, we must have timed out.
> > + else {
> > + DumpParams->TotalSize = 0;
> > + DumpParams->PageSize = 0;
> > + DumpParams->HasMore = FALSE;
> > + SubCommandStatus = EFI_TIMEOUT;
> > + }
> > +
> > + // There's currently no use for this, but it shouldn't be hard to implement.
> > + PolicyCommmHeader->Result = SubCommandStatus;
> > + break;
> > +
> > + case VAR_CHECK_POLICY_COMMAND_LOCK:
> > + PolicyCommmHeader->Result = LockVariablePolicy();
> > + break;
> > +
> > + default:
> > + // Mark unknown requested command as EFI_UNSUPPORTED.
> > + DEBUG(( DEBUG_INFO, "%a - Invalid command requested! %d\n", __FUNCTION__, PolicyCommmHeader->Command ));
> > + PolicyCommmHeader->Result = EFI_UNSUPPORTED;
> > + break;
> > + }
> > +
> > + DEBUG(( DEBUG_VERBOSE, "%a - Command %d returning %r.\n", __FUNCTION__,
> > + PolicyCommmHeader->Command, PolicyCommmHeader->Result ));
>
> (4) The indentation is not idiomatic.
>
> > +
> > + return Status;
> > +}
> > +
> > +
> > +/**
> > + Constructor function of VarCheckPolicyLib to register VarCheck handler and
> > + SW MMI handlers.
> > +
> > + @param[in] ImageHandle The firmware allocated handle for the EFI image.
> > + @param[in] SystemTable A pointer to the EFI System Table.
> > +
> > + @retval EFI_SUCCESS The constructor executed correctly.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +VarCheckPolicyLibConstructor (
> > + IN EFI_HANDLE ImageHandle,
> > + IN EFI_SYSTEM_TABLE *SystemTable
> > + )
> > +{
> > + EFI_STATUS Status;
> > + EFI_HANDLE DiscardedHandle;
> > +
> > + // Initialize the business logic with the internal GetVariable handler.
> > + Status = InitVariablePolicyLib( VariableServiceGetVariable );
> > +
> > + // Only proceed with init if the business logic could be initialized.
> > + if (!EFI_ERROR( Status )) {
> > + // Register the VarCheck handler for SetVariable filtering.
> > + // Forward the check to the business logic of the library.
> > + VarCheckLibRegisterSetVariableCheckHandler( ValidateSetVariable );
> > +
> > + // Register the MMI handlers for receiving policy commands.
> > + DiscardedHandle = NULL;
> > + Status = gMmst->MmiHandlerRegister( VarCheckPolicyLibMmiHandler,
> > + &gVarCheckPolicyLibMmiHandlerGuid,
> > + &DiscardedHandle );
> > + }
> > + // Otherwise, there's not much we can do.
> > + else {
> > + DEBUG(( DEBUG_ERROR, "%a - Cannot Initialize VariablePolicyLib! %r\n", __FUNCTION__, Status ));
> > + ASSERT_EFI_ERROR( Status );
> > + }
> > +
> > + return Status;
> > +}
> > diff --git a/MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h b/MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h
> > new file mode 100644
> > index 000000000000..77bcc62f3ccf
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h
> > @@ -0,0 +1,54 @@
> > +/** @file -- VarCheckPolicyMmiCommon.h
> > +This header contains communication definitions that are shared between DXE
> > +and the MM component of VarCheckPolicy.
> > +
> > +Copyright (c) Microsoft Corporation.
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > +**/
> > +
> > +#ifndef _VAR_CHECK_POLICY_MMI_COMMON_H_
> > +#define _VAR_CHECK_POLICY_MMI_COMMON_H_
> > +
> > +#define VAR_CHECK_POLICY_COMM_SIG SIGNATURE_32('V', 'C', 'P', 'C')
> > +#define VAR_CHECK_POLICY_COMM_REVISION 1
> > +
> > +#pragma pack(push, 1)
> > +
> > +typedef struct _VAR_CHECK_POLICY_COMM_HEADER {
> > + UINT32 Signature;
> > + UINT32 Revision;
> > + UINT32 Command;
> > + EFI_STATUS Result;
> > +} VAR_CHECK_POLICY_COMM_HEADER;
> > +
> > +typedef struct _VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS {
> > + BOOLEAN State;
> > +} VAR_CHECK_POLICY_COMM_IS_ENABLED_PARAMS;
> > +
> > +typedef struct _VAR_CHECK_POLICY_COMM_DUMP_PARAMS {
> > + UINT32 PageRequested;
> > + UINT32 TotalSize;
> > + UINT32 PageSize;
> > + BOOLEAN HasMore;
> > +} VAR_CHECK_POLICY_COMM_DUMP_PARAMS;
> > +
> > +#pragma pack(pop)
> > +
> > +// Make sure that we will hold at least the headers.
> > +#define VAR_CHECK_POLICY_MM_COMM_BUFFER_SIZE MAX((OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data) + sizeof
> (VAR_CHECK_POLICY_COMM_HEADER) + EFI_PAGES_TO_SIZE(1)), EFI_PAGES_TO_SIZE(4))
> > +#define VAR_CHECK_POLICY_MM_DUMP_BUFFER_SIZE (VAR_CHECK_POLICY_MM_COMM_BUFFER_SIZE - \
> > + (OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data) + \
> > + sizeof(VAR_CHECK_POLICY_COMM_HEADER) + \
> > + sizeof(VAR_CHECK_POLICY_COMM_DUMP_PARAMS)))
> > +STATIC_ASSERT (
> > + VAR_CHECK_POLICY_MM_DUMP_BUFFER_SIZE < VAR_CHECK_POLICY_MM_COMM_BUFFER_SIZE,
> > + "an integer underflow may have occurred calculating VAR_CHECK_POLICY_MM_DUMP_BUFFER_SIZE"
> > + );
> > +
> > +#define VAR_CHECK_POLICY_COMMAND_DISABLE 0x0001
> > +#define VAR_CHECK_POLICY_COMMAND_IS_ENABLED 0x0002
> > +#define VAR_CHECK_POLICY_COMMAND_REGISTER 0x0003
> > +#define VAR_CHECK_POLICY_COMMAND_DUMP 0x0004
> > +#define VAR_CHECK_POLICY_COMMAND_LOCK 0x0005
> > +
> > +#endif // _VAR_CHECK_POLICY_MMI_COMMON_H_
> > diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> > new file mode 100644
> > index 000000000000..7c407e254330
> > --- /dev/null
> > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> > @@ -0,0 +1,44 @@
> > +## @file VarCheckPolicyLib.inf
> > +# This is an instance of a VarCheck lib that leverages the business logic behind
> > +# the VariablePolicy code to make its decisions.
> > +#
> > +##
> > +# Copyright (c) Microsoft Corporation.
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +##
> > +
> > +[Defines]
> > + INF_VERSION = 0x00010005
> > + BASE_NAME = VarCheckPolicyLib
> > + FILE_GUID = 9C28A48F-C884-4B1F-8B95-DEF125448023
> > + MODULE_TYPE = DXE_RUNTIME_DRIVER
> > + VERSION_STRING = 1.0
> > + LIBRARY_CLASS = NULL|DXE_RUNTIME_DRIVER DXE_SMM_DRIVER
> > + CONSTRUCTOR = VarCheckPolicyLibConstructor
> > +
> > +
> > +[Sources]
> > + VarCheckPolicyLib.c
> > +
> > +
> > +[Packages]
> > + MdePkg/MdePkg.dec
> > + MdeModulePkg/MdeModulePkg.dec
> > +
> > +
> > +[LibraryClasses]
> > + BaseLib
> > + DebugLib
> > + BaseMemoryLib
> > + DxeServicesLib
> > + MemoryAllocationLib
> > + VarCheckLib
> > + VariablePolicyLib
> > + VariablePolicyHelperLib
> > + SafeIntLib
> > + MmServicesTableLib
> > +
> > +
> > +[Guids]
> > + gVarCheckPolicyLibMmiHandlerGuid ## CONSUME ## Used to register for MM Communication events.
> > diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
> b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
> > new file mode 100644
> > index 000000000000..eedeeed15d31
> > --- /dev/null
> > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni
> > @@ -0,0 +1,12 @@
> > +// /** @file
> > +// VarCheckPolicyLib.uni
> > +//
> > +// Copyright (c) Microsoft Corporation.
> > +// SPDX-License-Identifier: BSD-2-Clause-Patent
> > +//
> > +// **/
> > +
> > +
> > +#string STR_MODULE_ABSTRACT #language en-US "NULL library implementation that conforms to the VarCheck interface to
> allow VariablePolicy engine to enforce policies"
> > +
> > +#string STR_MODULE_DESCRIPTION #language en-US "NULL library implementation that conforms to the VarCheck interface
> to allow VariablePolicy engine to enforce policies"
> > diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> > index d66002fc9651..ecfa87ffa81d 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -385,6 +385,10 @@
> > ## Include/Guid/EndofS3Resume.h
> > gEdkiiEndOfS3ResumeGuid = { 0x96f5296d, 0x05f7, 0x4f3c, {0x84, 0x67, 0xe4, 0x56, 0x89, 0x0e, 0x0c, 0xb5 } }
> >
> > + ## Used (similar to Variable Services) to communicate policies to the enforcement engine.
> > + # {DA1B0D11-D1A7-46C4-9DC9-F3714875C6EB}
> > + gVarCheckPolicyLibMmiHandlerGuid = { 0xda1b0d11, 0xd1a7, 0x46c4, { 0x9d, 0xc9, 0xf3, 0x71, 0x48, 0x75, 0xc6, 0xeb }}
> > +
>
> (5) Should likely be called gEdkiiVarCheckPolicyLibMmiHandlerGuid.
>
> > ## Include/Guid/S3SmmInitDone.h
> > gEdkiiS3SmmInitDoneGuid = { 0x8f9d4825, 0x797d, 0x48fc, { 0x84, 0x71, 0x84, 0x50, 0x25, 0x79, 0x2e, 0xf6 } }
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
> > index 37795b9e4f58..f0a75a3b337b 100644
> > --- a/MdeModulePkg/MdeModulePkg.dsc
> > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > @@ -313,6 +313,7 @@
> > MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
> > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf
> > + MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> > MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> > MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiLib.inf
> > MdeModulePkg/Library/VarCheckPcdLib/VarCheckPcdLib.inf
> > @@ -458,6 +459,7 @@
> > MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf {
> > <LibraryClasses>
> > + NULL|MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf
> > NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> > NULL|MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiLib.inf
> > NULL|MdeModulePkg/Library/VarCheckPcdLib/VarCheckPcdLib.inf
> >
>
> Running this series through the ECC tool could help improve coding style
> conformance, I believe. (This is not a strong recommendation on my part,
> as I haven't run ECC too many times myself -- I've found ECC way too
> strict in some regards.)
>
Here is ECC wiki https://github.com/tianocore/tianocore.github.io/wiki/ECC-tool
And, we also plan to add ECC checker in open CI. (BZ 2606).
Thanks
Liming
> Thanks
> Laszlo
next prev parent reply other threads:[~2020-05-12 13:37 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20200512064635.14640-1-michael.kubacki@outlook.com>
2020-05-12 6:46 ` [PATCH v2 01/12] MdeModulePkg: Define the VariablePolicy protocol interface Michael Kubacki
2020-05-12 12:19 ` [edk2-devel] " Laszlo Ersek
2020-05-13 4:51 ` [EXTERNAL] " Bret Barkelew
2020-05-13 10:10 ` Laszlo Ersek
2020-05-12 6:46 ` [PATCH v2 02/12] MdeModulePkg: Define the VariablePolicyLib Michael Kubacki
2020-05-12 11:43 ` [edk2-devel] " Laszlo Ersek
2020-05-12 6:46 ` [PATCH v2 03/12] MdeModulePkg: Define the VariablePolicyHelperLib Michael Kubacki
2020-05-12 6:46 ` [PATCH v2 04/12] MdeModulePkg: Define the VarCheckPolicyLib and SMM interface Michael Kubacki
2020-05-12 12:26 ` [edk2-devel] " Laszlo Ersek
2020-05-12 13:37 ` Liming Gao [this message]
2020-05-12 6:46 ` [PATCH v2 05/12] MdeModulePkg: Connect VariablePolicy business logic to VariableServices Michael Kubacki
2020-05-12 6:46 ` [PATCH v2 06/12] MdeModulePkg: Allow VariablePolicy state to delete protected variables Michael Kubacki
2020-05-12 6:46 ` [PATCH v2 07/12] SecurityPkg: Allow VariablePolicy state to delete authenticated variables Michael Kubacki
2020-05-12 6:46 ` [PATCH v2 08/12] MdeModulePkg: Change TCG MOR variables to use VariablePolicy Michael Kubacki
2020-05-12 6:46 ` [PATCH v2 09/12] MdeModulePkg: Drop VarLock from RuntimeDxe variable driver Michael Kubacki
2020-05-12 6:46 ` [PATCH v2 10/12] MdeModulePkg: Add a shell-based functional test for VariablePolicy Michael Kubacki
2020-05-12 6:46 ` [PATCH v2 11/12] OvmfPkg: Add VariablePolicy engine to OvmfPkg platform Michael Kubacki
2020-05-12 12:05 ` [edk2-devel] " Laszlo Ersek
2020-05-12 6:46 ` [PATCH v2 12/12] EmulatorPkg: Add VariablePolicy engine to EmulatorPkg platform Michael Kubacki
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=BN6PR11MB39727188BC284A8BDF09D4E380BE0@BN6PR11MB3972.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