diff --git a/AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.c b/AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.c index 0cf4ad5217..ae41fdb486 100644 --- a/AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.c +++ b/AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.c @@ -15,63 +15,15 @@ #include #include #include -#include #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. @@ -87,7 +39,7 @@ 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 + NOTE: A debug statement here will cause recursion. Ensure that the recursion will be a straight path just to return the existing mLoggerInfo. @param - None @@ -102,48 +54,59 @@ AdvancedLoggerGetLoggerInfo ( VOID ) { - EFI_STATUS Status; - EFI_PHYSICAL_ADDRESS Address; + ADVANCED_LOGGER_INFO *LoggerInfo; + EFI_PHYSICAL_ADDRESS MaxAddress; + UINT32 HwPrintLevel; 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; } - if (((mLoggerInfo) != NULL) && !ValidateInfoBlock ()) { - mLoggerInfo = NULL; - DEBUG ((DEBUG_ERROR, "%a: LoggerInfo marked invalid\n", __FUNCTION__)); + // Initialize HdwPrintLevel if needed. + HwPrintLevel = FixedPcdGet32 (PcdAdvancedLoggerHdwPortDebugPrintErrorLevel); + if (LoggerInfo->HwPrintLevel != HwPrintLevel) { + LoggerInfo->HwPrintLevel = HwPrintLevel; + } + + // + // 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; + } + + // Make sure the size of the buffer does not overrun it's fixed size. + if ((LoggerInfo->LogBuffer + LoggerInfo->LogBufferSize) > + (FixedPcdGet32 (PcdAdvancedLoggerPages) * EFI_PAGE_SIZE)) + { + return FALSE; + } + + // Ensure the current pointer does not overrun. + MaxAddress = LoggerInfo->LogBuffer + LoggerInfo->LogBufferSize; + if ((LoggerInfo->LogCurrent > MaxAddress) || + (LoggerInfo->LogCurrent < LoggerInfo->LogBuffer)) + { + return NULL; } - return mLoggerInfo; + return LoggerInfo; } /** diff --git a/AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.inf b/AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.inf index 7bf5cdb9dd..c35ef5274d 100644 --- a/AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.inf +++ b/AdvLoggerPkg/Library/AdvancedLoggerLib/MmCoreArm/AdvancedLoggerLib.inf @@ -36,7 +36,6 @@ BaseMemoryLib DebugLib SynchronizationLib - MmuLib [Guids] gAdvancedLoggerHobGuid