From 83f065e0c6453488adc564493655e199a7eecef3 Mon Sep 17 00:00:00 2001 From: liqiqiii <132628835+liqiqiii@users.noreply.github.com> Date: Fri, 2 Aug 2024 00:47:03 -0700 Subject: [PATCH 1/3] [Cherry-Pick]Add debug level prefix for advanced logger memory message entries - Driver implementation (#525) Add debug level prefix for advanced logger memory message entries - Driver implementation This change added the existing metadata - debug level into the final advanced logger memory entries through AdvancedFileLogger. This is a followup PR for https://github.com/microsoft/mu_plus/commit/243bd187e4aa6ac68a3eb323d07554576d39ad47. After this PR checked in, we can easily track the DEBUG_ERRORs through advanced logger files on UEFI that included this driver. Added an extra space in the decodeuefilog.py to improve readability of the log. For the for loop in the code, ran with perf_trace enabled, and influence is very low and can be ignored. Updated the prefix of python script to match the design here. Use [ERROR] instead of [DEBUG_ERROR], which could save overall log file sizes and memory buffer usage. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... Tested with two platforms that uses AdvancedFileLogger and can see debug level prefixes in the logs in the EFI Partition and USB UefiLogs folder. [MM_CORE] [ERROR] Image - MmSupervisorCore.pdb N/A --- AdvLoggerPkg/AdvLoggerPkg.ci.yaml | 3 +- .../DecodeUefiLog/DecodeUefiLog.py | 54 ++++--- .../AdvancedLoggerAccessLib.c | 150 ++++++++++++++---- 3 files changed, 152 insertions(+), 55 deletions(-) diff --git a/AdvLoggerPkg/AdvLoggerPkg.ci.yaml b/AdvLoggerPkg/AdvLoggerPkg.ci.yaml index 644530283c..0ab8fac76b 100644 --- a/AdvLoggerPkg/AdvLoggerPkg.ci.yaml +++ b/AdvLoggerPkg/AdvLoggerPkg.ci.yaml @@ -88,7 +88,8 @@ LOGTELEMETRY, DEBUGAGENT, POSTMEM, - MMARM + MMARM, + BLKIO ] }, diff --git a/AdvLoggerPkg/Application/DecodeUefiLog/DecodeUefiLog.py b/AdvLoggerPkg/Application/DecodeUefiLog/DecodeUefiLog.py index 10e0492cdf..cdc5e7d6f3 100644 --- a/AdvLoggerPkg/Application/DecodeUefiLog/DecodeUefiLog.py +++ b/AdvLoggerPkg/Application/DecodeUefiLog/DecodeUefiLog.py @@ -260,9 +260,9 @@ class AdvLogParser (): ADVANCED_LOGGER_PHASE_SMM = 9 ADVANCED_LOGGER_PHASE_TFA = 10 ADVANCED_LOGGER_PHASE_CNT = 11 - PHASE_STRING_LIST = ["[UNSPECIFIED] ", "[SEC] ", "[PEI] ", "[PEI64] ", - "[DXE] ", "[RUNTIME] ", "[MM_CORE] ", "[MM] ", - "[SMM_CORE] ", "[SMM] ", "[TFA] "] + PHASE_STRING_LIST = ["[UNSPECIFIED]", "[SEC]", "[PEI]", "[PEI64]", + "[DXE]", "[RUNTIME]", "[MM_CORE]", "[MM]", + "[SMM_CORE]", "[SMM]", "[TFA]"] # Debug levels from MU_BASECORE\MdePkg\Include\Library\DebugLib.h # // @@ -293,27 +293,27 @@ class AdvLogParser (): # #define DEBUG_ERROR 0x80000000 // Error debug_levels_dict = { - 0x00000001: "[DEBUG_INIT]", - 0x00000002: "[DEBUG_WARN]", - 0x00000004: "[DEBUG_LOAD]", - 0x00000008: "[DEBUG_FS]", - 0x00000010: "[DEBUG_POOL]", - 0x00000020: "[DEBUG_PAGE]", - 0x00000040: "[DEBUG_INFO]", - 0x00000080: "[DEBUG_DISPATCH]", - 0x00000100: "[DEBUG_VARIABLE]", - 0x00000200: "[DEBUG_SMI]", - 0x00000400: "[DEBUG_BM]", - 0x00001000: "[DEBUG_BLKIO]", - 0x00004000: "[DEBUG_NET]", - 0x00010000: "[DEBUG_UNDI]", - 0x00020000: "[DEBUG_LOADFILE]", - 0x00080000: "[DEBUG_EVENT]", - 0x00100000: "[DEBUG_GCD]", - 0x00200000: "[DEBUG_CACHE]", - 0x00400000: "[DEBUG_VERBOSE]", - 0x00800000: "[DEBUG_MANAGEABILITY]", - 0x80000000: "[DEBUG_ERROR]" + 0x00000001: "[INIT]", + 0x00000002: "[WARN]", + 0x00000004: "[LOAD]", + 0x00000008: "[FS]", + 0x00000010: "[POOL]", + 0x00000020: "[PAGE]", + 0x00000040: "[INFO]", + 0x00000080: "[DISPATCH]", + 0x00000100: "[VARIABLE]", + 0x00000200: "[SMI]", + 0x00000400: "[BM]", + 0x00001000: "[BLKIO]", + 0x00004000: "[NET]", + 0x00010000: "[UNDI]", + 0x00020000: "[LOADFILE]", + 0x00080000: "[EVENT]", + 0x00100000: "[GCD]", + 0x00200000: "[CACHE]", + 0x00400000: "[VERBOSE]", + 0x00800000: "[MANAGEABILITY]", + 0x80000000: "[ERROR]" } # @@ -713,7 +713,8 @@ def _GetPhaseString(self, Phase): elif Phase <= self.ADVANCED_LOGGER_PHASE_UNSPECIFIED: PhaseString = "" else: - PhaseString = self.PHASE_STRING_LIST[Phase] + # Add an extra space for readability + PhaseString = self.PHASE_STRING_LIST[Phase] + ' ' return PhaseString # @@ -721,7 +722,8 @@ def _GetPhaseString(self, Phase): # def _GetDebugLevelString(self, DebugLevel): if DebugLevel in list(self.debug_levels_dict.keys()): - DebugLevelString = self.debug_levels_dict[DebugLevel] + # Add an extra space for readability + DebugLevelString = self.debug_levels_dict[DebugLevel] + ' ' else: DebugLevelString = "" return DebugLevelString diff --git a/AdvLoggerPkg/Library/AdvancedLoggerAccessLib/AdvancedLoggerAccessLib.c b/AdvLoggerPkg/Library/AdvancedLoggerAccessLib/AdvancedLoggerAccessLib.c index 893e32256b..b4845011d9 100644 --- a/AdvLoggerPkg/Library/AdvancedLoggerAccessLib/AdvancedLoggerAccessLib.c +++ b/AdvLoggerPkg/Library/AdvancedLoggerAccessLib/AdvancedLoggerAccessLib.c @@ -27,23 +27,55 @@ STATIC EFI_PHYSICAL_ADDRESS mLowAddress = 0; STATIC EFI_PHYSICAL_ADDRESS mHighAddress = 0; STATIC UINT16 mMaxMessageSize = ADVANCED_LOGGER_MAX_MESSAGE_SIZE; CONST CHAR8 *AdvMsgEntryPrefix[ADVANCED_LOGGER_PHASE_CNT] = { - "[UNSPECIFIED] ", - "[SEC] ", - "[PEI] ", - "[PEI64] ", - "[DXE] ", - "[RUNTIME] ", - "[MM_CORE] ", - "[MM] ", - "[SMM_CORE] ", - "[SMM] ", - "[TFA] ", + "[UNSPECIFIED]", + "[SEC]", + "[PEI]", + "[PEI64]", + "[DXE]", + "[RUNTIME]", + "[MM_CORE]", + "[MM]", + "[SMM_CORE]", + "[SMM]", + "[TFA]", }; -#define ADV_TIME_STAMP_FORMAT "%2.2d:%2.2d:%2.2d.%3.3d : " -#define ADV_TIME_STAMP_RESULT "hh:mm:ss:ttt : " -#define ADV_PHASE_ERR_FORMAT "[%04X] " -#define ADV_PHASE_MAX_SIZE 32 +// Define a structure to hold debug level information +typedef struct { + CONST CHAR8 *Name; + UINT32 Value; +} DEBUG_LEVEL; + +// Create an array of DebugLevel structures +DEBUG_LEVEL DebugLevels[] = { + { "[INIT]", 0x00000001 }, + { "[WARN]", 0x00000002 }, + { "[LOAD]", 0x00000004 }, + { "[FS]", 0x00000008 }, + { "[POOL]", 0x00000010 }, + { "[PAGE]", 0x00000020 }, + { "[INFO]", 0x00000040 }, + { "[DISPATCH]", 0x00000080 }, + { "[VARIABLE]", 0x00000100 }, + { "[SMI]", 0x00000200 }, + { "[BM]", 0x00000400 }, + { "[BLKIO]", 0x00001000 }, + { "[NET]", 0x00004000 }, + { "[UNDI]", 0x00010000 }, + { "[LOADFILE]", 0x00020000 }, + { "[EVENT]", 0x00080000 }, + { "[GCD]", 0x00100000 }, + { "[CACHE]", 0x00200000 }, + { "[VERBOSE]", 0x00400000 }, + { "[MANAGEABILITY]", 0x00800000 }, + { "[ERROR]", 0x80000000 } +}; + +#define ADV_LOG_TIME_STAMP_FORMAT "%2.2d:%2.2d:%2.2d.%3.3d : " +#define ADV_LOG_TIME_STAMP_RESULT "hh:mm:ss:ttt : " +#define ADV_LOG_PHASE_ERR_FORMAT "[%04X] " +#define ADV_LOG_PHASE_MAX_SIZE 32 +#define ADV_LOG_DEBUG_LEVEL_MAX_SIZE 32 /** @@ -88,21 +120,19 @@ FormatTimeStamp ( TimeStampLen = AsciiSPrint ( MessageBuffer, MessageBufferSize, - ADV_TIME_STAMP_FORMAT, + ADV_LOG_TIME_STAMP_FORMAT, Hours, Minutes, Seconds, Milliseconds ); - ASSERT (TimeStampLen == AsciiStrLen (ADV_TIME_STAMP_RESULT)); + ASSERT (TimeStampLen == AsciiStrLen (ADV_LOG_TIME_STAMP_RESULT)); return (UINT16)TimeStampLen; } /** - FormatPhasePrefix - Adds a phase indicator to the message being returned. If phase is recognized and specified, returns the phase prefix in from the AdvMsgEntryPrefix, otherwise raw phase value is returned. @@ -128,14 +158,64 @@ FormatPhasePrefix ( } else if (Phase < ADVANCED_LOGGER_PHASE_CNT) { // Normal message we recognize PhaseStringLen = AsciiSPrint (MessageBuffer, MessageBufferSize, AdvMsgEntryPrefix[Phase]); + // Verify string length and add an extra space for readability + if (PhaseStringLen < MessageBufferSize - 1) { + MessageBuffer[PhaseStringLen] = ' '; + MessageBuffer[PhaseStringLen + 1] = '\0'; + PhaseStringLen++; + } } else { // Unrecognized phase, just print the raw value - PhaseStringLen = AsciiSPrint (MessageBuffer, MessageBufferSize, ADV_PHASE_ERR_FORMAT, Phase); + PhaseStringLen = AsciiSPrint (MessageBuffer, MessageBufferSize, ADV_LOG_PHASE_ERR_FORMAT, Phase); } return (UINT16)PhaseStringLen; } +/** + Adds a debug level indicator to the message being returned. If debug level is recognized and specified, + returns the debug_level prefix in from the AdvMsgEntryPrefix, otherwise raw debug level value is returned. + + @param MessageBuffer + @param MessageBufferSize + @param DebugLevel + + @retval Number of characters printed +*/ +STATIC +UINT16 +FormatDebugLevelPrefix ( + IN CHAR8 *MessageBuffer, + IN UINTN MessageBufferSize, + IN UINT32 DebugLevel + ) +{ + if ((MessageBuffer == NULL) || (MessageBufferSize == 0)) { + return 0; + } + + UINTN DebugLevelStringLen; + UINTN Index; + + // Print the debug flags + for (Index = 0; Index < ARRAY_SIZE (DebugLevels); Index++) { + if ((DebugLevel & DebugLevels[Index].Value) == DebugLevels[Index].Value) { + DebugLevelStringLen = AsciiSPrint (MessageBuffer, MessageBufferSize, DebugLevels[Index].Name); + // Verify string length and add an extra space for readability + if (DebugLevelStringLen < MessageBufferSize - 1) { + MessageBuffer[DebugLevelStringLen] = ' '; + MessageBuffer[DebugLevelStringLen + 1] = '\0'; + DebugLevelStringLen++; + } + + return (UINT16)DebugLevelStringLen; + } + } + + // If this is not a known debug level, just don't print it out and return 0 + return 0; +} + /** Get Next Message Block. @@ -286,8 +366,11 @@ AdvancedLoggerAccessLibGetNextFormattedLine ( UINT16 TargetLen; UINT16 PhaseStringLen; UINT16 CurrPhaseStringLen; - CHAR8 TimeStampString[] = { ADV_TIME_STAMP_RESULT }; - CHAR8 PhaseString[ADV_PHASE_MAX_SIZE] = { 0 }; + UINT16 DebugLevelStringLen; + UINT16 CurrDebugLevelStringLen; + CHAR8 TimeStampString[] = { ADV_LOG_TIME_STAMP_RESULT }; + CHAR8 PhaseString[ADV_LOG_PHASE_MAX_SIZE] = { 0 }; + CHAR8 DebugLevelString[ADV_LOG_DEBUG_LEVEL_MAX_SIZE] = { 0 }; if (LineEntry == NULL) { return EFI_INVALID_PARAMETER; @@ -298,7 +381,7 @@ AdvancedLoggerAccessLibGetNextFormattedLine ( // reuse the previous LineBuffer // if (LineEntry->Message == NULL) { - LineBuffer = AllocatePool (mMaxMessageSize + sizeof (TimeStampString) + ADV_PHASE_MAX_SIZE); + LineBuffer = AllocatePool (mMaxMessageSize + sizeof (TimeStampString) + ADV_LOG_PHASE_MAX_SIZE); if (LineBuffer == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -314,15 +397,18 @@ AdvancedLoggerAccessLibGetNextFormattedLine ( // GetNextLine. // In case this is a restart of the same Message, initialize the time stamp and prefix. - PhaseStringLen = 0; + PhaseStringLen = 0; + DebugLevelStringLen = 0; if (LineEntry->BlockEntry.Message != NULL) { FormatTimeStamp (TimeStampString, sizeof (TimeStampString), LineEntry->BlockEntry.TimeStamp); CopyMem (LineBuffer, TimeStampString, sizeof (TimeStampString) - sizeof (CHAR8)); PhaseStringLen = FormatPhasePrefix (PhaseString, sizeof (PhaseString), LineEntry->BlockEntry.Phase); CopyMem (LineBuffer + sizeof (TimeStampString) - sizeof (CHAR8), PhaseString, PhaseStringLen); + DebugLevelStringLen = FormatDebugLevelPrefix (DebugLevelString, sizeof (DebugLevelString), LineEntry->BlockEntry.DebugLevel); + CopyMem (LineBuffer + sizeof (TimeStampString) + PhaseStringLen - sizeof (CHAR8), DebugLevelString, DebugLevelStringLen); } - TargetPtr = &LineBuffer[sizeof (TimeStampString) - sizeof (CHAR8) + PhaseStringLen]; + TargetPtr = &LineBuffer[sizeof (TimeStampString) - sizeof (CHAR8) + PhaseStringLen + DebugLevelStringLen]; TargetLen = 0; Status = EFI_SUCCESS; @@ -380,17 +466,25 @@ AdvancedLoggerAccessLibGetNextFormattedLine ( CopyMem (LineBuffer, TimeStampString, sizeof (TimeStampString) - sizeof (CHAR8)); CurrPhaseStringLen = FormatPhasePrefix (PhaseString, sizeof (PhaseString), LineEntry->BlockEntry.Phase); if (PhaseStringLen != CurrPhaseStringLen) { - // Adjust the TargetPtr to point to the end of the PhaseString + // Update the PhaseStringLen PhaseStringLen = CurrPhaseStringLen; - TargetPtr = &LineBuffer[sizeof (TimeStampString) - sizeof (CHAR8) + PhaseStringLen]; } CopyMem (LineBuffer + sizeof (TimeStampString) - sizeof (CHAR8), PhaseString, PhaseStringLen); + + CurrDebugLevelStringLen = FormatDebugLevelPrefix (DebugLevelString, sizeof (DebugLevelString), LineEntry->BlockEntry.DebugLevel); + if (DebugLevelStringLen != CurrDebugLevelStringLen) { + // Adjust the TargetPtr to point to the end of the DebugLevelString + DebugLevelStringLen = CurrDebugLevelStringLen; + TargetPtr = &LineBuffer[sizeof (TimeStampString) - sizeof (CHAR8) + PhaseStringLen + DebugLevelStringLen]; + } + + CopyMem (LineBuffer + sizeof (TimeStampString) - sizeof (CHAR8) + PhaseStringLen, DebugLevelString, DebugLevelStringLen); } } while (!EFI_ERROR (Status)); if (!EFI_ERROR (Status)) { - LineEntry->MessageLen = TargetLen + sizeof (TimeStampString) - sizeof (CHAR8) + PhaseStringLen; + LineEntry->MessageLen = TargetLen + sizeof (TimeStampString) - sizeof (CHAR8) + PhaseStringLen + DebugLevelStringLen; LineEntry->TimeStamp = LineEntry->BlockEntry.TimeStamp; LineEntry->DebugLevel = LineEntry->BlockEntry.DebugLevel; LineEntry->Phase = LineEntry->BlockEntry.Phase; From 18147d9abd75f264a0acc7d3ba43b2274d04955b Mon Sep 17 00:00:00 2001 From: Aaron <105021049+apop5@users.noreply.github.com> Date: Wed, 7 Aug 2024 16:43:59 -0700 Subject: [PATCH 2/3] [Cherry-Pick] Switch to use edk2-pytool-library UefiVariableSupportLib (#500) There are multiple copies of VariableSupportLib floating across repos, mostly only supporting Windows. Functionality has been consolidated into edk2-pytool-library version 0.21.7. Support for Linux has been added. Switch MfciPolicy.py, DecodeUefiLog.py and UefiVarAudit.py to use consolidated version from edk2-pytool-library. Removed local copies of VariableSupportLib.py - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... N/A --- .../DecodeUefiLog/DecodeUefiLog.py | 9 +- .../DecodeUefiLog/UefiVariablesSupportLib.py | 114 ------------ MfciPkg/Application/MfciPolicy/MfciPolicy.py | 20 +-- .../UefiVariablesSupportLib.py | 169 ------------------ .../UefiVarLockAudit/Windows/UefiVarAudit.py | 21 +-- .../Windows/UefiVariablesSupportLib.py | 92 ---------- 6 files changed, 21 insertions(+), 404 deletions(-) delete mode 100644 AdvLoggerPkg/Application/DecodeUefiLog/UefiVariablesSupportLib.py delete mode 100644 MfciPkg/Application/MfciPolicy/UefiVariableSupport/UefiVariablesSupportLib.py delete mode 100644 UefiTestingPkg/AuditTests/UefiVarLockAudit/Windows/UefiVariablesSupportLib.py diff --git a/AdvLoggerPkg/Application/DecodeUefiLog/DecodeUefiLog.py b/AdvLoggerPkg/Application/DecodeUefiLog/DecodeUefiLog.py index cdc5e7d6f3..19840779e8 100644 --- a/AdvLoggerPkg/Application/DecodeUefiLog/DecodeUefiLog.py +++ b/AdvLoggerPkg/Application/DecodeUefiLog/DecodeUefiLog.py @@ -12,8 +12,7 @@ import copy from win32com.shell import shell - -from UefiVariablesSupportLib import UefiVariable +from edk2toollib.os.uefivariablesupport import UefiVariable class AdvLogParser (): @@ -1003,7 +1002,7 @@ def ReadLogFromUefiInterface(): while rc == 0: VariableName = 'V'+str(Index) - (rc, var, errorstring) = UefiVar.GetUefiVar(VariableName, 'a021bf2b-34ed-4a98-859c-420ef94f3e94') + (rc, var) = UefiVar.GetUefiVar(VariableName, 'a021bf2b-34ed-4a98-859c-420ef94f3e94') if (rc == 0): Index += 1 InFile.write(var) @@ -1060,7 +1059,7 @@ def main(): CountOfLines = len(lines) print(f"{CountOfLines} lines written to {options.OutFilePath}") - except Exception as ex: + except Exception: print("Error processing log output.") traceback.print_exc() @@ -1072,7 +1071,7 @@ def main(): RawFile.close() print("RawFile complete") - except Exception as ex: + except Exception: print("Error processing raw file output.") traceback.print_exc() diff --git a/AdvLoggerPkg/Application/DecodeUefiLog/UefiVariablesSupportLib.py b/AdvLoggerPkg/Application/DecodeUefiLog/UefiVariablesSupportLib.py deleted file mode 100644 index ba63fa458c..0000000000 --- a/AdvLoggerPkg/Application/DecodeUefiLog/UefiVariablesSupportLib.py +++ /dev/null @@ -1,114 +0,0 @@ -# @file -# -# Python lib to support Reading and writing UEFI variables from windows -# -# Modified from original source to not produce error messages for Variable NOT FOUND -# -# Copyright (c), Microsoft Corporation -# SPDX-License-Identifier: BSD-2-Clause-Patent - -import os, sys -from ctypes import * -import logging -import pywintypes -import win32api, win32process, win32security, win32file -import winerror - -kernel32 = windll.kernel32 -EFI_VAR_MAX_BUFFER_SIZE = 1024*1024 - -class UefiVariable(object): - - def __init__(self): - # enable required SeSystemEnvironmentPrivilege privilege - privilege = win32security.LookupPrivilegeValue( None, 'SeSystemEnvironmentPrivilege' ) - token = win32security.OpenProcessToken( win32process.GetCurrentProcess(), win32security.TOKEN_READ|win32security.TOKEN_ADJUST_PRIVILEGES ) - win32security.AdjustTokenPrivileges( token, False, [(privilege, win32security.SE_PRIVILEGE_ENABLED)] ) - win32api.CloseHandle( token ) - - # import firmware variable API - try: - self._GetFirmwareEnvironmentVariable = kernel32.GetFirmwareEnvironmentVariableW - self._GetFirmwareEnvironmentVariable.restype = c_int - self._GetFirmwareEnvironmentVariable.argtypes = [c_wchar_p, c_wchar_p, c_void_p, c_int] - self._SetFirmwareEnvironmentVariable = kernel32.SetFirmwareEnvironmentVariableW - self._SetFirmwareEnvironmentVariable.restype = c_int - self._SetFirmwareEnvironmentVariable.argtypes = [c_wchar_p, c_wchar_p, c_void_p, c_int] - self._SetFirmwareEnvironmentVariableEx = kernel32.SetFirmwareEnvironmentVariableExW - self._SetFirmwareEnvironmentVariableEx.restype = c_int - self._SetFirmwareEnvironmentVariableEx.argtypes = [c_wchar_p, c_wchar_p, c_void_p, c_int, c_int] - except AttributeError as msg: - logging.warn( "G[S]etFirmwareEnvironmentVariableW function doesn't seem to exist" ) - pass - - # - # Helper function to create buffer for var read/write - # - def CreateBuffer(self, init, size=None): - """CreateBuffer(aString) -> character array - CreateBuffer(anInteger) -> character array - CreateBuffer(aString, anInteger) -> character array - """ - if isinstance(init, str): - if size is None: - size = len(init)+1 - buftype = c_char * size - buf = buftype() - buf.value = init - return buf - elif isinstance(init, int): - buftype = c_char * init - buf = buftype() - return buf - raise TypeError(init) - - # - #Function to get variable - # return a tuple of error code and variable data as string - # - def GetUefiVar(self, name, guid ): - err = 0 #success - efi_var = create_string_buffer( EFI_VAR_MAX_BUFFER_SIZE ) - if self._GetFirmwareEnvironmentVariable is not None: - logging.info("calling GetFirmwareEnvironmentVariable( name='%s', GUID='%s' ).." % (name, "{%s}" % guid) ) - length = self._GetFirmwareEnvironmentVariable( name, "{%s}" % guid, efi_var, EFI_VAR_MAX_BUFFER_SIZE ) - if (0 == length) or (efi_var is None): - err = kernel32.GetLastError() - # - # Don't produce an error message for NOT_FOUND - # - if err != 203: # 203 is NOT FOUND - logging.error( 'GetFirmwareEnvironmentVariable[Ex] failed (GetLastError = 0x%x)' % err) - logging.error(WinError()) - return (err, None, WinError(err)) - return (err, efi_var[:length], None) - # - #Function to set variable - # return a tuple of boolean status, errorcode, errorstring (None if not error) - # - def SetUefiVar(self, name, guid, var=None, attrs=None): - var_len = 0 - err = 0 - errorstring = None - if var is None: - var = bytes(0) - else: - var_len = len(var) - success = 0 # Fail - if(attrs == None): - if self._SetFirmwareEnvironmentVariable is not None: - logging.info("Calling SetFirmwareEnvironmentVariable (name='%s', Guid='%s')..." % (name, "{%s}" % guid, )) - success = self._SetFirmwareEnvironmentVariable(name, "{%s}" % guid, var, var_len) - else: - attrs = int(attrs) - if self._SetFirmwareEnvironmentVariableEx is not None: - logging.info(" calling SetFirmwareEnvironmentVariableEx( name='%s', GUID='%s', length=0x%X, attributes=0x%X ).." % (name, "{%s}" % guid, var_len, attrs) ) - success = self._SetFirmwareEnvironmentVariableEx( name, "{%s}" % guid, var, var_len, attrs ) - - if 0 == success: - err = kernel32.GetLastError() - logging.error('SetFirmwareEnvironmentVariable failed (GetLastError = 0x%x)' % err ) - logging.error(WinError()) - errorstring = WinError(err) - return (success,err, errorstring) - diff --git a/MfciPkg/Application/MfciPolicy/MfciPolicy.py b/MfciPkg/Application/MfciPolicy/MfciPolicy.py index 97de7979fb..ac6470eddc 100644 --- a/MfciPkg/Application/MfciPolicy/MfciPolicy.py +++ b/MfciPkg/Application/MfciPolicy/MfciPolicy.py @@ -11,7 +11,7 @@ import logging import argparse import ctypes -from UefiVariableSupport.UefiVariablesSupportLib import UefiVariable +from edk2toollib.os.uefivariablesupport import UefiVariable MFCI_VENDOR_GUID = "EBA1A9D2-BF4D-4736-B680-B36AFB4DD65B" CURRENT_POLICY_BLOB = "CurrentMfciPolicyBlob" @@ -77,7 +77,7 @@ def get_system_info(): UefiVar = UefiVariable() for Variable in MFCI_POLICY_INFO_VARIABLES: - (errorcode, data, errorstring) = UefiVar.GetUefiVar(Variable, MFCI_VENDOR_GUID) + (errorcode, data) = UefiVar.GetUefiVar(Variable, MFCI_VENDOR_GUID) if errorcode != 0: logging.critical(f"Failed to get policy variable {Variable}") else: @@ -91,7 +91,7 @@ def get_system_info(): def get_current_mfci_policy(): UefiVar = UefiVariable() - (errorcode, data, errorstring) = UefiVar.GetUefiVar(CURRENT_MFCI_POLICY, MFCI_VENDOR_GUID) + (errorcode, data) = UefiVar.GetUefiVar(CURRENT_MFCI_POLICY, MFCI_VENDOR_GUID) if errorcode == 0: result = hex(int.from_bytes(data, byteorder="little", signed=False)) logging.info(f" Current MFCI Policy is {result}") @@ -104,17 +104,17 @@ def get_current_mfci_policy(): def delete_current_mfci_policy(): UefiVar = UefiVariable() - (errorcode, data, errorstring) = UefiVar.SetUefiVar(CURRENT_POLICY_BLOB, MFCI_VENDOR_GUID, None, 3) + (errorcode, data) = UefiVar.SetUefiVar(CURRENT_POLICY_BLOB, MFCI_VENDOR_GUID, None, 3) if errorcode == 0: - logging.info(f"Failed to Delete {CURRENT_POLICY_BLOB}\n {errorcode}{errorstring}") + logging.info(f"Failed to Delete {CURRENT_POLICY_BLOB}\n {errorcode}") print(f"Failed to delete {CURRENT_POLICY_BLOB}") else: logging.info(f"{CURRENT_POLICY_BLOB} was deleted") print(f"{CURRENT_POLICY_BLOB} was deleted") - (errorcode, data, errorstring) = UefiVar.SetUefiVar(NEXT_MFCI_POLICY_BLOB, MFCI_VENDOR_GUID, None, 3) + (errorcode, data) = UefiVar.SetUefiVar(NEXT_MFCI_POLICY_BLOB, MFCI_VENDOR_GUID, None, 3) if errorcode == 0: - logging.info(f"Failed to Delete {NEXT_MFCI_POLICY_BLOB}\n {errorcode}{errorstring}") + logging.info(f"Failed to Delete {NEXT_MFCI_POLICY_BLOB}\n {errorcode}") print(f"Failed to delete {NEXT_MFCI_POLICY_BLOB}") else: logging.info(f"{NEXT_MFCI_POLICY_BLOB} was deleted") @@ -128,9 +128,9 @@ def set_next_mfci_policy(policy): var = file.read() UefiVar = UefiVariable() - (errorcode, data, errorstring) = UefiVar.SetUefiVar(NEXT_MFCI_POLICY_BLOB, MFCI_VENDOR_GUID, var, 3) + (errorcode, data) = UefiVar.SetUefiVar(NEXT_MFCI_POLICY_BLOB, MFCI_VENDOR_GUID, var, 3) if errorcode == 0: - logging.info("Next Policy failed: {errorstring}") + logging.info("Next Policy failed: {errorcode}") else: logging.info("Next Policy was set") print(f"{NEXT_MFCI_POLICY_BLOB} was set") @@ -139,7 +139,7 @@ def set_next_mfci_policy(policy): def get_next_mfci_policy(): UefiVar = UefiVariable() - (errorcode, data, errorstring) = UefiVar.GetUefiVar(NEXT_MFCI_POLICY_BLOB, MFCI_VENDOR_GUID) + (errorcode, data) = UefiVar.GetUefiVar(NEXT_MFCI_POLICY_BLOB, MFCI_VENDOR_GUID) if errorcode != 0: logging.info("No Next Mfci Policy Set") print(f"No variable {NEXT_MFCI_POLICY_BLOB} found") diff --git a/MfciPkg/Application/MfciPolicy/UefiVariableSupport/UefiVariablesSupportLib.py b/MfciPkg/Application/MfciPolicy/UefiVariableSupport/UefiVariablesSupportLib.py deleted file mode 100644 index 0197efb844..0000000000 --- a/MfciPkg/Application/MfciPolicy/UefiVariableSupport/UefiVariablesSupportLib.py +++ /dev/null @@ -1,169 +0,0 @@ -# @file -# -# Python lib to support Reading and writing UEFI variables from windows -# -# Copyright (c) Microsoft Corporation. -# SPDX-License-Identifier: BSD-2-Clause-Patent - -from ctypes import ( - windll, - c_wchar_p, - c_void_p, - c_int, - c_char, - create_string_buffer, - WinError, -) -import logging -from win32 import win32api -from win32 import win32process -from win32 import win32security - -kernel32 = windll.kernel32 -EFI_VAR_MAX_BUFFER_SIZE = 1024 * 1024 - - -class UefiVariable(object): - def __init__(self): - # enable required SeSystemEnvironmentPrivilege privilege - privilege = win32security.LookupPrivilegeValue( - None, "SeSystemEnvironmentPrivilege" - ) - token = win32security.OpenProcessToken( - win32process.GetCurrentProcess(), - win32security.TOKEN_READ | win32security.TOKEN_ADJUST_PRIVILEGES, - ) - win32security.AdjustTokenPrivileges( - token, False, [(privilege, win32security.SE_PRIVILEGE_ENABLED)] - ) - win32api.CloseHandle(token) - - # import firmware variable API - try: - self._GetFirmwareEnvironmentVariable = ( - kernel32.GetFirmwareEnvironmentVariableW - ) - self._GetFirmwareEnvironmentVariable.restype = c_int - self._GetFirmwareEnvironmentVariable.argtypes = [ - c_wchar_p, - c_wchar_p, - c_void_p, - c_int, - ] - self._SetFirmwareEnvironmentVariable = ( - kernel32.SetFirmwareEnvironmentVariableW - ) - self._SetFirmwareEnvironmentVariable.restype = c_int - self._SetFirmwareEnvironmentVariable.argtypes = [ - c_wchar_p, - c_wchar_p, - c_void_p, - c_int, - ] - self._SetFirmwareEnvironmentVariableEx = ( - kernel32.SetFirmwareEnvironmentVariableExW - ) - self._SetFirmwareEnvironmentVariableEx.restype = c_int - self._SetFirmwareEnvironmentVariableEx.argtypes = [ - c_wchar_p, - c_wchar_p, - c_void_p, - c_int, - c_int, - ] - except Exception: - logging.warn( - "G[S]etFirmwareEnvironmentVariableW function doesn't seem to exist" - ) - pass - - # - # Helper function to create buffer for var read/write - # - def CreateBuffer(self, init, size=None): - """CreateBuffer(aString) -> character array - CreateBuffer(anInteger) -> character array - CreateBuffer(aString, anInteger) -> character array - """ - if isinstance(init, str): - if size is None: - size = len(init) + 1 - buftype = c_char * size - buf = buftype() - buf.value = init - return buf - elif isinstance(init, int): - buftype = c_char * init - buf = buftype() - return buf - raise TypeError(init) - - # - # Function to get variable - # return a tuple of error code and variable data as string - # - def GetUefiVar(self, name, guid): - # success - err = 0 - efi_var = create_string_buffer(EFI_VAR_MAX_BUFFER_SIZE) - if self._GetFirmwareEnvironmentVariable is not None: - logging.info( - "calling GetFirmwareEnvironmentVariable( name='%s', GUID='%s' ).." - % (name, "{%s}" % guid) - ) - length = self._GetFirmwareEnvironmentVariable( - name, "{%s}" % guid, efi_var, EFI_VAR_MAX_BUFFER_SIZE - ) - if (0 == length) or (efi_var is None): - err = kernel32.GetLastError() - logging.error( - "GetFirmwareEnvironmentVariable[Ex] failed (GetLastError = 0x%x)" % err - ) - logging.error(WinError()) - return (err, None, WinError(err)) - return (err, efi_var[:length], None) - - # - # Function to set variable - # return a tuple of boolean status, error_code, error_string (None if not error) - # - def SetUefiVar(self, name, guid, var=None, attrs=None): - var_len = 0 - err = 0 - error_string = None - if var is None: - var = bytes(0) - else: - var_len = len(var) - success = 0 # Fail - if attrs is None: - if self._SetFirmwareEnvironmentVariable is not None: - logging.info( - "Calling SetFirmwareEnvironmentVariable (name='%s', Guid='%s')..." - % ( - name, - "{%s}" % guid, - ) - ) - success = self._SetFirmwareEnvironmentVariable( - name, "{%s}" % guid, var, var_len - ) - else: - attrs = int(attrs) - if self._SetFirmwareEnvironmentVariableEx is not None: - logging.info( - "Calling SetFirmwareEnvironmentVariableEx( name='%s', GUID='%s', length=0x%X, attributes=0x%X ).." - % (name, "{%s}" % guid, var_len, attrs) - ) - success = self._SetFirmwareEnvironmentVariableEx( - name, "{%s}" % guid, var, var_len, attrs - ) - - if 0 == success: - err = kernel32.GetLastError() - logging.error( - "SetFirmwareEnvironmentVariable failed (GetLastError = 0x%x)" % err - ) - logging.error(WinError()) - error_string = WinError(err) - return (success, err, error_string) diff --git a/UefiTestingPkg/AuditTests/UefiVarLockAudit/Windows/UefiVarAudit.py b/UefiTestingPkg/AuditTests/UefiVarLockAudit/Windows/UefiVarAudit.py index 2507142c93..fb38bd776e 100644 --- a/UefiTestingPkg/AuditTests/UefiVarLockAudit/Windows/UefiVarAudit.py +++ b/UefiTestingPkg/AuditTests/UefiVarLockAudit/Windows/UefiVarAudit.py @@ -8,17 +8,14 @@ ## -import os, sys +import os +import sys import argparse import logging import datetime -import struct -import hashlib -import shutil -import time import xml.etree.ElementTree as ET from xml.etree.ElementTree import Element -from UefiVariablesSupportLib import UefiVariable +from edk2toollib.os.uefivariablesupport import UefiVariable # #main script function @@ -72,13 +69,13 @@ def main(): for var in XmlRoot.findall("Variable"): name = var.get("Name") guid = var.get("Guid") - (ReadStatus, Data, ReadErrorString) = Uefi.GetUefiVar(name, guid) - (WriteSuccess, ErrorCode, WriteErrorString)= Uefi.SetUefiVar(name, guid) + (ReadStatus, Data) = Uefi.GetUefiVar(name, guid) + (WriteSuccess, ErrorCode)= Uefi.SetUefiVar(name, guid) if(WriteSuccess != 0): logging.info("Must Restore Var %s:%s" % (name, guid)) - (RestoreSuccess, RestoreEC, RestoreErrorString) = Uefi.SetUefiVar(name, guid, Data) + (RestoreSuccess, RestoreEC) = Uefi.SetUefiVar(name, guid, Data) if (RestoreSuccess == 0): - logging.critical("Restoring failed for Var %s:%s 0x%X ErrorCode: 0x%X %s" % (name, guid, RestoreSuccess, RestoreEC, RestoreErrorString)) + logging.critical("Restoring failed for Var %s:%s 0x%X ErrorCode: 0x%X %s" % (name, guid, RestoreSuccess, RestoreEC)) #append # #0x0 Success @@ -87,11 +84,7 @@ def main(): rs = Element("ReadStatus") ws = Element("WriteStatus") rs.text = "0x%lX" % (ReadStatus) - if(ReadErrorString is not None): - rs.text = rs.text + " %s" % ReadErrorString ws.text = "0x%lX" % ErrorCode - if(WriteErrorString is not None): - ws.text = ws.text + " %s" % WriteErrorString ele.append(rs) ele.append(ws) var.append(ele) diff --git a/UefiTestingPkg/AuditTests/UefiVarLockAudit/Windows/UefiVariablesSupportLib.py b/UefiTestingPkg/AuditTests/UefiVarLockAudit/Windows/UefiVariablesSupportLib.py deleted file mode 100644 index 9398cbc195..0000000000 --- a/UefiTestingPkg/AuditTests/UefiVarLockAudit/Windows/UefiVariablesSupportLib.py +++ /dev/null @@ -1,92 +0,0 @@ -## -## Python lib to support Reading and writing UEFI variables from windows -## -# -# -# Copyright (C) Microsoft Corporation. All rights reserved. -# SPDX-License-Identifier: BSD-2-Clause-Patent -## - -import os, sys -from ctypes import * -import logging -import pywintypes -import win32api, win32process, win32security, win32file -import winerror - -kernel32 = windll.kernel32 -EFI_VAR_MAX_BUFFER_SIZE = 1024*1024 - -class UefiVariable(object): - - def __init__(self): - # enable required SeSystemEnvironmentPrivilege privilege - privilege = win32security.LookupPrivilegeValue( None, 'SeSystemEnvironmentPrivilege' ) - token = win32security.OpenProcessToken( win32process.GetCurrentProcess(), win32security.TOKEN_READ|win32security.TOKEN_ADJUST_PRIVILEGES ) - win32security.AdjustTokenPrivileges( token, False, [(privilege, win32security.SE_PRIVILEGE_ENABLED)] ) - win32api.CloseHandle( token ) - - # get windows firmware variable API - try: - self._GetFirmwareEnvironmentVariable = kernel32.GetFirmwareEnvironmentVariableW - self._GetFirmwareEnvironmentVariable.restype = c_int - self._GetFirmwareEnvironmentVariable.argtypes = [c_wchar_p, c_wchar_p, c_void_p, c_int] - self._SetFirmwareEnvironmentVariable = kernel32.SetFirmwareEnvironmentVariableW - self._SetFirmwareEnvironmentVariable.restype = c_int - self._SetFirmwareEnvironmentVariable.argtypes = [c_wchar_p, c_wchar_p, c_void_p, c_int] - self._SetFirmwareEnvironmentVariableEx = kernel32.SetFirmwareEnvironmentVariableExW - self._SetFirmwareEnvironmentVariableEx.restype = c_int - self._SetFirmwareEnvironmentVariableEx.argtypes = [c_wchar_p, c_wchar_p, c_void_p, c_int, c_int] - except AttributeError: - logging.warn( "Some get/set functions don't't seem to exist" ) - - # - #Function to get variable - # return a tuple of error code and variable data as string - # - def GetUefiVar(self, name, guid ): - err = 0 #success - efi_var = create_string_buffer( EFI_VAR_MAX_BUFFER_SIZE ) - if self._GetFirmwareEnvironmentVariable is not None: - logging.info("calling GetFirmwareEnvironmentVariable( name='%s', GUID='%s' ).." % (name, "{%s}" % guid) ) - length = self._GetFirmwareEnvironmentVariable( name, "{%s}" % guid, efi_var, EFI_VAR_MAX_BUFFER_SIZE ) - if (0 == length) or (efi_var is None): - err = kernel32.GetLastError() - logging.error( 'GetFirmwareEnvironmentVariable[Ex] failed (GetLastError = 0x%x)' % err) - logging.error(WinError()) - return (err, None, WinError(err)) - return (err, efi_var[:length], None) - # - #Function to set variable - # return a tuple of boolean status, errorcode, errorstring (None if not error) - # - def SetUefiVar(self, name, guid, var=None, attrs=None): - var_len = 0 - err = 0 - errorstring = None - if var is None: - var = bytes(0) - else: - var_len = len(var) - if(attrs == None): - if self._SetFirmwareEnvironmentVariable is not None: - logging.info("Calling SetFirmwareEnvironmentVariable (name='%s', Guid='%s')..." % (name, "{%s}" % guid, )) - success = self._SetFirmwareEnvironmentVariable(name, "{%s}" % guid, var, var_len) - else: - if self._SetFirmwareEnvironmentVariableEx is not None: - logging.info(" calling SetFirmwareEnvironmentVariableEx( name='%s', GUID='%s', length=0x%X, attributes=0x%X ).." % (name, "{%s}" % guid, var_len, attrs) ) - success = self._SetFirmwareEnvironmentVariableEx( name, "{%s}" % guid, var, var_len, attrs ) - - if 0 == success: - err = kernel32.GetLastError() - logging.error('SetFirmwareEnvironmentVariable failed (GetLastError = 0x%x)' % err ) - logging.error(WinError()) - errorstring = WinError(err) - return (success,err, errorstring) - - - -#Test code -#UefiVar = UefiVariable() -#(errorcode, data, errorstring) = UefiVar.GetUefiVar('PK', '8BE4DF61-93CA-11D2-AA0D-00E098032B8C') -#(status, errorcode, errorstring) = UefiVar.SetUefiVar('PK','8BE4DF61-93CA-11D2-AA0D-00E098032B8C',None, None) \ No newline at end of file From 1697b02daadecccbec75006d3dd040d9f617546a Mon Sep 17 00:00:00 2001 From: julorenz117 <60625130+julorenz117@users.noreply.github.com> Date: Wed, 21 Aug 2024 19:09:53 -0700 Subject: [PATCH 3/3] [Cherry-Pick] MsGraphicsPkg: Correct positioning of trash can icon in Load Option's list box (#552) Fixes #554 - Adjusted CellTrashcanBounds.Left to be CellBounds->Right - TrashcanHitAreaWidth to ensure the trash can icon is displayed to the right of the list box. - Updated width parameter in SWM_RECT_INIT2 to use TrashcanHitAreaWidth instead of CheckBoxHitAreaWidth for correct dimensions. This resolves the issue of the trash can icon overlapping with the ListBox's deletable item's checkbox thus ensuring its related operations work correctly: activating/deactivating the Load Option or deleting it. Verified that a Load Option allowed to be deleted, such as 'Windows Boot Manager', can now be deleted by pressing the trash icon in its proper position or activated via its check-box. N/A Co-authored-by: Michael Kubacki --- MsGraphicsPkg/Library/SimpleUIToolKit/ListBox.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MsGraphicsPkg/Library/SimpleUIToolKit/ListBox.c b/MsGraphicsPkg/Library/SimpleUIToolKit/ListBox.c index fcc42d1a73..2468a7f142 100644 --- a/MsGraphicsPkg/Library/SimpleUIToolKit/ListBox.c +++ b/MsGraphicsPkg/Library/SimpleUIToolKit/ListBox.c @@ -982,9 +982,9 @@ Ctor ( SWM_RECT_INIT2 ( this->m_pCells[Index].CellTrashcanBounds, - CellBounds->Left, + CellBounds->Right - TrashcanHitAreaWidth, CellBounds->Top, - CheckBoxHitAreaWidth, + TrashcanHitAreaWidth, SWM_RECT_HEIGHT (*CellBounds) ); }