From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web10.10.1615590365605140805 for ; Fri, 12 Mar 2021 15:06:05 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@kernel.org header.s=k20201202 header.b=ZLJzma7T; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 5BDC164F49; Fri, 12 Mar 2021 23:06:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1615590365; bh=nfIucucp+Kl+V91Vz2lK0P8/301BU9mgMIENX3NMU5Q=; h=From:To:Cc:Subject:Date:From; b=ZLJzma7TJX1DD3dCax6F+SbPtWz2mWMQhhW6CqP2+t1IAxf8qMoy+M4RAetxnWGIP rFd5++ggg2yc6MK1tKfttxa8JuuTyJ2l9qQR9fP/RxJQDhYQPmE8zf5UmIOEgZQBut HN0glXYoN+QUZ8elOv0WI6fPcwSne5t/dUG03cLHaOPpfqxW5A2PlzOm8wMaAKbQrM XzkF7yCoRvUabrYmnxcTUoRhT+FpYvF52kWTIUHzY5iQVF90rwZwlez4pjfAHXdM7Z 5m7xMoI8Cn41wsps/CzA2nQNCvgvrTCYiJh7IYWBHcvkLrnxGkdCkeGeS5mAP6jaRL cEkcEUEIsZCwA== From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: lersek@redhat.com, liming.gao@intel.com, jon@solid-run.com, leif@nuviainc.com, Ard Biesheuvel Subject: [PATCH 1/1] MdeModulePkg/VariableRuntimeDxe: avoid double VA conversion of FVB protocol Date: Sat, 13 Mar 2021 00:05:54 +0100 Message-Id: <20210312230554.292964-1-ardb@kernel.org> X-Mailer: git-send-email 2.30.1 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable For historical reasons, the VariableRuntimeDxe performs virtual address conversion on the FVB protocol member pointers of the protocol instance that backs the EFI variable store. However, the driver that produces the actual instance should be doing this, as it is the owner, and provides the actual implementation of those methods. Unfortunately, we cannot simply change this: existing drivers may rely on this behavior, and so the variable driver should take care to only convert the pointers when necessary, but leave them alone when the owner is taking care of this. The virtual address switch event can be delivered in arbitrary order, and so we should take care of this in a way that does not rely on whether thi= s driver converts its pointers either before or after the owner of the FVB protocol receives the event. So let's not convert the pointers at all when the event is delivered, but record the converted addresses in a shadow FVB protocol. This way, we can check when the variable driver is invoked at runtime whether the switch has occurred or not, and perform the switch if it hasn't. Signed-off-by: Ard Biesheuvel --- Build tested only. MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 50 ++++++++++= +++++++--- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/M= deModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c index 0fca0bb2a9b5..3d83ac4452ee 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c @@ -37,6 +37,8 @@ EDKII_VAR_CHECK_PROTOCOL mVarCheck = =3D { VarCheckRegis VarC= heckVariablePropertySet, VarC= heckVariablePropertyGet }; =20 +STATIC EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolShadow; + /** Some Secure Boot Policy Variable may update following other variable c= hanges(SecureBoot follows PK change, etc). Record their initial State when variable write service is ready. @@ -105,8 +107,26 @@ AcquireLockOnlyAtBootTime ( IN EFI_LOCK *Lock ) { + EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL *FvbInstance; + if (!AtRuntime ()) { EfiAcquireLock (Lock); + } else { + // + // Check whether we need to swap in the converted pointer values for= the + // FVB protocol methods + // + FvbInstance =3D mVariableModuleGlobal->FvbInstance; + if (FvbInstance !=3D NULL && + FvbInstance->GetBlockSize !=3D mFvbProtocolShadow.GetBlockSize) = { + FvbInstance->GetBlockSize =3D mFvbProtocolShadow.GetBlockSiz= e; + FvbInstance->GetPhysicalAddress =3D mFvbProtocolShadow.GetPhysical= Address; + FvbInstance->GetAttributes =3D mFvbProtocolShadow.GetAttribut= es; + FvbInstance->SetAttributes =3D mFvbProtocolShadow.SetAttribut= es; + FvbInstance->Read =3D mFvbProtocolShadow.Read; + FvbInstance->Write =3D mFvbProtocolShadow.Write; + FvbInstance->EraseBlocks =3D mFvbProtocolShadow.EraseBlocks= ; + } } } =20 @@ -247,13 +267,22 @@ VariableClassAddressChangeEvent ( UINTN Index; =20 if (mVariableModuleGlobal->FvbInstance !=3D NULL) { - EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstanc= e->GetBlockSize); - EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstanc= e->GetPhysicalAddress); - EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstanc= e->GetAttributes); - EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstanc= e->SetAttributes); - EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstanc= e->Read); - EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstanc= e->Write); - EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstanc= e->EraseBlocks); + // + // This module did not produce the FVB protocol instance that provid= es the + // variable store, so it should not be the one performing the pointe= r + // conversion on its members. However, some FVB protocol implementat= ions + // may rely on this behavior, which was present in older versions of= this + // driver, so we need to perform the conversion if the protocol prod= ucer + // fails to do so. So let's record the converted values now, and swa= p them + // in later if they haven't changed values. + // + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.GetBlockSize); + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.GetPhysicalAddr= ess); + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.GetAttributes); + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.SetAttributes); + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.Read); + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.Write); + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.EraseBlocks); EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->FvbInstanc= e); } EfiConvertPointer (0x0, (VOID **) &mVariableModuleGlobal->PlatformLang= Codes); @@ -454,6 +483,13 @@ FtwNotificationEvent ( } mVariableModuleGlobal->FvbInstance =3D FvbProtocol; =20 + // + // Store the boot time values of the function pointers so we can compa= re + // them later. This is needed to avoid double conversion during the ca= ll + // to SetVirtualAddressMap. + // + CopyMem (&mFvbProtocolShadow, FvbProtocol, sizeof mFvbProtocolShadow); + // // Mark the variable storage region of the FLASH as RUNTIME. // --=20 2.30.1