-
Notifications
You must be signed in to change notification settings - Fork 68
psm: add .note.GNU-stack section as executable stack is not needed #136
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
base: master
Are you sure you want to change the base?
Conversation
36ac295 to
5abb754
Compare
| @@ -0,0 +1,3 @@ | |||
| #if defined(__linux__) && defined(__ELF__) | |||
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.
Probably should be checking for __gnu_linux__ or __GNUC__ (which despite having "C" in its name) is defined when compiling assembly and is going to be specific for gnu targets where this matters.
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.
Additionally, lets rename the file to be more self-descriptive (e.g. gnu_stack_note.s)
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.
gentoo wiki recommends it this way. https://wiki.gentoo.org/wiki/Hardened/GNU_stack_quickstart#Patching
I suspect it covers the case where the assembler is not gnu but the linker is (as this note is directed to the linker, as far as I understand)
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.
Additionally, lets rename the file to be more self-descriptive (e.g. gnu_stack_note.s)
Done
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.
Hm, having done some more research, I think this check could drop the linux check altogether and just condition on just the
defined(__ELF__)
https://github.com/llvm/llvm-project/blob/66df6cceea001a2082bc78678305a86b9bb29ecb/llvm/lib/MC/MCAsmInfoELF.cpp#L30-L37 does not emit this section on solaris, but psm does not need to worry too much about creating that one superfluous mini-section on targets that might not use it.
On GNU/Linux, when linking an executable comprising a static library created by cargo because Cargo.toml contains crate-type = ["staticlib"] the linker outputs the following warning: ``` /nix/store/dc9vaz50jg7mibk9xvqw5dqv89cxzla3-binutils-2.44/bin/ld: warning: 4f9a91766097c4c5-x86_64.o: missing .note.GNU-stack section implies executable stack /nix/store/dc9vaz50jg7mibk9xvqw5dqv89cxzla3-binutils-2.44/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker ``` (where 4f9a91766097c4c5-x86_64.o is the object corresponding to psm/src/arch/x86_64.s) Add the note requested by the linker. For some reason, executables linked directly by rustc do not seem affected. I only observed this with a c++ project linking a rust static library comprising psm (lnav).
|
can you restart the job that failed due to a timeout? It seems the button to do it is privileged. |
On GNU/Linux, when linking an executable comprising psm in a static library created by cargo because Cargo.toml contains
crate-type = ["staticlib"], the linker outputs the following warning:(where 4f9a91766097c4c5-x86_64.o is the object corresponding to psm/src/arch/x86_64.s)
Add the note requested by the linker.
For some reason, executables linked directly by rustc do not seem affected. I only observed this with a c++ project linking a rust static library comprising psm (lnav).
Note that I'm quite out of my depth here, and that I have only tested on x86_64-linux-gnu. I expect that a note will not be problematic with non-gnu toolchains, which should ignore it.