Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Take a stab at solving the lock ordering problem
  • Loading branch information
Bret Barkelew committed Sep 27, 2020
1 parent 00014a6 commit e7d0164
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 76 deletions.
10 changes: 7 additions & 3 deletions MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
Expand Up @@ -302,6 +302,9 @@ OnReadyToBoot (

if (!mEndOfDxe) {
MorLockInitAtEndOfDxe ();

Status = LockVariablePolicy ();
ASSERT_EFI_ERROR (Status);
//
// Set the End Of DXE bit in case the EFI_END_OF_DXE_EVENT_GROUP_GUID event is not signaled.
//
Expand All @@ -321,9 +324,6 @@ OnReadyToBoot (
}
}

Status = LockVariablePolicy ();
ASSERT_EFI_ERROR (Status);

gBS->CloseEvent (Event);
}

Expand All @@ -343,8 +343,12 @@ OnEndOfDxe (
VOID *Context
)
{
EFI_STATUS Status;

DEBUG ((EFI_D_INFO, "[Variable]END_OF_DXE is signaled\n"));
MorLockInitAtEndOfDxe ();
Status = LockVariablePolicy ();
ASSERT_EFI_ERROR (Status);
mEndOfDxe = TRUE;
mVarCheckAddressPointer = VarCheckLibInitializeAtEndOfDxe (&mVarCheckAddressPointerCount);
//
Expand Down
77 changes: 4 additions & 73 deletions MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c
Expand Up @@ -450,57 +450,6 @@ InitMmCommonCommBuffer (
}


/**
This helper is responsible for telemetry and any other actions that
need to be taken if the VariablePolicy fails to lock.
NOTE: It's possible that parts of this handling will need to become
part of a platform policy.
@param[in] FailureStatus The failure that was reported by LockVariablePolicy
**/
STATIC
VOID
VariablePolicyHandleFailureToLock (
IN EFI_STATUS FailureStatus
)
{
// For now, there's no agreed-upon policy for this.
return;
}


/**
EndOfDxe Callback
Lock the VariablePolicy interface if it hasn't already been locked.
@param[in] Event Event whose notification function is being invoked
@param[in] Context Pointer to the notification function's context
**/
STATIC
VOID
EFIAPI
LockPolicyInterfaceAtEndOfDxe (
IN EFI_EVENT Event,
IN VOID *Context
)
{
EFI_STATUS Status;

Status = ProtocolLockVariablePolicy();

if (EFI_ERROR( Status )) {
VariablePolicyHandleFailureToLock( Status );
}
else {
gBS->CloseEvent( Event );
}

}


/**
Convert internal pointer addresses to virtual addresses.
Expand Down Expand Up @@ -540,14 +489,11 @@ VariablePolicySmmDxeMain (
{
EFI_STATUS Status;
BOOLEAN ProtocolInstalled;
BOOLEAN CallbackRegistered;
BOOLEAN VirtualAddressChangeRegistered;
EFI_EVENT EndOfDxeEvent;
EFI_EVENT VirtualAddressChangeEvent;

Status = EFI_SUCCESS;
ProtocolInstalled = FALSE;
CallbackRegistered = FALSE;
VirtualAddressChangeRegistered = FALSE;

// Update the minimum buffer size.
Expand Down Expand Up @@ -588,22 +534,10 @@ VariablePolicySmmDxeMain (
ProtocolInstalled = TRUE;
}

//
// Register a callback for EndOfDxe so that the interface is at least locked before
// dispatching any bootloaders or UEFI apps.
Status = gBS->CreateEventEx( EVT_NOTIFY_SIGNAL,
TPL_CALLBACK,
LockPolicyInterfaceAtEndOfDxe,
NULL,
&gEfiEndOfDxeEventGroupGuid,
&EndOfDxeEvent );
if (EFI_ERROR( Status )) {
DEBUG(( DEBUG_ERROR, "%a - Failed to create EndOfDxe event! %r\n", __FUNCTION__, Status ));
goto Exit;
}
else {
CallbackRegistered = TRUE;
}
// Normally, we might want to register a callback
// to lock the interface, but this is integrated
// into the existing callbacks in VaraiableSmm.c
// and VariableDxe.c.

//
// Register a VirtualAddressChange callback for the MmComm protocol and Comm buffer.
Expand All @@ -630,9 +564,6 @@ VariablePolicySmmDxeMain (
if (ProtocolInstalled) {
gBS->UninstallProtocolInterface( &ImageHandle, &gEdkiiVariablePolicyProtocolGuid, &mVariablePolicyProtocol );
}
if (CallbackRegistered) {
gBS->CloseEvent( EndOfDxeEvent );
}
if (VirtualAddressChangeRegistered) {
gBS->CloseEvent( VirtualAddressChangeEvent );
}
Expand Down
7 changes: 7 additions & 0 deletions MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
Expand Up @@ -27,6 +27,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Protocol/SmmVarCheck.h>

#include <Library/MmServicesTableLib.h>
#include <Library/VariablePolicyLib.h>

#include <Guid/SmmVariableCommon.h>
#include "Variable.h"
Expand Down Expand Up @@ -689,6 +690,8 @@ SmmVariableHandler (
}
if (!mEndOfDxe) {
MorLockInitAtEndOfDxe ();
Status = LockVariablePolicy ();
ASSERT_EFI_ERROR (Status);
mEndOfDxe = TRUE;
VarCheckLibInitializeAtEndOfDxe (NULL);
//
Expand Down Expand Up @@ -974,8 +977,12 @@ SmmEndOfDxeCallback (
IN EFI_HANDLE Handle
)
{
EFI_STATUS Status;

DEBUG ((EFI_D_INFO, "[Variable]SMM_END_OF_DXE is signaled\n"));
MorLockInitAtEndOfDxe ();
Status = LockVariablePolicy ();
ASSERT_EFI_ERROR (Status);
mEndOfDxe = TRUE;
VarCheckLibInitializeAtEndOfDxe (NULL);
//
Expand Down

0 comments on commit e7d0164

Please sign in to comment.