Skip to content
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

Do not discard .gnu.note, GNU ld complains when we link object files #570

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dinosaure
Copy link
Collaborator

Fix #562 (/cc @palainp)

@palainp
Copy link
Contributor

palainp commented Mar 1, 2024

Thank you @dinosaure . As @Kensan pointed in #562 I though about adding something rather than removing the /DISCARD/ part. It seems that it's mandatory to add the lines in the asm files and not in thge linker script. About this PR and the change proposed at https://github.com/codelabs-ch/solo5/tree/asm-noexec-stack, when I compile qubes-mirage-firewall, I have:

  • current head
/usr/bin/ld: warning: /home/user/.opam/4.14.1/bin/../lib/x86_64-solo5-none-static/solo5_xen.o: requires executable stack (because the .note.GNU-stack section is executable)
  • this PR
/usr/bin/ld: warning: /home/user/.opam/4.14.1/bin/../lib/x86_64-solo5-none-static/solo5_xen.o: requires executable stack (because the .note.GNU-stack section is executable)
  • the commit proposed that add some asm notes
/usr/bin/ld: warning: amd64.o: missing .note.GNU-stack section implies executable stack
/usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker

That warning comes from the caml compiler directory, so it might be relevant to also investigate there.

So to me the issue should be fixed by https://github.com/codelabs-ch/solo5/tree/asm-noexec-stack :)

@dinosaure
Copy link
Collaborator Author

Yes, I agree to put the .note.GNUinto assembly code and discard it at the link time 👍.

That warning comes from the caml compiler directory, so it might be relevant to also investigate there.

You mean with ocaml-solo5? Not sure to understand well this warning. The warning will be deleted then in next versions of the linker?

@palainp
Copy link
Contributor

palainp commented Mar 3, 2024

You mean with ocaml-solo5? Not sure to understand well this warning. The warning will be deleted then in next versions of the linker?

The only amd64.o I've found is in ~/.opam/default/.opam-switch/build/ocaml-solo5.xxx/ocaml/runtime/ and ocaml has assembly files (e.g. https://github.com/ocaml/ocaml/blob/trunk/runtime/amd64.S). It seems that this warning has been recently added and reading https://discuss.ocaml.org/t/ld-error-missing-note-gnu-stack-section/12478 leave me thinking about adding the directive to ocaml-solo5.

So to me this is unrelated to solo5, the commit proposed by @Kensan LGTM.

To move forward I wonder if the best is to add (flags (:standard -cclib "-z noexecstack")) in ocaml-solo5 (if possible) or PR at ocaml to add a similar .section .note.GNU-stack,"",%progbits in the asm files? EDIT: As it seems there's no issue about that in the ocaml repository, it's probably relevant to ocaml-solo5 :)

Add .note.GNU-stack section to assembly files which recent ld versions
require to determine whether an executable stack is required. If no such
ELF section is present, the linker assumes that the stack needs
executable rights. See also [1].

[1] - https://wiki.gentoo.org/wiki/Hardened/GNU_stack_quickstart
@dinosaure
Copy link
Collaborator Author

To move forward I wonder if the best is to add (flags (:standard -cclib "-z noexecstack")) in ocaml-solo5 (if possible) or PR at ocaml to add a similar .section .note.GNU-stack,"",%progbits in the asm files? EDIT: As it seems there's no issue about that in the ocaml repository, it's probably relevant to ocaml-solo5 :)

So, I will let this PR opens but we probably should check if the warning continues to appear if we update ocaml-solo5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GNU ld wants executable stack
3 participants