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

[Bug]: Buffer size not big enough for DFCI setting strings #41

Closed
1 task done
wenbhou opened this issue Mar 1, 2023 · 0 comments · Fixed by #36
Closed
1 task done

[Bug]: Buffer size not big enough for DFCI setting strings #41

wenbhou opened this issue Mar 1, 2023 · 0 comments · Fixed by #36
Assignees
Labels
type:bug Something isn't working urgency:medium Important with a moderate impact

Comments

@wenbhou
Copy link
Contributor

wenbhou commented Mar 1, 2023

Is there an existing issue for this?

  • I have searched existing issues

Current Behavior

The USB_PORT_STATE_STRING_SIZE & ENABLED_STRING_SIZE defined in ProviderValueAsAscii() are not big enough to accommodate DFCI setting strings, resulting some strings, like "UsbPortAuthenticated", being truncated.

Expected Behavior

The size should be greater than or equal to the MAX size of the DFCI strings with "\0" counted in.

Steps To Reproduce

  1. Install SEMM tool on the DUT
  2. Modify the ConfigureSEMM.ps1 script to set Virtual USB-C port 1 to UsbPortAuthenticated
  3. Reboot
  4. Run VerifySettings.ps1 and it shows the "USB-C Port 1 (380) = " (Expected: "USB-C Port 1 (380) = UsbPortAuthenticated")
    image

Build Environment

- OS(s): Windows 11
- Tool Chain(s): VS2022
- Targets Impacted: RELEASE, DEBUG

Version Information

0323b80191c32a7b4b32a827c863340958fc16fd

Urgency

Medium

Are you going to fix this?

I will fix it

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

No response

@wenbhou wenbhou added state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working labels Mar 1, 2023
@github-actions github-actions bot added the urgency:medium Important with a moderate impact label Mar 1, 2023
@makubacki makubacki removed the state:needs-triage Needs to triaged to determine next steps label Mar 2, 2023
mikeytdisco added a commit that referenced this issue Mar 2, 2023
Fixes #41 

## Description

1. Increase ENABLED_STRING_SIZE & USB_PORT_STATE_STRING_SIZE so that the
buffer allocated can be big enough to accommodate the string of DFCI
setting, otherwise the generated DFCI setting string will be truncated.
2. Remove define of ASSET_TAG_STRING_MAX_SIZE which is no longer used.

- [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

Verified with SEMM tool: 
1. Set USB port to UsbPortAuthenticated via ConfigureSEMM.ps1 
2. Reboot 
3. Verify with CurrentSettings.ps1. The USB port is in correct mode.

## Integration Instructions

N/A

Co-authored-by: Wenbo Hou <[email protected]>
Co-authored-by: Michael Turner <[email protected]>
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 urgency:medium Important with a moderate impact
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants