Re: [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock inconsistency

["Zeng, Star" <star.zeng@intel.com>]

Laszlo,

If the series is not so urgent to be pushed, I want to take some time(of maybe one or two days) to look at the discussion background and the patches.
If it is urgent, go ahead to push the patches if you have got Jiewen's RB.


Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Wednesday, October 4, 2017 5:28 AM
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ladi Prosek <lprosek@redhat.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock inconsistency

Repo:   https://github.com/lersek/edk2.git
Branch: mor_lock_init_at_end_of_dxe

This patch set fixes the issue reported in the following items:

* Inconsistent MOR control variables exposed by OVMF, breaks Windows
  Device Guard

  https://bugzilla.redhat.com/show_bug.cgi?id=1496170

* VariableSmm MorLockInit(): create MORLock only if / after MOR exists

  https://bugzilla.tianocore.org/show_bug.cgi?id=727

Patches #1 through #3 are cleanups.

Patch #4 is a small helper patch for patch #5.

Patch #5 is the actual fix, following Jiewen's suggestions from the edk2-devel thread

* [edk2] multiple levels of support for MOR / MORLock

  https://lists.01.org/pipermail/edk2-devel/2017-September/015444.html
  https://lists.01.org/pipermail/edk2-devel/2017-October/015530.html

Patch #6 is a workaround for some OSes (minimally Fedora 24-26, and some Debian versions) that create the MOR variable even if the platform doesn't offer it up-front. This patch also follows Jiewen's suggestion from the same edk2-devel thread.

(

BTW, at Paolo's recommendation, I've now reported this kernel issue for Fedora, under

* incorrect downstream-only Platform Reset Attack Mitigation patch in
  the F24-F26 kernels

  https://bugzilla.redhat.com/show_bug.cgi?id=1498159

)

I've checked this set for basic regressions, using OVMF, normal boot and
S3 suspend/resume:

* Q35, SMM, IA32:
  - Fedora 25 -- verified patch #6 specifically

* i440fx, no SMM, X64:
  - Fedora 24

* Q35, SMM, IA32X64:
  - Fedora 26 -- verified patch #6 specifically
  - Windows 7
  - Windows 8.1
  - Windows 10
  - Windows Server 2008 R2
  - Windows Server 2012 R2

I didn't / couldn't test this set in the following two environments:

- on platforms where TcgMor.inf is included in the firmware, and the MOR
  variable exists genuinely,

- in the nested virt setup where Ladi reported the Device Guard
  breakage. (If I understand correctly, ATM this requires additional
  host kernel (KVM) patches.)

Test results / feedback from those envs would be appreciated.

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>

Thanks,
Laszlo

Laszlo Ersek (6):
  MdeModulePkg/Variable/RuntimeDxe: move SecureBootHook() decl to new
    header
  MdeModulePkg/Variable/RuntimeDxe: move MOR func. declarations to
    header
  MdeModulePkg/Variable/RuntimeDxe: introduce MorLockInitAtEndOfDxe()
    hook
  MdeModulePkg/Variable/RuntimeDxe: permit MorLock deletion for passthru
    req
  MdeModulePkg/Variable/RuntimeDxe: delay MorLock creation until
    EndOfDxe
  MdeModulePkg/Variable/RuntimeDxe: delete and lock OS-created MOR
    variable

 MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c             |   2 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h    |  89 ++++++++++
 MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c           |  45 +++--
 MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c           | 173 ++++++++++++++++++--
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c                |  51 ------
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h                |   2 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c             |   2 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf    |   1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c             |   2 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf           |   4 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c   |  16 +-
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf |   1 +
 12 files changed, 294 insertions(+), 94 deletions(-)  create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h

--
2.14.1.3.gb7cf6e02401b