Skip to content

Commit

Permalink
Remove AdvancedLogger MmCoreArm dependence on global variables. (#437)
Browse files Browse the repository at this point in the history
## Description

The Advanced logger Arm MM Core library may be invoked before the
imagine has completed it's PeCOFF section attribute fixup. Previously
this was handled by explicitly mapping the data page need for the adv
logger globals to be writable, but this leads to potential infinite
recursion if any of the functions in the MMU stack calls a print.
Instead this change is to remove the dependence on global variables and
so remove the need for external calls in these library functions.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

Tested and validated presence of MM logs in serial output and adv logger
buffer.

## Integration Instructions

N/A
  • Loading branch information
cfernald authored Feb 12, 2024
1 parent 9855702 commit 06c7662
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 89 deletions.
130 changes: 42 additions & 88 deletions AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,63 +15,13 @@
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/SynchronizationLib.h>
#include <Library/MmuLib.h>

#include "../AdvancedLoggerCommon.h"

#define ADV_LOGGER_MIN_SIZE (65536)

STATIC ADVANCED_LOGGER_INFO *mLoggerInfo = NULL;
STATIC UINT32 mBufferSize = 0;
STATIC EFI_PHYSICAL_ADDRESS mMaxAddress = 0;
STATIC BOOLEAN mInitialized = FALSE;

/**
Validate Info Blocks
The address of the ADVANCE_LOGGER_INFO block pointer is captured during the first debug print. The
pointers LogBuffer and LogCurrent, and LogBufferSize, could be written to by untrusted code. Here,
we check that the pointers are within the allocated mLoggerInfo space, and that LogBufferSize, which
is used in multiple places to see if a new message will fit into the log buffer, is valid.
@param NONE
@return BOOLEAN TRUE = mLoggerInfo Block passes security checks
@return BOOLEAN FALSE= mLoggerInfo Block failed security checks
**/
STATIC
BOOLEAN
ValidateInfoBlock (
VOID
)
{
if (mLoggerInfo == NULL) {
return FALSE;
}

if (mLoggerInfo->Signature != ADVANCED_LOGGER_SIGNATURE) {
return FALSE;
}

if (mLoggerInfo->LogBuffer != (PA_FROM_PTR (mLoggerInfo + 1))) {
return FALSE;
}

if ((mLoggerInfo->LogCurrent > mMaxAddress) ||
(mLoggerInfo->LogCurrent < mLoggerInfo->LogBuffer))
{
return FALSE;
}

if (mBufferSize == 0) {
mBufferSize = mLoggerInfo->LogBufferSize;
} else if (mLoggerInfo->LogBufferSize != mBufferSize) {
return FALSE;
}

return TRUE;
}
//
// NO GLOBALS! This routine may run before data sections are writable, and so
// cannot presume globals will be available.
//

/**
The logger Information Block is carved from the Trust Zone at a specific fixed address.
Expand All @@ -87,8 +37,8 @@ ValidateInfoBlock (
PcdAdvancedLoggerCarBase -- NOT USED, leave at default
PcdAdvancedLoggerPreMemPages -- NOT USED, leave at default
NOTE: A debug statement here will cause recursion. Insure that the recursion will be
a straight path just to return the existing mLoggerInfo.
NOTE: A debug statement here will cause recursion. Avoid debug statements
or calls to external functions that may contain debug prints.
@param - None
Expand All @@ -102,48 +52,52 @@ AdvancedLoggerGetLoggerInfo (
VOID
)
{
EFI_STATUS Status;
EFI_PHYSICAL_ADDRESS Address;
ADVANCED_LOGGER_INFO *LoggerInfo;
EFI_PHYSICAL_ADDRESS MaxAddress;

ASSERT (FeaturePcdGet (PcdAdvancedLoggerFixedInRAM));
if (!FeaturePcdGet (PcdAdvancedLoggerFixedInRAM)) {
return NULL;
}

if (!mInitialized) {
// If this is the first time for MM core to get here, the memory attributes of this module
// may not be fully set yet. Thus set the memory for global variables attributes to RW first.
Address = ALIGN_VALUE ((EFI_PHYSICAL_ADDRESS)(UINTN)&mInitialized - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE);
Status = MmuSetAttributes (Address, EFI_PAGE_SIZE, EFI_MEMORY_XP);
ASSERT_EFI_ERROR (Status);
Status = MmuClearAttributes (Address, EFI_PAGE_SIZE, EFI_MEMORY_RO);
ASSERT_EFI_ERROR (Status);

Address = ALIGN_VALUE ((EFI_PHYSICAL_ADDRESS)(UINTN)&mLoggerInfo - EFI_PAGE_SIZE + 1, EFI_PAGE_SIZE);
Status = MmuSetAttributes (Address, EFI_PAGE_SIZE, EFI_MEMORY_XP);
ASSERT_EFI_ERROR (Status);
Status = MmuClearAttributes (Address, EFI_PAGE_SIZE, EFI_MEMORY_RO);
ASSERT_EFI_ERROR (Status);

mInitialized = TRUE; // Only allow initialization once
mLoggerInfo = (ADVANCED_LOGGER_INFO *)(VOID *)FixedPcdGet64 (PcdAdvancedLoggerBase);
ASSERT (mLoggerInfo != NULL);
if (mLoggerInfo == NULL) {
return NULL;
}

mLoggerInfo->HwPrintLevel = FixedPcdGet32 (PcdAdvancedLoggerHdwPortDebugPrintErrorLevel);

mMaxAddress = mLoggerInfo->LogBuffer + mLoggerInfo->LogBufferSize;
mBufferSize = mLoggerInfo->LogBufferSize;
LoggerInfo = (ADVANCED_LOGGER_INFO *)(VOID *)FixedPcdGet64 (PcdAdvancedLoggerBase);
if (LoggerInfo == NULL) {
return NULL;
}

//
// The pointers LogBuffer and LogCurrent, and LogBufferSize, could be written
// to by untrusted code. Here, we check that the pointers are within the
// allocated LoggerInfo space, and that LogBufferSize, which is used in
// multiple places to see if a new message will fit into the log buffer, is
// valid.
//

if (LoggerInfo->Signature != ADVANCED_LOGGER_SIGNATURE) {
return NULL;
}

// Ensure the start of the log is in the correct location.
if (LoggerInfo->LogBuffer != (PA_FROM_PTR (LoggerInfo + 1))) {
return NULL;
}

if (((mLoggerInfo) != NULL) && !ValidateInfoBlock ()) {
mLoggerInfo = NULL;
DEBUG ((DEBUG_ERROR, "%a: LoggerInfo marked invalid\n", __FUNCTION__));
// Make sure the size of the buffer does not overrun it's fixed size.
MaxAddress = LoggerInfo->LogBuffer + LoggerInfo->LogBufferSize;
if ((MaxAddress - PA_FROM_PTR (LoggerInfo)) >
(FixedPcdGet32 (PcdAdvancedLoggerPages) * EFI_PAGE_SIZE))
{
return NULL;
}

// Ensure the current pointer does not overrun.
if ((LoggerInfo->LogCurrent > MaxAddress) ||
(LoggerInfo->LogCurrent < LoggerInfo->LogBuffer))
{
return NULL;
}

return mLoggerInfo;
return LoggerInfo;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
BaseMemoryLib
DebugLib
SynchronizationLib
MmuLib

[Guids]
gAdvancedLoggerHobGuid
Expand Down

0 comments on commit 06c7662

Please sign in to comment.