Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MdePkg/DebugLib: Add null pointer check to CR macro #10591

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

r1chard-lyu
Copy link

  • Added a null pointer check in the CR macro.
  • If Record is NULL, an assertion is triggered.
  • This change improves the CR by preventing dereferencing of null pointers.

REF: #10510

Description

<Include a description of the change and why this change was made.>

<For each item, place an "x" in between [ and ] if true. Example: [x] (you can also check items in GitHub UI)>

<Create the PR as a Draft PR if it is only created to run CI checks.>

<Delete lines in <> tags before creating the PR.>

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

<Describe the test(s) that were run to verify the changes.>

Integration Instructions

<Describe how these changes should be integrated. Use N/A if nothing is required.>

@@ -657,11 +657,12 @@ UnitTestDebugAssert (
**/
#if !defined (MDEPKG_NDEBUG)
#define CR(Record, TYPE, Field, TestSignature) \
((Record == NULL) ? (_ASSERT (CR has Null Pointer), NULL) : \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Record may point to zero address.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your comment. The line was added to check if Record is pointing to zero. If Record is pointing to zero, the assertion _ASSERT (CR has Null Pointer) is triggered, and Record (which is NULL) is returned. I have updated the code accordingly. Please review the updated code and provide any suggestions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgao4 I have updated the code. Could you please review the latest changes?

@r1chard-lyu r1chard-lyu force-pushed the cr-null-pointer-check branch from 6c1fef8 to d2a9618 Compare January 7, 2025 02:04
- Added a null pointer check in the CR macro.
- If Record is NULL, an assertion is triggered.
- This change improves the CR by preventing dereferencing of null pointers.

REF: tianocore#10510

Signed-off-by: Richard Lyu <[email protected]>
@r1chard-lyu r1chard-lyu force-pushed the cr-null-pointer-check branch from d2a9618 to 7b6f2d7 Compare January 7, 2025 02:06
@r1chard-lyu r1chard-lyu requested a review from lgao4 January 7, 2025 05:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants