public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Eric Dong <eric.dong@intel.com>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Ladi Prosek <lprosek@redhat.com>, Star Zeng <star.zeng@intel.com>
Subject: [PATCH 1/6] MdeModulePkg/Variable/RuntimeDxe: move SecureBootHook() decl to new header
Date: Tue,  3 Oct 2017 23:28:29 +0200	[thread overview]
Message-ID: <20171003212834.25740-2-lersek@redhat.com> (raw)
In-Reply-To: <20171003212834.25740-1-lersek@redhat.com>

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 <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ladi Prosek <lprosek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf    |  1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf           |  1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |  1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h    | 38 ++++++++++++++++++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h                |  2 ++
 MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c             |  2 ++
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c                | 14 --------
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   | 16 ++-------
 8 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index bc24a251c894..e840fc9bff40 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -41,6 +41,7 @@ [Sources]
   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 ccfb6fc740c1..404164366579 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
@@ -51,6 +51,7 @@ [Sources]
   VariableSmm.c
   VarCheck.c
   Variable.h
+  PrivilegePolymorphic.h
   VariableExLib.c
   TcgMorLockSmm.c
 
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
index 9975f5ae1d6e..bd73f7ac29f2 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
@@ -42,6 +42,7 @@ [Defines]
 
 [Sources]
   VariableSmmRuntimeDxe.c
+  PrivilegePolymorphic.h
   Measurement.c
 
 [Packages]
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h
new file mode 100644
index 000000000000..0aa0d4f48f10
--- /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.<BR>
+  Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+
+  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 <Uefi/UefiBaseType.h>
+
+/**
+  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.h b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h
index 8b1b1332b3da..ec9b9849ec09 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 <Guid/FaultTolerantWrite.h>
 #include <Guid/VarErrorFlag.h>
 
+#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/Measurement.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
index a8ed51495e2a..6acc167224ba 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 <Library/BaseLib.h>
 #include <Library/TpmMeasurementLib.h>
 
+#include "PrivilegePolymorphic.h"
+
 typedef struct {
   CHAR16                                 *VariableName;
   EFI_GUID                               *VendorGuid;
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 71a6fd209364..28e4ac8f3819 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/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index e209d54755ef..85d655dc19ff 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 <Guid/EventGroup.h>
 #include <Guid/SmmVariableCommon.h>
 
+#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.
-- 
2.14.1.3.gb7cf6e02401b




  reply	other threads:[~2017-10-03 21:25 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-03 21:28 [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock inconsistency Laszlo Ersek
2017-10-03 21:28 ` Laszlo Ersek [this message]
2017-10-03 21:28 ` [PATCH 2/6] MdeModulePkg/Variable/RuntimeDxe: move MOR func. declarations to header Laszlo Ersek
2017-10-09  6:55   ` Zeng, Star
2017-10-09 12:47     ` Laszlo Ersek
2017-10-03 21:28 ` [PATCH 3/6] MdeModulePkg/Variable/RuntimeDxe: introduce MorLockInitAtEndOfDxe() hook Laszlo Ersek
2017-10-03 21:28 ` [PATCH 4/6] MdeModulePkg/Variable/RuntimeDxe: permit MorLock deletion for passthru req Laszlo Ersek
2017-10-03 21:28 ` [PATCH 5/6] MdeModulePkg/Variable/RuntimeDxe: delay MorLock creation until EndOfDxe Laszlo Ersek
2017-10-03 21:28 ` [PATCH 6/6] MdeModulePkg/Variable/RuntimeDxe: delete and lock OS-created MOR variable Laszlo Ersek
2017-10-09  7:12   ` Zeng, Star
2017-10-09 15:20     ` Laszlo Ersek
2017-10-10  4:15       ` Yao, Jiewen
2017-10-10 13:14         ` Zeng, Star
2017-10-04  1:18 ` [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock inconsistency Yao, Jiewen
2017-10-04 10:39   ` Laszlo Ersek
2017-10-04 12:24     ` Ladi Prosek
2017-10-10  4:17     ` Yao, Jiewen
2017-10-10 10:09       ` Laszlo Ersek
2017-10-10 12:16         ` Zeng, Star
2017-10-05  7:42 ` Zeng, Star
2017-10-05  7:57   ` Laszlo Ersek
2017-10-05  9:12     ` Yao, Jiewen

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=20171003212834.25740-2-lersek@redhat.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