From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8724C21CF58D9 for ; Thu, 5 Oct 2017 00:54:05 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DE78CC04AC50; Thu, 5 Oct 2017 07:57:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DE78CC04AC50 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-16.rdu2.redhat.com [10.10.120.16]) by smtp.corp.redhat.com (Postfix) with ESMTP id 46F045D9C0; Thu, 5 Oct 2017 07:57:25 +0000 (UTC) To: "Zeng, Star" , edk2-devel-01 Cc: "Dong, Eric" , "Yao, Jiewen" , Ladi Prosek References: <20171003212834.25740-1-lersek@redhat.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B97E72A@shsmsx102.ccr.corp.intel.com> From: Laszlo Ersek Message-ID: <4a23f3d3-bb26-bde4-478a-d5135007e55e@redhat.com> Date: Thu, 5 Oct 2017 09:57:24 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B97E72A@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 05 Oct 2017 07:57:27 +0000 (UTC) Subject: Re: [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock inconsistency X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Oct 2017 07:54:05 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Hi Star, On 10/05/17 09:42, Zeng, Star wrote: > 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. Jiewen hasn't given his official R-b yet; he said that the patches looked good to him in general. Beyond an R-b from Jiewen and/or you and/or Eric, I wouldn't like to push the patches until Intel QA (or one of you guys) can regression-test the series, on a platform where TcgMor.inf is included -- that is, on a platform where the MOR and MorLock variables exist *genuinely*. I don't have access to such a platform (OVMF does not support these variables), so I couldn't regression-test the series that way. The variable driver is very important and it is shipped on all physical platforms as well, so we shouldn't push these patches before thorough regression-testing. I'd rather delay committing this set and do a bit more work in RHEL7 downstream (backports) than have to fix an ugly upstream regression in a panic. Please take your time and review the patches and the background discussion in detail. And, again, I would very much appreciate if you guys or someone from Intel QA could fetch the branch and regression-test the work, using a platform that supports MOR and MorLock for real. Thanks! Laszlo > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Wednesday, October 4, 2017 5:28 AM > To: edk2-devel-01 > Cc: Dong, Eric ; Yao, Jiewen ; Ladi Prosek ; Zeng, Star > 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 > Cc: Jiewen Yao > Cc: Ladi Prosek > Cc: Star Zeng > > 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 >