From 00663d047fc948172fd6f020f6c187240d4316be Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Sat, 30 Sep 2017 13:40:32 +0200 Subject: [PATCH] MdeModulePkg/Variable/RuntimeDxe: move SecureBootHook() decl to new header If the platform supports SMM, a gRT->SetVariable() call at boot time results in the following call tree to SecureBootHook(): RuntimeServiceSetVariable() [VariableSmmRuntimeDxe.c, unprivileged] SmmVariableHandler() [VariableSmm.c, PRIVILEGED] VariableServiceSetVariable() [Variable.c, PRIVILEGED] SecureBootHook() [VariableSmm.c, PRIVILEGED] // // do nothing // SecureBootHook() [Measurement.c, unprivileged] // // measure variable if it // is related to SB policy // And if the platform does not support SMM: VariableServiceSetVariable() [Variable.c, unprivileged] SecureBootHook() [Measurement.c, unprivileged] // // measure variable if it // is related to SB policy // In other words, the measurement always happens outside of SMM. Because there are two implementations of the SecureBootHook() API, one that is called from SMM and does nothing, and another that is called outside of SMM and measures variables, the function declaration should be in a header file. This way the compiler can enforce that the function declaration and all function definitions match. "Variable.h" is used for "including common header files, defining internal structures and functions used by Variable modules". Technically, we could declare SecureBootHook() in "Variable.h". However, "Measurement.c" and "VariableSmmRuntimeDxe.c" themselves do not include "Variable.h", and that is likely intentional -- "Variable.h" exposes so much of the privileged variable implementation that it is likely excluded from these C source files on purpose. Therefore introduce a new header file called "PrivilegePolymorphic.h". "Variable.h" includes this header (so that all C source files that have been allowed to see the variable internals learn about the new SecureBootHook() declaration immediately). In "Measurement.c" and "VariableSmmRuntimeDxe.c", include *only* the new header. This change cleans up commit fa0737a839d0 ("MdeModulePkg Variable: Merge from Auth Variable driver in SecurityPkg", 2015-07-01). Cc: Eric Dong Cc: Jiewen Yao Cc: Ladi Prosek Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek Reviewed-by: Jiewen Yao Tested-by: Ladi Prosek --- .../Variable/RuntimeDxe/Measurement.c | 2 + .../RuntimeDxe/PrivilegePolymorphic.h | 38 +++++++++++++++++++ .../Universal/Variable/RuntimeDxe/Variable.c | 14 ------- .../Universal/Variable/RuntimeDxe/Variable.h | 2 + .../RuntimeDxe/VariableRuntimeDxe.inf | 1 + .../Variable/RuntimeDxe/VariableSmm.inf | 1 + .../RuntimeDxe/VariableSmmRuntimeDxe.c | 16 +------- .../RuntimeDxe/VariableSmmRuntimeDxe.inf | 1 + 8 files changed, 47 insertions(+), 28 deletions(-) create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c index a8ed51495e..6acc167224 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c @@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include +#include "PrivilegePolymorphic.h" + typedef struct { CHAR16 *VariableName; EFI_GUID *VendorGuid; diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h new file mode 100644 index 0000000000..0aa0d4f48f --- /dev/null +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h @@ -0,0 +1,38 @@ +/** @file + Polymorphic functions that are called from both the privileged driver (i.e., + the DXE_SMM variable module) and the non-privileged drivers (i.e., one or + both of the DXE_RUNTIME variable modules). + + Each of these functions has two implementations, appropriate for privileged + vs. non-privileged driver code. + + Copyright (c) 2017, Red Hat, Inc.
+ Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+ + This program and the accompanying materials are licensed and made available + under the terms and conditions of the BSD License which accompanies this + distribution. The full text of the license may be found at + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. +**/ +#ifndef _PRIVILEGE_POLYMORPHIC_H_ +#define _PRIVILEGE_POLYMORPHIC_H_ + +#include + +/** + SecureBoot Hook for auth variable update. + + @param[in] VariableName Name of Variable to be found. + @param[in] VendorGuid Variable vendor GUID. +**/ +VOID +EFIAPI +SecureBootHook ( + IN CHAR16 *VariableName, + IN EFI_GUID *VendorGuid + ); + +#endif diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c index 71a6fd2093..28e4ac8f38 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c @@ -97,20 +97,6 @@ AUTH_VAR_LIB_CONTEXT_IN mAuthContextIn = { AUTH_VAR_LIB_CONTEXT_OUT mAuthContextOut; -/** - - SecureBoot Hook for auth variable update. - - @param[in] VariableName Name of Variable to be found. - @param[in] VendorGuid Variable vendor GUID. -**/ -VOID -EFIAPI -SecureBootHook ( - IN CHAR16 *VariableName, - IN EFI_GUID *VendorGuid - ); - /** Initialization for MOR Lock Control. diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h index 8b1b1332b3..ec9b9849ec 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h @@ -44,6 +44,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include +#include "PrivilegePolymorphic.h" + #define EFI_VARIABLE_ATTRIBUTES_MASK (EFI_VARIABLE_NON_VOLATILE | \ EFI_VARIABLE_BOOTSERVICE_ACCESS | \ EFI_VARIABLE_RUNTIME_ACCESS | \ diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf index bc24a251c8..e840fc9bff 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf @@ -41,6 +41,7 @@ Variable.c VariableDxe.c Variable.h + PrivilegePolymorphic.h Measurement.c TcgMorLockDxe.c VarCheck.c diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf index ccfb6fc740..4041643665 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf @@ -51,6 +51,7 @@ VariableSmm.c VarCheck.c Variable.h + PrivilegePolymorphic.h VariableExLib.c TcgMorLockSmm.c diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c index e209d54755..85d655dc19 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c @@ -44,6 +44,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include +#include "PrivilegePolymorphic.h" + EFI_HANDLE mHandle = NULL; EFI_SMM_VARIABLE_PROTOCOL *mSmmVariable = NULL; EFI_EVENT mVirtualAddressChangeEvent = NULL; @@ -56,20 +58,6 @@ EFI_LOCK mVariableServicesLock; EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock; EDKII_VAR_CHECK_PROTOCOL mVarCheck; -/** - SecureBoot Hook for SetVariable. - - @param[in] VariableName Name of Variable to be found. - @param[in] VendorGuid Variable vendor GUID. - -**/ -VOID -EFIAPI -SecureBootHook ( - IN CHAR16 *VariableName, - IN EFI_GUID *VendorGuid - ); - /** Some Secure Boot Policy Variable may update following other variable changes(SecureBoot follows PK change, etc). Record their initial State when variable write service is ready. diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf index 9975f5ae1d..bd73f7ac29 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf @@ -42,6 +42,7 @@ [Sources] VariableSmmRuntimeDxe.c + PrivilegePolymorphic.h Measurement.c [Packages] -- 2.39.2