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

[Feature]: Switch AdvancedLogger to use Safe Strings. #292

Closed
apop5 opened this issue Aug 23, 2023 · 0 comments · Fixed by #293
Closed

[Feature]: Switch AdvancedLogger to use Safe Strings. #292

apop5 opened this issue Aug 23, 2023 · 0 comments · Fixed by #293
Labels
complexity:easy Requires minimal background information and effort to accomplish impact:non-functional Does not have a functional impact urgency:low Little to no impact

Comments

@apop5
Copy link
Contributor

apop5 commented Aug 23, 2023

Feature Overview

BaseLib contains both AsciiStrLen and AsciiStrnLenS. Currently advacned logger is just using AsciiStrLen, but switching ot the safe string version does not require any additional changes.

Solution Overview

Switch to using AsciiStrnLenS, with appropriate maximum string length parameters.

Alternatives Considered

No response

Urgency

Low

Are you going to implement the feature request?

I will implement the feature

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

No response

@apop5 apop5 added complexity:easy Requires minimal background information and effort to accomplish impact:non-functional Does not have a functional impact labels Aug 23, 2023
@github-actions github-actions bot added the urgency:low Little to no impact label Aug 23, 2023
apop5 added a commit that referenced this issue Aug 23, 2023
## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity:easy Requires minimal background information and effort to accomplish impact:non-functional Does not have a functional impact urgency:low Little to no impact
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant