On Sat, Mar 13, 2021 at 12:06 AM Ard Biesheuvel <a...@kernel.org> wrote:
>
> 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 this
> 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 <a...@kernel.org>
> ---
> 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/MdeModulePkg/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
> = { VarCheckRegis
>
> VarCheckVariablePropertySet,
>
> VarCheckVariablePropertyGet };
>
> +STATIC EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL mFvbProtocolShadow;
> +
> /**
> 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.
> @@ -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 = mVariableModuleGlobal->FvbInstance;
> + if (FvbInstance != NULL &&
> + FvbInstance->GetBlockSize != mFvbProtocolShadow.GetBlockSize) {
> + FvbInstance->GetBlockSize = mFvbProtocolShadow.GetBlockSize;
> + FvbInstance->GetPhysicalAddress =
> mFvbProtocolShadow.GetPhysicalAddress;
> + FvbInstance->GetAttributes = mFvbProtocolShadow.GetAttributes;
> + FvbInstance->SetAttributes = mFvbProtocolShadow.SetAttributes;
> + FvbInstance->Read = mFvbProtocolShadow.Read;
> + FvbInstance->Write = mFvbProtocolShadow.Write;
> + FvbInstance->EraseBlocks = mFvbProtocolShadow.EraseBlocks;
> + }
> }
> }
>
> @@ -247,13 +267,22 @@ VariableClassAddressChangeEvent (
> UINTN Index;
>
> if (mVariableModuleGlobal->FvbInstance != NULL) {
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->GetBlockSize);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->GetPhysicalAddress);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->GetAttributes);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->SetAttributes);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->Read);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->Write);
> - EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->FvbInstance->EraseBlocks);
> + //
> + // This module did not produce the FVB protocol instance that provides
> the
> + // variable store, so it should not be the one performing the pointer
> + // conversion on its members. However, some FVB protocol implementations
> + // may rely on this behavior, which was present in older versions of this
> + // driver, so we need to perform the conversion if the protocol producer
> + // fails to do so. So let's record the converted values now, and swap
> them
> + // in later if they haven't changed values.
> + //
> + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.GetBlockSize);
> + EfiConvertPointer (0x0, (VOID **)&mFvbProtocolShadow.GetPhysicalAddress);
> + 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->FvbInstance);
> }
> EfiConvertPointer (0x0, (VOID **)
> &mVariableModuleGlobal->PlatformLangCodes);
> @@ -454,6 +483,13 @@ FtwNotificationEvent (
> }
> mVariableModuleGlobal->FvbInstance = FvbProtocol;
>
> + //
> + // Store the boot time values of the function pointers so we can compare
> + // them later. This is needed to avoid double conversion during the call
> + // to SetVirtualAddressMap.
> + //
> + CopyMem (&mFvbProtocolShadow, FvbProtocol, sizeof mFvbProtocolShadow);
> +
> //
> // Mark the variable storage region of the FLASH as RUNTIME.
> //
> --
> 2.30.1
>
You can add my
Tested-By: Jon Nettleton <j...@solid-run.com>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72740): https://edk2.groups.io/g/devel/message/72740
Mute This Topic: https://groups.io/mt/81292874/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
