Enable LTO without weakening secure isolation - #18
Open
aidangarske wants to merge 4 commits into
Open
aidangarske wants to merge 4 commits into
aidangarske wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Two critical secure-layout and zero-heap validation issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
Open (2)
What changed in this PR
Enables Secure-image LTO by default while preserving isolation-sensitive placement and validating final ELF layout.
Changes:
- Adds LTO configuration, exclusions, map output, and cleanup.
- Adds post-link validation and host/CI coverage.
- Updates linker placement and documentation.
| File | Summary | Final review comment |
|---|---|---|
tools/check_secure_layout.py |
Validates ELF layout and heap policy. | Critical (1 vote): Add _calloc_r, _realloc_r, and _sbrk_r to the zero-heap denylist and generated tests. |
tests/host/secure_layout/Makefile |
Runs validator and configuration tests. | No final findings. |
tests/host/Makefile |
Registers the secure-layout suite. | No final findings. |
src/services/wolfhsm/runner/secure.ld |
Preserves isolation-sensitive data placement. | Critical (2 votes): Ensure conformance NVM state remains in the protected section under LTO by adding matching patterns or keeping the object non-LTO. |
mk/common.mk |
Configures LTO, exclusions, maps, and cleanup. | Nit (1 vote): Wire exclusions to the actual native object list or remove the unsupported path. |
mk/arch-armv8m.mk |
Adds post-link validation. | No final findings. |
docs/Testing.md |
Documents LTO validation coverage. | No final findings. |
docs/Security-Model.md |
Documents LTO isolation safeguards. | No final findings. |
docs/Macros.md |
Documents WT_LTO. |
No final findings. |
docs/Building.md |
Documents LTO builds and outputs. | No final findings. |
.github/workflows/cross-compile.yml |
Tests LTO builds, size reduction, and cleanup. | No final findings. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #18
No scan targets match the changed files in this PR. Review skipped.
This file contains hidden or 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
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.

enables LTO by default while excluding assembly, CMSE, and isolation-band objects whose placement must remain stable. It adds post-link secure-layout validation, removes rejected image artifacts, and tests configuration toggles and failure paths.