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

cc-wrapper: add support for ibt hardening flag on aarch64 & x86_64 #333895

Draft
wants to merge 7 commits into
base: staging
Choose a base branch
from

Conversation

risicle
Copy link
Contributor

@risicle risicle commented Aug 11, 2024

Description of changes

This sits on top of #331596 so perhaps review that first?

Background: https://en.wikipedia.org/wiki/Indirect_branch_tracking

Individual commit comments contain more detail on specific choices.

For IBT to be enabled at runtime, a binary's ELF files need to be marked as supporting IBT, and the linker won't mark them as such unless all their constituent .o files are marked as such. This includes the c runtime files crti.o, crtn.o, crtbegin.o et al. So for IBT to be usable at all on a toolchain, these files (belonging to glibc and gcc) must be built with IBT enabled. For x86_64, this is handled automagically by glibc and gcc's build systems. But for aarch64, it is not.

So, perhaps controversially, this patches glibc and gcc for all systems (not just pkgsExtraHardening) to build these crt files with IBT on aarch64. My reasoning is that this is safe because it's basically what is being done on x86_64 anyway and if we don't do this, the ibt hardening flag will have no real effect on individual packages in normal package sets, leaving people unable to try it out gradually. Also, because these changes rely on patches, I fear they would become bitrotted if they were made optional.

"Tested" this (via the added feature tests) extensively on x86_64 (clang & gcc, cross-building for aarch64 with clang & gcc) and aarch64 (clang & gcc, cross-building for x86_64 with clang & gcc).

However, as before I have not been able to test this on IBT-supporting hardware because I don't have access to any. This requires a recent Intel/AMD processor or, on aarch64, an ARM v8.5+-based machine (a Graviton 4 or Apple M1 might do). So again I'm going to need some community help in trying to build as many packages as possible from pkgsExtraHardening on some of these systems.

TODO: release notes, manual additions...

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@risicle risicle added the 8.has: tests This PR has tests label Aug 11, 2024
@risicle risicle requested a review from a team August 11, 2024 11:30
@github-actions github-actions bot added 6.topic: stdenv Standard environment 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related labels Aug 11, 2024
despite the feature being known as "bti" (Branch Target
Identification) on aarch64, using "ibt" (Indirect Branch
Tracking, intel's preferred term) as the hardening flag
name because bti has an unfortunate acronym collision with
Branch Target Injection, a spectre variant.
includes tests covering use of ibt/shadowstack/pacret together
as they have overlapping compiler flags on both x86_64 and
aarch64 creating some potential for conflict
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation 8.has: changelog labels Aug 11, 2024
@risicle
Copy link
Contributor Author

risicle commented Aug 11, 2024

In digging up details for the manual entry, I'm reminded that linux currently lacks userspace support for x86_64's IBT. So for now at least that's completely untestable.

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: stdenv Standard environment 8.has: changelog 8.has: documentation 8.has: package (new) This PR adds a new package 8.has: tests This PR has tests 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv This PR causes stdenv to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants