fix: enable Linux build hardening flags and CI checksec gate - #5227
Open
causten wants to merge 2 commits into
Open
fix: enable Linux build hardening flags and CI checksec gate#5227causten wants to merge 2 commits into
causten wants to merge 2 commits into
Conversation
Add cmake/Hardening.cmake with stack protector, FORTIFY_SOURCE, RELRO, PIE, and optional CET flags for Linux builds. Wire a checksec verification step into the release CI matrix to catch regressions. Fixes ROCM-26641, ROCM-26622 (SWSPLAT-32802, SWSPLAT-32783) Co-authored-by: Cursor <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Linux-focused hardening layer to MIGraphX’s CMake build and enforces the expected mitigation properties in CI for release builds.
Changes:
- Introduces
cmake/Hardening.cmakeand enables it via a newMIGRAPHX_ENABLE_HARDENINGCMake option (default ON). - Adds
tools/checksec.shto validate hardening properties (canary, RELRO, PIE) formigraphx-driverandlibmigraphx.so*. - Extends the GitHub Actions Linux release workflow to install
checksecand run the validation gate.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tools/checksec.sh |
New script to enforce hardening properties on built artifacts. |
CMakeLists.txt |
Adds MIGRAPHX_ENABLE_HARDENING option and includes hardening module. |
cmake/Hardening.cmake |
New module applying Linux compiler/linker hardening flags. |
.github/workflows/ci.yaml |
Adds a release-only CI step to run the checksec gate. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+29
to
+37
| message(STATUS "Linux build hardening enabled") | ||
| add_compile_options( | ||
| -fstack-protector-strong | ||
| -Wformat-security | ||
| -fstack-clash-protection | ||
| ) | ||
| add_compile_definitions(_FORTIFY_SOURCE=2) | ||
| add_link_options(-Wl,-z,relro -Wl,-z,now -pie) | ||
|
|
Probe optional compiler flags, apply -pie via CMAKE_EXE_LINKER_FLAGS for executables only, parse checksec JSON with readelf fallback, and install checksec 3.2.0 from a pinned deb with SHA512 verification in CI. Co-authored-by: Cursor <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
Suppressed comments (1)
cmake/Hardening.cmake:54
- Same language-scoping issue here:
check_cxx_compiler_flagonly validates the C++ compiler, butadd_compile_options(...)will be applied to C sources too. This can break C compilation on toolchains where-fcf-protection=fullis accepted for C++ but not for C (or vice versa).
check_cxx_compiler_flag("-fcf-protection=full" MIGRAPHX_HAS_FCF_PROTECTION)
if(MIGRAPHX_HAS_FCF_PROTECTION)
add_compile_options(-fcf-protection=full)
endif()
Comment on lines
+28
to
+30
| if(NOT WIN32 AND NOT APPLE AND MIGRAPHX_ENABLE_HARDENING) | ||
| message(STATUS "Linux build hardening enabled") | ||
|
|
Comment on lines
+31
to
+44
| check_cxx_compiler_flag("-fstack-protector-strong" MIGRAPHX_HAS_STACK_PROTECTOR_STRONG) | ||
| if(MIGRAPHX_HAS_STACK_PROTECTOR_STRONG) | ||
| add_compile_options(-fstack-protector-strong) | ||
| endif() | ||
|
|
||
| check_cxx_compiler_flag("-Wformat-security" MIGRAPHX_HAS_FORMAT_SECURITY) | ||
| if(MIGRAPHX_HAS_FORMAT_SECURITY) | ||
| add_compile_options(-Wformat-security) | ||
| endif() | ||
|
|
||
| check_cxx_compiler_flag("-fstack-clash-protection" MIGRAPHX_HAS_STACK_CLASH_PROTECTION) | ||
| if(MIGRAPHX_HAS_STACK_CLASH_PROTECTION) | ||
| add_compile_options(-fstack-clash-protection) | ||
| endif() |
Comment on lines
+46
to
+50
| add_compile_definitions(_FORTIFY_SOURCE=2) | ||
|
|
||
| add_link_options(-Wl,-z,relro -Wl,-z,now) | ||
| set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -pie") | ||
|
|
Comment on lines
+126
to
+138
| echo "Checking ${bin}" | ||
| json="" | ||
| if json="$(run_checksec "${bin}" 2>/dev/null)"; then | ||
| echo "${json}" | ||
| if ! verify_with_checksec_json "${bin}" "${is_shared}" "${json}"; then | ||
| failed=1 | ||
| fi | ||
| else | ||
| echo "checksec JSON unavailable; falling back to readelf for ${bin}" | ||
| if ! verify_with_readelf "${bin}" "${is_shared}"; then | ||
| failed=1 | ||
| fi | ||
| fi |
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.
Summary
cmake/Hardening.cmakewith Linux exploit-mitigation compiler/linker flags (stack protector, FORTIFY_SOURCE, RELRO/BIND_NOW, PIE, stack-clash, optional CET).MIGRAPHX_ENABLE_HARDENING(default ON).tools/checksec.shand a release-only CI step to verifymigraphx-driverandlibmigraphx.so.JIRA
Test plan
linuxrelease matrix passes build + checksec gate./tools/checksec.shreports canary, full RELRO, and PIE on shipped binariesMade with Cursor