-
Notifications
You must be signed in to change notification settings - Fork 1.9k
build: prevent the toolchain from emitting an executable stack #11258
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
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds Linux-specific assembler and linker flags to disable executable stacks for executables and shared libraries in the non-MSVC build path via CMake. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (27)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CMakeLists.txt
Outdated
| # security hardening. | ||
| add_compile_options(-Wa,--noexecstack) | ||
| set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,-z,noexecstack") | ||
| set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,-z,noexecstack") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard noexecstack flags for non-ELF platforms
The new linker flags -Wl,-z,noexecstack are applied unconditionally for all non-MSVC builds, but Darwin’s ld64 (and other non-ELF linkers) reject -z options, causing the build to fail on macOS and similar targets. These flags should be conditioned on ELF toolchains or excluded for Apple builds to avoid breaking supported platforms.
Useful? React with 👍 / 👎.
|
@edsiper think it needs to be conditional for only the platforms that support it, don't think macOS does. Need to double check the older compilers as well |
Signed-off-by: Eduardo Silva <[email protected]>
64c6483 to
8912cdb
Compare
Fixes #10513
Prevent the toolchain from emitting an executable stack, which triggers kernel warnings (e.g. "started with executable stack") and weakens security hardening.
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.