From d99045f39220e2ad075ae5b93b53bbabd5923cb4 Mon Sep 17 00:00:00 2001 From: Oliver Smith-Denny Date: Thu, 26 Sep 2024 14:13:42 -0700 Subject: MdePkg: DebugLib: Check Signature in CR in Release Builds The CR macro is used to access an enclosing structure from a pointer within the structure. In DEBUG builds (i.e. when MDEPKG_NDEBUG is not set and debug asserts are enabled), this macro does signature validation checking to ensure that the structure that has been found is the correct structure, based on a signature passed in by the caller. However, if MDEPKG_NDEBUG is set or debug asserts are disabled, no signature validation is performed, meaning that CR may return an invalid structure that the caller believes is valid and has had signature validation on, causing undefined behavior (memory corruption). We should where at all possible have defined behavior, particularly in RELEASE builds, which are what typical platforms will ship to consumers. This patch updates CR to do the signature validation in all scenarios to provide defined behavior from the macro. In the event of a signature failure, CR will either 1) assert if !MDEPKG_NDEBUG and debug asserts are enabled (existing behavior) or 2) return NULL to indicate to the caller that signature validation failed. There exist consumers today who already, erroneously, rely on this behavior. Another macro, BASE_CR, exists for callers who do not wish to perform signature validation. Any code that wishes to avoid the signature validation should move to this macro. Signed-off-by: Oliver Smith-Denny --- MdePkg/Include/Library/DebugLib.h | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h index b0742969a8..033c4e1a66 100644 --- a/MdePkg/Include/Library/DebugLib.h +++ b/MdePkg/Include/Library/DebugLib.h @@ -593,8 +593,12 @@ UnitTestDebugAssert ( If MDEPKG_NDEBUG is defined or the DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of PcdDebugProperyMask is clear, then this macro computes the offset, in bytes, of the field specified by Field from the beginning of the data structure specified - by TYPE. This offset is subtracted from Record, and is used to return a pointer - to a data structure of the type specified by TYPE. + by TYPE. This offset is subtracted from Record, and is used to compute a pointer + to a data structure of the type specified by TYPE. The Signature field of the + data structure specified by TYPE is compared to TestSignature. If the signatures + match, then a pointer to the pointer to a data structure of the type specified by + TYPE is returned. If the signatures do not match, then NULL is returned to + signify that the passed in data structure is invalid. If MDEPKG_NDEBUG is not defined and the DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of PcdDebugProperyMask is set, then this macro computes the offset, in bytes, @@ -628,9 +632,13 @@ UnitTestDebugAssert ( #define CR(Record, TYPE, Field, TestSignature) \ (DebugAssertEnabled () && (BASE_CR (Record, TYPE, Field)->Signature != TestSignature)) ? \ (TYPE *) (_ASSERT (CR has Bad Signature), Record) : \ + (BASE_CR (Record, TYPE, Field)->Signature != TestSignature) ? \ + NULL : \ BASE_CR (Record, TYPE, Field) #else #define CR(Record, TYPE, Field, TestSignature) \ + (BASE_CR (Record, TYPE, Field)->Signature != TestSignature) ? \ + NULL : \ BASE_CR (Record, TYPE, Field) #endif -- cgit v1.2.3