Skip to content

Batch of CodeQL Fixes [Rebase & FF]#257

Merged
makubacki merged 3 commits into
microsoft:release/202208from
makubacki:codeql_202208_fixes
Feb 10, 2023
Merged

Batch of CodeQL Fixes [Rebase & FF]#257
makubacki merged 3 commits into
microsoft:release/202208from
makubacki:codeql_202208_fixes

Conversation

@makubacki

@makubacki makubacki commented Feb 8, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Description

Includes 3 commits that do the following:

  1. Cherry picks CodeQL deltas from release/202202
  2. Introduces a CodeQL global config file to the repo
  3. Fixes a new set of CodeQL issues (for the default rule set)

Note: Another PR is expected to address a smaller set of rules
but this covers the default rules specified in the CodeQL
plugin in this repo.

  • 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 build
  • Verified CodeQL results from build plugin
  • Tested boot to EFI shell with QemuQ35Pkg

Integration Instructions

N/A - Can help resolve some CodeQL alerts that may have been observed.

Signed-off-by: Michael Kubacki michael.kubacki@microsoft.com

@github-actions github-actions Bot added impact:non-functional Does not have a functional impact impact:security Has a security impact labels Feb 8, 2023
os-d
os-d approved these changes Feb 9, 2023
View reviewed changes
Comment thread MdeModulePkg/Application/UiApp/FrontPageCustomizedUiSupport.c Outdated
Comment thread MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c Outdated
@makubacki makubacki force-pushed the codeql_202208_fixes branch 2 times, most recently from 7032915 to 12363d8 Compare February 10, 2023 03:42
@makubacki
[Cherry-pick] CodeQL Fixes - Second Pass from 202202
d18600e 
Makes changes to comply with alerts raised by CodeQL.

A prior pass with a subset of queries used here was already done so this
is mostly addressing issues discovered by enabling a few new queries.

Most of the issues here fall into the following two categories:

1. Potential use of uninitialized pointer
2. Inconsistent integer width used in loop comparison

- [ ] Breaking change?
- Will this change break pre-existing builds or functionality without
action being taken?
  **No** - Should just be refactoring to address alerts

1. Verified CI build
2. Verified boot to EFI shell on QemuQ35Pkg

N/A

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
(cherry picked from commit 43e657c)
@makubacki
Add global CodeQL filter file to repo
9fc257f 
The CodeQL Analyze build plugin allows a global configuration file
to be specified for a repo that can define file patterns against
CodeQL rules to include and exclude results for the given file and
rule.

More details are in the plugin readme:
https://github.com/microsoft/mu_basecore/blob/release/202208/.pytool/Plugin/CodeQL/Readme.md#global-configuration

This change calls out addition of this file for future usage.

The global configuration file is useful in mu_basecore (vs specifying
the filters in individual package CI YAML files) because the repo
has high reuse. Integration repos can easily see all of the filter
activity in one file that is easy to inspect and include in their
build if needed.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
@makubacki
Batch of CodeQL Fixes
bfab119 
Includes changes across the repo for the following CodeQL rules:

- cpp/comparison-with-wider-type
- cpp/overflow-buffer
- cpp/redundant-null-check-param
- cpp/uselesstest

Some false positives are filtered in CodeQlFilters.yml.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
@makubacki makubacki merged commit 2771641 into microsoft:release/202208 Feb 10, 2023
kat-perez pushed a commit to kat-perez/mu_basecore that referenced this pull request Nov 9, 2025
@TaylorBeebe
Reduce RWX Test UT_LOG Verbosity in DxePagingAuditTestApp (microsoft#257
933c7b2 
)

## Description

There are often many ranges exempt from the RWX test, and printing every
exempt region to the log can overflow the XML buffer and result in the
actual failing ranges not to be printed.

To reduce the likelihood of missing failing ranges in the output XML,
this PR removes the UT_LOG calls for exempt regions.

- [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, ...
- [x] 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

Running on Q35

## Integration Instructions

N/A
apop5 pushed a commit to apop5/mu_basecore that referenced this pull request Dec 2, 2025
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (micros…
d38b1f1 
…oft#257)

Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit to apop5/mu_basecore that referenced this pull request Dec 15, 2025
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (micros…
5d410fa 
…oft#257)

Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit to apop5/mu_basecore that referenced this pull request Dec 16, 2025
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (micros…
8e1ad58 
…oft#257)

Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit to apop5/mu_basecore that referenced this pull request Dec 17, 2025
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (micros…
f9eb53c 
…oft#257)

Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit to apop5/mu_basecore that referenced this pull request Dec 19, 2025
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (micros…
0306263 
…oft#257)

Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit to apop5/mu_basecore that referenced this pull request Dec 22, 2025
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (micros…
0e16eeb 
…oft#257)

Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit to apop5/mu_basecore that referenced this pull request Dec 30, 2025
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (micros…
f438665 
…oft#257)

Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit to apop5/mu_basecore that referenced this pull request Jan 7, 2026
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (micros…
f645db0 
…oft#257)

Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit that referenced this pull request Jan 10, 2026
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (#257)
7bbbdd1 
Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit that referenced this pull request Jan 12, 2026
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (#257)
4705703 
Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit that referenced this pull request Jan 13, 2026
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (#257)
15b016f 
Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit that referenced this pull request Jan 15, 2026
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (#257)
d9f6bbe 
Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit that referenced this pull request Jan 28, 2026
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (#257)
5c419eb 
Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
apop5 pushed a commit that referenced this pull request Feb 5, 2026
@cfernald @apop5
SecurityPkg: Add an assert to TCG log function if log is full (#257)
348b4bd 
Currently, if the TCG log fills up, the firmware will boot only logging
some errors and the OS may or may not fail depending on scenario and
configuration. This PR adds an assert so that these truncations can be
found in testing rather then having to wait for failures in production.

- [X] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [ ] Includes tests?
- [ ] Includes documentation?

N/A

N/A

cherry-pick from 1c115e6083

Signed-off-by:  Chris Fernald <chfernal@microsoft.com>
Raymond-MS added a commit to Raymond-MS/mu_basecore that referenced this pull request Jun 12, 2026
@Raymond-MS
Revert "SecurityPkg: Add an assert to TCG log function if log is full (…
b1e3848 
…microsoft#257)"

This reverts commit 348b4bd.
makubacki pushed a commit to Raymond-MS/mu_basecore that referenced this pull request Jun 24, 2026
@Raymond-MS @makubacki
Revert "SecurityPkg: Add an assert to TCG log function if log is full (…
8532443 
…microsoft#257)"

This reverts commit 348b4bd.
Raymond-MS added a commit to Raymond-MS/mu_basecore that referenced this pull request Jun 25, 2026
@Raymond-MS
Revert "SecurityPkg: Add an assert to TCG log function if log is full (…
945c142 
…microsoft#257)"

This reverts commit 348b4bd.
Raymond-MS added a commit to Raymond-MS/mu_basecore that referenced this pull request Jun 25, 2026
@Raymond-MS
Revert "SecurityPkg: Add an assert to TCG log function if log is full (…
f9e01d3 
…microsoft#257)"

This reverts commit 348b4bd.
Raymond-MS added a commit that referenced this pull request Jun 25, 2026
@Raymond-MS
Revert "SecurityPkg: Add an assert to TCG log function if log is full (…
cb92f39 
…#257)"

This reverts commit 348b4bd.
@mu-automation mu-automation Bot mentioned this pull request Jun 26, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@os-d os-d os-d approved these changes
@apop5 apop5 apop5 approved these changes
@cfernald cfernald Awaiting requested review from cfernald
@kenlautner kenlautner Awaiting requested review from kenlautner

Assignees

No one assigned

Labels

impact:non-functional Does not have a functional impact impact:security Has a security impact

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@makubacki @os-d @apop5