fluent-bit
fluent-bit copied to clipboard
build: Set security flags for release builds
Signed-off-by: Christian Menges [email protected]
Set security flags for release builds using gcc or clang:
-Wl,-z,relro,-z,nowSet Global Offset Table (GOT) to read-only. In theory, this increases startup times, but I couldn't observe a performance degradation.-fstack-protectorProtect against stack manipulation.-D_FORTIFY_SOURCE=1Replace certain functions with more secure alternatives. With level 1, most of the added security checks are optimized away.
Fixes #7315
Enter [N/A] in the box, if an item is not applicable to your change.
Testing Before we can approve your change; please submit the following in a comment:
- [N/A] Example configuration file for the change
- [N/A] Debug log output from testing the change
- [N/A] Attached Valgrind output that shows no leaks or memory corruption was found
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
- [ ] Attached local packaging test output showing all targets (including any new ones) build.
Documentation
- [N/A] Documentation required for this feature
Backporting
- [N/A] Backport to latest stable release.
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.
We probably need a build of all targets to confirm this works ok on everything.
@patrick-stephens I executed the package tests in my fork. Unfortunately, the timeout was reached before the tests completed. However, all executed tests were successful. Build log: fluent-bit-build-logs.txt
@niedbalski Could you please review this PR? Thanks
@Garfield96 thanks, we're both on PTO at the moment but I'll catch up with @niedbalski once he's back.
Hi @patrick-stephens and @niedbalski, this PR is already open for some time. Shall I change something or do you require additional information to review the PR?
@Garfield96 I appreciate your time - unfortunately @niedbalski is off for an extended period.
Hi @niedbalski, I saw you are back. Could you please have a look at this PR? Thanks
@Garfield96 can you update the merge commit title (and ideally rebase)?
@patrick-stephens I rebased the PR. The test failures are unrelated to the change and are also present in several other recently opened PRs.
@leonardo-albertovich Can this PR get merged?
Is this the cause of the following in my dmesg output?
[ 365.250531] process 'opt/fluent-bit/bin/fluent-bit' started with executable stack
Is this the cause of the following in my
dmesgoutput?[ 365.250531] process 'opt/fluent-bit/bin/fluent-bit' started with executable stack
@rossigee I'm not sure what you're asking there - this PR is not merged so any changes in it are not in official releases. As to why you're seeing that message, this really should be captured on an issue (if it is a problem) along with full details on the version of FB, OS, config, etc.?
@patrick-stephens - sorry I wasn't clear. I saw this message while reviewing the logs on one of my hosts, and was alarmed. I wasn't sure if my 'fluent-bit' binary (official release build v2.1.9, deployed as a K8S DS) had somehow been tampered with, but on further investigation I came across this PR which addresses certain 'official release build' compile-time issues, one of which appears to be related to preventing the executable stack problem.
I wasn't able to find anyone else reporting the same issue by Googling the error message, and I was curious as to whether or not this PR would solve my problem (once it's merged and released). Apologies for the noise.
@rossigee I assume that this PR will remove the warning from your logs.
@patrick-stephens I'm sorry for asking again, but can this PR be merged soon? I think a lot of users would benefit from it and as mentioned by @rossigee and https://github.com/fluent/fluent-bit/issues/7315, some platforms even issue warnings for the current fluent-bit binary.
@Garfield96 as I understand it I think we're happy with this - I'm not the best person to review the compilation flags themselves however @leonardo-albertovich indicated he was so I'll ping @edsiper to see if we can get it merged.
Hi @leonardo-albertovich and @edsiper, I'd highly appreciate if you could review and, ideally, merge this PR. Thanks
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.
added to Milestone v3.1.0