From 23043756b9d76f0c6fe7bdd8231727c3690d1e16 Mon Sep 17 00:00:00 2001 From: Aaron <105021049+apop5@users.noreply.github.com> Date: Wed, 23 Aug 2023 13:47:24 -0700 Subject: [PATCH] AdvLoggerPkg: Switch to using AsciiStrnLenS (#293) ## Description A compounds assert was found in a platform. An assert was triggered, and when attempting to generate the assert messages, print lib triggered an assert as well. The system was found to have reached the PcdMaximumAsciiStringLength referenced in AsciiStrLen. To combat this, the AsciiStrLen calls in AdvancedLogger are being switched to use the safe version, with the associated max lengths being the stack buffers that are used in the functions. This change allowed assert messages to be displayed. Fixes #292 - [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, ... ## How This Was Tested Tested on a platform that was triggering asserts. Tested in CI ## Integration Instructions N/A - Changes should be --- AdvLoggerPkg/Library/AssertLib/AssertLib.c | 2 +- AdvLoggerPkg/Library/AssertTelemetryLib/AssertLib.c | 6 +++--- AdvLoggerPkg/Library/BaseDebugLibAdvancedLogger/DebugLib.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/AdvLoggerPkg/Library/AssertLib/AssertLib.c b/AdvLoggerPkg/Library/AssertLib/AssertLib.c index 8304862958..640e6ec9ae 100644 --- a/AdvLoggerPkg/Library/AssertLib/AssertLib.c +++ b/AdvLoggerPkg/Library/AssertLib/AssertLib.c @@ -63,7 +63,7 @@ DebugAssert ( // // Send the print string to the Logging device device // - AdvancedLoggerWrite (DEBUG_ERROR, Buffer, AsciiStrLen (Buffer)); + AdvancedLoggerWrite (DEBUG_ERROR, Buffer, AsciiStrnLenS (Buffer, sizeof (Buffer))); if ((PcdGet8 (PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { CpuBreakpoint (); diff --git a/AdvLoggerPkg/Library/AssertTelemetryLib/AssertLib.c b/AdvLoggerPkg/Library/AssertTelemetryLib/AssertLib.c index 0517cf2600..e6f262c22b 100644 --- a/AdvLoggerPkg/Library/AssertTelemetryLib/AssertLib.c +++ b/AdvLoggerPkg/Library/AssertTelemetryLib/AssertLib.c @@ -71,9 +71,9 @@ DebugAssert ( // Check to make sure the file name is valid and at least two characters long (which would be just the extension) if ((FileName != NULL) && - (AsciiStrLen (FileName) >= 2)) + (AsciiStrnLenS (FileName, sizeof (Buffer)) >= 2)) { - FileNameLength = AsciiStrLen (FileName) - (2 * sizeof (CHAR8)); // We don't care about the extension + FileNameLength = AsciiStrnLenS (FileName, sizeof (Buffer)) - (2 * sizeof (CHAR8)); // We don't care about the extension } // END LOGTELEMETRY @@ -86,7 +86,7 @@ DebugAssert ( // // Send the print string to the Logging device device // - AdvancedLoggerWrite (DEBUG_ERROR, Buffer, AsciiStrLen (Buffer)); + AdvancedLoggerWrite (DEBUG_ERROR, Buffer, AsciiStrnLenS (Buffer, sizeof (Buffer))); // // Generate a Breakpoint, DeadLoop, or Telemetry based on PCD settings diff --git a/AdvLoggerPkg/Library/BaseDebugLibAdvancedLogger/DebugLib.c b/AdvLoggerPkg/Library/BaseDebugLibAdvancedLogger/DebugLib.c index 366f008aad..1d3287c445 100644 --- a/AdvLoggerPkg/Library/BaseDebugLibAdvancedLogger/DebugLib.c +++ b/AdvLoggerPkg/Library/BaseDebugLibAdvancedLogger/DebugLib.c @@ -171,7 +171,7 @@ DebugPrintMarker ( // // Send the print string to the Advanced Logger // - AdvancedLoggerWrite (ErrorLevel, Buffer, AsciiStrLen (Buffer)); + AdvancedLoggerWrite (ErrorLevel, Buffer, AsciiStrnLenS (Buffer, sizeof (Buffer))); } /**