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>,
	Star Zeng <star.zeng@intel.com>
Subject: [PATCH] MdeModulePkg/Variable/RuntimeDxe: delete & lock MOR in the absence of SMM
Date: Tue, 10 Oct 2017 15:24:43 +0200	[thread overview]
Message-ID: <20171010132443.11383-1-lersek@redhat.com> (raw)

VariableRuntimeDxe deletes and locks the MorLock variable in
MorLockInit(), with the argument that any protection provided by MorLock
can be circumvented if MorLock can be overwritten by unprivileged code
(i.e., outside of SMM).

Extend the argument and the logic to the MOR variable, which is supposed
to be protected by MorLock.

This change was suggested by Star; it is inspired by earlier VariableSmm
commit fda8f631edbb ("MdeModulePkg/Variable/RuntimeDxe: delete and lock
OS-created MOR variable", 2017-10-03).

Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Suggested-by: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    Repo:   https://github.com/lersek/edk2.git
    Branch: del_and_lock_mor_without_smm

 MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c | 24 ++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
index 7142e2da2073..1610c7aa0706 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
@@ -69,29 +69,53 @@ EFI_STATUS
 MorLockInit (
   VOID
   )
 {
   //
   // Always clear variable to report unsupported to OS.
   // The reason is that the DXE version is not proper to provide *protection*.
   // BIOS should use SMM version variable driver to provide such capability.
   //
   VariableServiceSetVariable (
     MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME,
     &gEfiMemoryOverwriteRequestControlLockGuid,
     EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
     0,
     NULL
     );
 
   //
   // Need set this variable to be read-only to prevent other module set it.
   //
   VariableLockRequestToLock (&mVariableLock, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, &gEfiMemoryOverwriteRequestControlLockGuid);
+
+  //
+  // The MOR variable can effectively improve platform security only when the
+  // MorLock variable protects the MOR variable. In turn MorLock cannot be made
+  // secure without SMM support in the platform firmware (see above).
+  //
+  // Thus, delete the MOR variable, should it exist for any reason (some OSes
+  // are known to create MOR unintentionally, in an attempt to set it), then
+  // also lock the MOR variable, in order to prevent other modules from
+  // creating it.
+  //
+  VariableServiceSetVariable (
+    MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,
+    &gEfiMemoryOverwriteControlDataGuid,
+    0,                                      // Attributes
+    0,                                      // DataSize
+    NULL                                    // Data
+    );
+  VariableLockRequestToLock (
+    &mVariableLock,
+    MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,
+    &gEfiMemoryOverwriteControlDataGuid
+    );
+
   return EFI_SUCCESS;
 }
 
 /**
   Delayed initialization for MOR Control Lock at EndOfDxe.
 
   This function performs any operations queued by MorLockInit().
 **/
-- 
2.14.1.3.gb7cf6e02401b



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

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-10 13:24 Laszlo Ersek [this message]
2017-10-10 13:54 ` [PATCH] MdeModulePkg/Variable/RuntimeDxe: delete & lock MOR in the absence of SMM Zeng, Star
2017-10-10 17:22   ` Laszlo Ersek
2017-10-24  2:19     ` Zeng, Star

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=20171010132443.11383-1-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