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

AdvLoggerPkg: Exit from write call if signature mismatch #565

Conversation

VivianNK
Copy link
Contributor

@VivianNK VivianNK commented Aug 28, 2024

Description

Current behavior allows the PPI to be used if there is a signature or version mismatch. This fix returns from the function to exit gracefully and prevent memory corruption.

Create a bug to track this behavior elsewhere in Advanced Logger Library: #567

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

Local CI and unit test.

Integration Instructions

N/A

@github-actions github-actions bot added the impact:non-functional Does not have a functional impact label Aug 28, 2024
@VivianNK VivianNK added the type:bug Something isn't working label Aug 28, 2024
@VivianNK VivianNK changed the title AdvLoggerPkg: Return from write attempt if there is a mismatch AdvLoggerPkg: Exit from write call if signature mismatch Aug 28, 2024
@VivianNK VivianNK marked this pull request as ready for review August 28, 2024 18:22
@VivianNK VivianNK requested a review from apop5 August 28, 2024 18:22
@VivianNK VivianNK removed the impact:non-functional Does not have a functional impact label Aug 28, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.99%. Comparing base (b5632fc) to head (568f4ec).

Additional details and impacted files
@@                Coverage Diff                 @@
##           release/202405     #565      +/-   ##
==================================================
- Coverage           11.00%   10.99%   -0.01%     
==================================================
  Files                 145      145              
  Lines               22226    22229       +3     
  Branches             2370     2370              
==================================================
  Hits                 2445     2445              
- Misses              19748    19751       +3     
  Partials               33       33              
Flag Coverage Δ
AdvLoggerPkg 3.97% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@VivianNK VivianNK requested review from os-d and kuqin12 August 28, 2024 18:51
Copy link
Contributor

@os-d os-d left a comment

Choose a reason for hiding this comment

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

Do the other libs need this same behavior for the protocol? Do the unit tests need updating?

@VivianNK
Copy link
Contributor Author

Do the other libs need this same behavior for the protocol? Do the unit tests need updating?

@os-d Dxe library was fixed to not rely on asserts, and then its unit test was updated before being merged. Planning on doing the same with Pei Lib.
I can add an issue to check the rest of the libs?

of signature or version of the AdvancedLoggerPpi.
@VivianNK VivianNK force-pushed the personal/vnowkakeane/advloggerpeilib branch from 99c8690 to 568f4ec Compare August 28, 2024 21:11
@VivianNK VivianNK enabled auto-merge (squash) August 28, 2024 21:25
@VivianNK VivianNK merged commit bde9c52 into microsoft:release/202405 Aug 28, 2024
30 checks passed
ProjectMuBot referenced this pull request in microsoft/mu_tiano_platforms Sep 25, 2024
Introduces 9 new commits in [Common/MU](https://github.com/microsoft/mu_plus.git).

<details>
<summary>Commits</summary>
<ul>
<li><a href="https://github.com/microsoft/mu_plus/commit/bde9c52c420dc4fc075197ae5406cfb4ecc3f3c4">bde9c5</a> AdvLoggerPkg: Exit from write call if signature mismatch (<a href="https://github.com/microsoft/mu_plus/pull/565">#565</a>)</li>
<li><a href="https://github.com/microsoft/mu_plus/commit/f56280b7280ccdbb3877f53a3fd348ad7937f4eb">f56280</a> [CHERRY-PICK] Removed All References to PcdAdvancedLoggerPeiInRam (<a href="https://github.com/microsoft/mu_plus/pull/540">#540</a>)</li>
<li><a href="https://github.com/microsoft/mu_plus/commit/7c981ae9d492ba9403b65777a4d46174743bea98">7c981a</a> [CHERRY-PICK] Removed reference to PcdAdvancedHdwLoggerDisable in the README (<a href="https://github.com/microsoft/mu_plus/pull/541">#541</a>)</li>
<li><a href="https://github.com/microsoft/mu_plus/commit/0f790244e3273db5eae465ae49cbf07468047b52">0f7902</a> pip: bump edk2-pytool-extensions from 0.27.11 to 0.27.12 (<a href="https://github.com/microsoft/mu_plus/pull/571">#571</a>)</li>
<li><a href="https://github.com/microsoft/mu_plus/commit/bedfce0da5047d2f27ff302b869fb2ad6d23ec23">bedfce</a> Add MockDeviceBootManagerLib (<a href="https://github.com/microsoft/mu_plus/pull/572">#572</a>)</li>
<li><a href="https://github.com/microsoft/mu_plus/commit/93d7793bdd1c88cf8f6747d91d8d0abe2ab9c76e">93d779</a> AdvLoggerPkg: Add GoogleTest for AdvancedLoggerPeiLib (<a href="https://github.com/microsoft/mu_plus/pull/555">#555</a>)</li>
<li><a href="https://github.com/microsoft/mu_plus/commit/ad283339db8ac5610f3d1e8f98a2d3b5fd806c1c">ad2833</a> Add DxeCore AdvancedLogger GoogleTest (<a href="https://github.com/microsoft/mu_plus/pull/569">#569</a>)</li>
<li><a href="https://github.com/microsoft/mu_plus/commit/f14beb145ac49ddb9a570f6788fff5e7da4f4aad">f14beb</a> pip: bump regex from 2024.7.24 to 2024.9.11 (<a href="https://github.com/microsoft/mu_plus/pull/579">#579</a>)</li>
<li><a href="https://github.com/microsoft/mu_plus/commit/5fb0d0462731513e3a28d5c0be1932a765dfe03d">5fb0d0</a> pip: bump edk2-pytool-library from 0.21.10 to 0.21.11 (<a href="https://github.com/microsoft/mu_plus/pull/578">#578</a>)</li>
</ul>
</details>

Signed-off-by: Project Mu Bot <mubot@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants