.pytool/ImageValidation: Print invalid dir paths #1142
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Print directory paths considered invalid to aid debugging - Built native OS filepaths using os.path.join for walk dirs - Clean up trailing whitespace throughout the file Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
github-actions
bot
added
the
impact:non-functional
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev/202405 #1142 +/- ##
===========================================
Coverage 1.59% 1.59%
===========================================
Files 1448 1448
Lines 362490 362490
Branches 5632 5632
===========================================
Hits 5776 5776
Misses 356607 356607
Partials 107 107
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
apop5
approved these changes
Sep 18, 2024
Javagedes
approved these changes
Sep 18, 2024
makubacki
added a commit
to makubacki/mu_basecore
that referenced
this pull request
Sep 23, 2024
…soft#1142) - Print directory paths considered invalid to aid debugging - Build native OS file paths using os.path.join for walk dirs - Clean up trailing whitespace throughout the file - [ ] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? - Local build with the plugin - Tested invalid directory printing by adding an invalid arch to `TARGET_ARCH` (so the directory doesn't exist in build output). N/A - Minor tweaks Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> (cherry picked from commit 66f3d70)
makubacki
added a commit
that referenced
this pull request
Sep 23, 2024
- Print directory paths considered invalid to aid debugging - Build native OS file paths using os.path.join for walk dirs - Clean up trailing whitespace throughout the file - [ ] Impacts functionality? - [ ] Impacts security? - [ ] Breaking change? - [ ] Includes tests? - [ ] Includes documentation? - Local build with the plugin - Tested invalid directory printing by adding an invalid arch to `TARGET_ARCH` (so the directory doesn't exist in build output). N/A - Minor tweaks Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com> (cherry picked from commit 66f3d70)
ProjectMuBot
referenced
this pull request
in microsoft/mu_tiano_platforms
Sep 25, 2024
Introduces 8 new commits in [MU_BASECORE](https://github.com/microsoft/mu_basecore.git). <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/microsoft/mu_basecore/commit/95a58f8ff1baa410405b6b0c56cb4666b7c23364">95a58f</a> [Cherry-Pick] Support Report Status Code in the UefiPxe driver.</li> <li><a href="https://github.com/microsoft/mu_basecore/commit/ba672c3aca5c96e493fbf3628ee779388d82634b">ba672c</a> [CHERRY-PICK] MdePkg: Bring in mocks from 2311 (<a href="https://github.com/microsoft/mu_basecore/pull/1139">#1139</a>)</li> <li><a href="https://github.com/microsoft/mu_basecore/commit/b0921a62ca99bf5ddd56a65fd2d1bb4dcb5d87ad">b0921a</a> Repo File Sync: Update PR template text and Rust version to 1.80.0 (<a href="https://github.com/microsoft/mu_basecore/pull/1135">#1135</a>)</li> <li><a href="https://github.com/microsoft/mu_basecore/commit/a8f504e0ae2138f4b0c4b00bd0e1664b6186e444">a8f504</a> pip: bump regex from 2024.7.24 to 2024.9.11 (<a href="https://github.com/microsoft/mu_basecore/pull/1144">#1144</a>)</li> <li><a href="https://github.com/microsoft/mu_basecore/commit/5724e5f736815eaa8430ea19f7b13bf6f383a01d">5724e5</a> pip: update edk2-pytool-library requirement from ~=0.21.10 to ~=0.21.11 (<a href="https://github.com/microsoft/mu_basecore/pull/1143">#1143</a>)</li> <li><a href="https://github.com/microsoft/mu_basecore/commit/d857e20486701750a011dbd15f65597495728ea7">d857e2</a> [CHERRY-PICK] .pytool/ImageValidation: Print invalid dir paths (<a href="https://github.com/microsoft/mu_basecore/pull/1142">#1142</a>)</li> <li><a href="https://github.com/microsoft/mu_basecore/commit/85f493899fc73171f84782747da2d6e3b9ebfb65">85f493</a> [SQUASH ON REBASE] ImageValidation.py: Support gitignore style syntax for file exclusion (<a href="https://github.com/microsoft/mu_basecore/pull/1140">#1140</a>)</li> <li><a href="https://github.com/microsoft/mu_basecore/commit/f2547000cccf6f8d37d730498c8f0b5a91ce8d89">f25470</a> MdeModulePkg/SMM: Initialize 'WillReturn' variable</li> </ul> </details> Signed-off-by: Project Mu Bot <mubot@microsoft.com>
apop5
pushed a commit
to apop5/mu_basecore
that referenced
this pull request
Jan 2, 2025
The addition of an PE/COFF image validation build plugin that will
verify all pe images against requirements defined in the configuration
file.
This plugin only runs if a path to a config file is provided via the
command line (PE_VALIDATION_PATH) or preferably Platform hardcoded in
the PlatformBuild.py file, so that each platform can have their own
requirements.
Previously, ImageValidation was an "opt-in" plugin by setting a build
variable `PE_VALIDATION_PATH`, however with this pull request, Image
Validation will be on by default, with some default configuration that
can be changed with a custom configuration yaml file.
The default requirements are:
1. All efi binaries must not be both write and execute
2. All efi binaries must have an image base of 0x0
3. All dxe phase binaries must be 4k section aligned, with the one
exception of AARCH64 DXE_RUNTIME_DRIVERS, which must be 64k aligned.
compiled binaries that need to be opted out of, can do so by adding an
`IGNORE_LIST` in the configuration file
```json
{
"IGNORE_LIST": ["Shell.efi", "etc"]
}
```
In previous iterations, the profile was determined by parsing the
makefile, looking for MODULE_TYPE. As each OS / tool chain may use a
different makefile type, this was not a reliable method. This updates
the plugin to read the INF for the compiled efi file to determine the
MODULE_TYPE and thus the profile.
The PE parsing functionality was modified to only parse the
headers of the image, rather than the entire image. This change is made
to improve performance and also the probability of failing to parse the
entire image. This comes after this commit
(erocarrera/pefile#365) in pefile resulted in
efi image parsing failures, breaking the build.
This commit also wraps the parsing of the image in a try-except block to
catch any exceptions that may be raised during parsing, to cleanly exit.
- Print directory paths considered invalid to aid debugging
- Build native OS file paths using os.path.join for walk dirs
- Clean up trailing whitespace throughout the file
Add gitignore style syntax for file exclusion
Contains:
- microsoft#340
- microsoft#537
- microsoft#539
- microsoft#747
- microsoft#1100
- microsoft#1127
- microsoft#1142
- microsoft#1140
Confirmed successful execution of the plugin on Windows with QemuQ35 and
Ubuntu with QemuSbsa
Validated on qemuq35 that the module type was successfully parsed.
Validated pipelines build on mu_tiano_platforms
- Local build with the plugin
- Tested invalid directory printing by adding an invalid arch
to `TARGET_ARCH` (so the directory doesn't exist in build output).
Ensured existing syntax (filename only) continues to work. Ensured
gitignore style syntax now works.
Platforms that begin to fail this test will need to generate a
configuration yaml file, and set a stuart build variable,
`PE_VALIDATION_PATH` to it. It is suggested to do this in the Platform's
`PlatformBuild.py`.
**The Correct Integration** is to evaluate the binary and why it is not
meeting the requirements. The platform can elect to update the
compilation of the binary to meet the requirements, add or override
validation rules for certain MODULE_TYPEs, or simply add the binary to
the ignore list. Please review the Plugin's readme.md file for more
details on doing any of these things.
Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
apop5
pushed a commit
to apop5/mu_basecore
that referenced
this pull request
Feb 2, 2025
The addition of an PE/COFF image validation build plugin that will
verify all pe images against requirements defined in the configuration
file.
This plugin only runs if a path to a config file is provided via the
command line (PE_VALIDATION_PATH) or preferably Platform hardcoded in
the PlatformBuild.py file, so that each platform can have their own
requirements.
Previously, ImageValidation was an "opt-in" plugin by setting a build
variable `PE_VALIDATION_PATH`, however with this pull request, Image
Validation will be on by default, with some default configuration that
can be changed with a custom configuration yaml file.
The default requirements are:
1. All efi binaries must not be both write and execute
2. All efi binaries must have an image base of 0x0
3. All dxe phase binaries must be 4k section aligned, with the one
exception of AARCH64 DXE_RUNTIME_DRIVERS, which must be 64k aligned.
compiled binaries that need to be opted out of, can do so by adding an
`IGNORE_LIST` in the configuration file
```json
{
"IGNORE_LIST": ["Shell.efi", "etc"]
}
```
In previous iterations, the profile was determined by parsing the
makefile, looking for MODULE_TYPE. As each OS / tool chain may use a
different makefile type, this was not a reliable method. This updates
the plugin to read the INF for the compiled efi file to determine the
MODULE_TYPE and thus the profile.
The PE parsing functionality was modified to only parse the
headers of the image, rather than the entire image. This change is made
to improve performance and also the probability of failing to parse the
entire image. This comes after this commit
(erocarrera/pefile#365) in pefile resulted in
efi image parsing failures, breaking the build.
This commit also wraps the parsing of the image in a try-except block to
catch any exceptions that may be raised during parsing, to cleanly exit.
- Print directory paths considered invalid to aid debugging
- Build native OS file paths using os.path.join for walk dirs
- Clean up trailing whitespace throughout the file
Add gitignore style syntax for file exclusion
Contains:
- microsoft#340
- microsoft#537
- microsoft#539
- microsoft#747
- microsoft#1100
- microsoft#1127
- microsoft#1142
- microsoft#1140
Confirmed successful execution of the plugin on Windows with QemuQ35 and
Ubuntu with QemuSbsa
Validated on qemuq35 that the module type was successfully parsed.
Validated pipelines build on mu_tiano_platforms
- Local build with the plugin
- Tested invalid directory printing by adding an invalid arch
to `TARGET_ARCH` (so the directory doesn't exist in build output).
Ensured existing syntax (filename only) continues to work. Ensured
gitignore style syntax now works.
Platforms that begin to fail this test will need to generate a
configuration yaml file, and set a stuart build variable,
`PE_VALIDATION_PATH` to it. It is suggested to do this in the Platform's
`PlatformBuild.py`.
**The Correct Integration** is to evaluate the binary and why it is not
meeting the requirements. The platform can elect to update the
compilation of the binary to meet the requirements, add or override
validation rules for certain MODULE_TYPEs, or simply add the binary to
the ignore list. Please review the Plugin's readme.md file for more
details on doing any of these things.
Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
apop5
pushed a commit
to apop5/mu_basecore
that referenced
this pull request
Feb 3, 2025
The addition of an PE/COFF image validation build plugin that will
verify all pe images against requirements defined in the configuration
file.
This plugin only runs if a path to a config file is provided via the
command line (PE_VALIDATION_PATH) or preferably Platform hardcoded in
the PlatformBuild.py file, so that each platform can have their own
requirements.
Previously, ImageValidation was an "opt-in" plugin by setting a build
variable `PE_VALIDATION_PATH`, however with this pull request, Image
Validation will be on by default, with some default configuration that
can be changed with a custom configuration yaml file.
The default requirements are:
1. All efi binaries must not be both write and execute
2. All efi binaries must have an image base of 0x0
3. All dxe phase binaries must be 4k section aligned, with the one
exception of AARCH64 DXE_RUNTIME_DRIVERS, which must be 64k aligned.
compiled binaries that need to be opted out of, can do so by adding an
`IGNORE_LIST` in the configuration file
```json
{
"IGNORE_LIST": ["Shell.efi", "etc"]
}
```
In previous iterations, the profile was determined by parsing the
makefile, looking for MODULE_TYPE. As each OS / tool chain may use a
different makefile type, this was not a reliable method. This updates
the plugin to read the INF for the compiled efi file to determine the
MODULE_TYPE and thus the profile.
The PE parsing functionality was modified to only parse the
headers of the image, rather than the entire image. This change is made
to improve performance and also the probability of failing to parse the
entire image. This comes after this commit
(erocarrera/pefile#365) in pefile resulted in
efi image parsing failures, breaking the build.
This commit also wraps the parsing of the image in a try-except block to
catch any exceptions that may be raised during parsing, to cleanly exit.
- Print directory paths considered invalid to aid debugging
- Build native OS file paths using os.path.join for walk dirs
- Clean up trailing whitespace throughout the file
Add gitignore style syntax for file exclusion
Contains:
- microsoft#340
- microsoft#537
- microsoft#539
- microsoft#747
- microsoft#1100
- microsoft#1127
- microsoft#1142
- microsoft#1140
Confirmed successful execution of the plugin on Windows with QemuQ35 and
Ubuntu with QemuSbsa
Validated on qemuq35 that the module type was successfully parsed.
Validated pipelines build on mu_tiano_platforms
- Local build with the plugin
- Tested invalid directory printing by adding an invalid arch
to `TARGET_ARCH` (so the directory doesn't exist in build output).
Ensured existing syntax (filename only) continues to work. Ensured
gitignore style syntax now works.
Platforms that begin to fail this test will need to generate a
configuration yaml file, and set a stuart build variable,
`PE_VALIDATION_PATH` to it. It is suggested to do this in the Platform's
`PlatformBuild.py`.
**The Correct Integration** is to evaluate the binary and why it is not
meeting the requirements. The platform can elect to update the
compilation of the binary to meet the requirements, add or override
validation rules for certain MODULE_TYPEs, or simply add the binary to
the ignore list. Please review the Plugin's readme.md file for more
details on doing any of these things.
Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
apop5
pushed a commit
that referenced
this pull request
Feb 3, 2025
The addition of an PE/COFF image validation build plugin that will
verify all pe images against requirements defined in the configuration
file.
This plugin only runs if a path to a config file is provided via the
command line (PE_VALIDATION_PATH) or preferably Platform hardcoded in
the PlatformBuild.py file, so that each platform can have their own
requirements.
Previously, ImageValidation was an "opt-in" plugin by setting a build
variable `PE_VALIDATION_PATH`, however with this pull request, Image
Validation will be on by default, with some default configuration that
can be changed with a custom configuration yaml file.
The default requirements are:
1. All efi binaries must not be both write and execute
2. All efi binaries must have an image base of 0x0
3. All dxe phase binaries must be 4k section aligned, with the one
exception of AARCH64 DXE_RUNTIME_DRIVERS, which must be 64k aligned.
compiled binaries that need to be opted out of, can do so by adding an
`IGNORE_LIST` in the configuration file
```json
{
"IGNORE_LIST": ["Shell.efi", "etc"]
}
```
In previous iterations, the profile was determined by parsing the
makefile, looking for MODULE_TYPE. As each OS / tool chain may use a
different makefile type, this was not a reliable method. This updates
the plugin to read the INF for the compiled efi file to determine the
MODULE_TYPE and thus the profile.
The PE parsing functionality was modified to only parse the
headers of the image, rather than the entire image. This change is made
to improve performance and also the probability of failing to parse the
entire image. This comes after this commit
(erocarrera/pefile#365) in pefile resulted in
efi image parsing failures, breaking the build.
This commit also wraps the parsing of the image in a try-except block to
catch any exceptions that may be raised during parsing, to cleanly exit.
- Print directory paths considered invalid to aid debugging
- Build native OS file paths using os.path.join for walk dirs
- Clean up trailing whitespace throughout the file
Add gitignore style syntax for file exclusion
Contains:
- #340
- #537
- #539
- #747
- #1100
- #1127
- #1142
- #1140
Confirmed successful execution of the plugin on Windows with QemuQ35 and
Ubuntu with QemuSbsa
Validated on qemuq35 that the module type was successfully parsed.
Validated pipelines build on mu_tiano_platforms
- Local build with the plugin
- Tested invalid directory printing by adding an invalid arch
to `TARGET_ARCH` (so the directory doesn't exist in build output).
Ensured existing syntax (filename only) continues to work. Ensured
gitignore style syntax now works.
Platforms that begin to fail this test will need to generate a
configuration yaml file, and set a stuart build variable,
`PE_VALIDATION_PATH` to it. It is suggested to do this in the Platform's
`PlatformBuild.py`.
**The Correct Integration** is to evaluate the binary and why it is not
meeting the requirements. The platform can elect to update the
compilation of the binary to meet the requirements, add or override
validation rules for certain MODULE_TYPEs, or simply add the binary to
the ignore list. Please review the Plugin's readme.md file for more
details on doing any of these things.
Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Print directory paths considered invalid to aid debugging
Build native OS file paths using os.path.join for walk dirs
Clean up trailing whitespace throughout the file
Impacts functionality?
Impacts security?
Breaking change?
Includes tests?
Includes documentation?
How This Was Tested
to
TARGET_ARCH(so the directory doesn't exist in build output).Integration Instructions
N/A - Minor tweaks