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

Synthetic object files disable control flow protection features #103001

Closed
pcc opened this issue Oct 13, 2022 · 4 comments · Fixed by #110304
Closed

Synthetic object files disable control flow protection features #103001

pcc opened this issue Oct 13, 2022 · 4 comments · Fixed by #110304
Assignees
Labels
A-linkage Area: linking into static, shared libraries and binaries A-security Area: Security (example: address space layout randomization). C-bug Category: This is a bug. PG-exploit-mitigations Project group: Exploit mitigations

Comments

@pcc
Copy link
Contributor

pcc commented Oct 13, 2022

I noticed that the synthetic object files added in #95604 will disable the IBT (on x86, enabled by -Z cf-protection=branch) and BTI (on AArch64, enabled by -Z branch-protection=bti) features because the object files are missing .note.gnu.property sections indicating that the object file is compatible with those features. Normally, if an object file is missing a .note.gnu.property section, the linker will disable all such features, on the assumption that the object file is not compatible.

This issue is reproducible on the master branch (slightly awkwardly because many distros don't ship IBT-enabled *crt*.o files, and neither is it enabled in Rust's standard library by default):

RUSTFLAGS_NOT_BOOTSTRAP='-Zcf-protection=branch' python3 x.py build  --target x86_64-unknown-linux-gnu --stage 1
rustup toolchain link stage1 build/x86_64-unknown-linux-gnu/stage1

In another directory:

> cat hello.rs
fn main() {
    println!("hello world");
}
> rustc +stage1 -Z cf-protection=branch hello.rs -C link-args='-nostartfiles'
> readelf -nW hello

Displaying notes found in: .note.gnu.build-id
  Owner                Data size 	Description
  GNU                  0x00000014	NT_GNU_BUILD_ID (unique build ID bitstring)	    Build ID: 9bc8182397b263d79d29c83448350ec033a6f66b

After commenting out the line of code that adds symbols.o to the link:

diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index 95e72184ff0..ed314db6772 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -1795,7 +1795,7 @@ fn add_linked_symbol_object(
     if let Err(e) = result {
         sess.fatal(&format!("failed to write {}: {}", path.display(), e));
     }
-    cmd.add_object(&path);
+    //cmd.add_object(&path);
 }
 
 /// Add object files containing code from the current crate.

the binary has the correct property note:

Displaying notes found in: .note.gnu.property
  Owner                Data size 	Description
  GNU                  0x00000010	NT_GNU_PROPERTY_TYPE_0	      Properties: x86 feature: IBT

Displaying notes found in: .note.gnu.build-id
  Owner                Data size 	Description
  GNU                  0x00000014	NT_GNU_BUILD_ID (unique build ID bitstring)	    Build ID: fb555c532955966767702c5af52844dbcc9a386c
@pcc pcc added the C-bug Category: This is a bug. label Oct 13, 2022
@bjorn3 bjorn3 added A-linkage Area: linking into static, shared libraries and binaries A-security Area: Security (example: address space layout randomization). labels Oct 14, 2022
@pcc
Copy link
Contributor Author

pcc commented Oct 19, 2022

I can think of three possible solutions:

  1. Emit the appropriate GNU property section into the synthetic object file according to the passed -Z flags. This would work, but it could easily get out of sync with the GNU properties produced by the compiler.
  2. Implement a change in linker behavior so that the GNU properties are not affected by object files that do not contain SHF_ALLOC sections. This would make sense because an object file without SHF_ALLOC sections cannot on its own affect the contents of the final binary, which means that it cannot cause the binary to become {BTI,IBT}-incompatible.
  3. Add a linker feature that implements equivalent behavior to the synthetic object file, e.g. a flag that is equivalent to -u but does not inhibit --gc-sections, and then use that on ELF platforms instead of the synthetic object file if the linker supports it.

@MaskRay may have thoughts on this.

@bjorn3
Copy link
Member

bjorn3 commented Oct 20, 2022

Option 3 is not enough to solve this. The crate metadata object is also produced without these properties and can't be replaced with a linker option. Option 2 will take a long time before everyone has a new linker and needs coordination with ld.bfd, ld.gold, ld.ldd and ld.mold. As such option 1 makes most sense to me.

@MaskRay
Copy link
Contributor

MaskRay commented Oct 20, 2022

Option 2 needs coordination among linker maintainers and it does impose some complexity to linkers. And as mentioned, it requires very new linkers. If Rust is open to 1, I think it is a great direction.

@cchiw
Copy link

cchiw commented Apr 3, 2023

@rustbot claim

This was referenced Apr 13, 2023
@cuviper cuviper added the PG-exploit-mitigations Project group: Exploit mitigations label Apr 27, 2023
@bors bors closed this as completed in 02a85bd May 9, 2023
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jul 12, 2023
Use constants from object crate

Replace hard-coded values with  `GNU_PROPERTY_{X86|AARCH64}_FEATURE_1_AND` from the object crate.

When working on  [issue](rust-lang#103001) it was suggested that we moved these constants to the object crate .  [PR](gimli-rs/object#537). Now that that the object crate has been updated  [PR](rust-lang#111413) we can make this change.
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jul 12, 2023
Use constants from object crate

Replace hard-coded values with  `GNU_PROPERTY_{X86|AARCH64}_FEATURE_1_AND` from the object crate.

When working on  [issue](rust-lang#103001) it was suggested that we moved these constants to the object crate .  [PR](gimli-rs/object#537). Now that that the object crate has been updated  [PR](rust-lang#111413) we can make this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries A-security Area: Security (example: address space layout randomization). C-bug Category: This is a bug. PG-exploit-mitigations Project group: Exploit mitigations
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants