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/Test/DevicePathLib: Remove FreePool(NULL) #6421

Conversation

mdkinney
Copy link
Member

@mdkinney mdkinney commented Nov 8, 2024

Description

Unit test checks if AppendDevicePathInstance() returns NULL. In those cases, AppendDevicePathInstance() does not allocate a device path, so the call to FreePool() must not be performed.

Detected with use of address sanitizer in host based unit tests in draft PR #6408

  • 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

Run host based unit tests with address sanitizer enabled and the issue is resolved with this change

Integration Instructions

Unit test checks if AppendDevicePathInstance() returns NULL.
In those cases, AppendDevicePathInstance() does not allocate
a device path, so the call to FreePool() must not be performed.

Signed-off-by: Michael D Kinney <[email protected]>
@github-actions github-actions bot added the impact:security This change has a direct security impact such as changing a crypto algorithm. label Nov 8, 2024
@mdkinney mdkinney added the push Auto push patch series in PR if all checks pass label Nov 8, 2024
@mergify mergify bot merged commit fadf4f3 into tianocore:master Nov 8, 2024
126 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact:security This change has a direct security impact such as changing a crypto algorithm. push Auto push patch series in PR if all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants