Use symcheck to locate writeable+executable object files

From what I have been able to find, compilers that try to emit object
files compatible with a GNU linker appear to add a `.note.GNU-stack`
section if the stack should not be writeable (this section is empty). We
never want a writeable stack, so extend the object file check to verify
that object files with any executable sections also have this
`.note.GNU-stack` section.

This appears to match what is done by `scanelf` to emit `!WX` [2], which
is the tool used to create the output in the issue.

Closes: #183

[1]: https://github.com/gentoo/pax-utils/blob/9ef54b472e42ba2c5479fbd86b8be2275724b064/scanelf.c#L428-L512

Author: tgross35

crates/symbol-check/Cargo.toml

@@ -8,5 +8,8 @@ publish = false
 object = "0.37.1"
 serde_json = "1.0.140"
 
+[build-dependencies]
+cc = "1.2.25"
+
 [features]
 wasm = ["object/wasm"]

crates/symbol-check/build.rs

@@ -0,0 +1,11 @@
+fn main() {
+    let intermediates = cc::Build::new()
+        .file("has_wx.c")
+        .try_compile_intermediates();
+    if let Ok(list) = intermediates {
+        let [obj] = list.as_slice() else {
+            panic!(">1 output")
+        };
+        println!("cargo::rustc-env=HAS_WX_OBJ={}", obj.display());
+    }
+}

crates/symbol-check/has_wx.c

@@ -0,0 +1,9 @@
+void intermediate(void (*)(int, int), int);
+
+int hack(int *array, int size) {
+    void store (int index, int value) {
+        array[index] = value;
+    }
+
+    intermediate(store, size);
+}

crates/symbol-check/src/main.rs

@@ -7,9 +7,11 @@ use std::io::{BufRead, BufReader};
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 
+use object::elf::SHF_EXECINSTR;
 use object::read::archive::{ArchiveFile, ArchiveMember};
 use object::{
-    File as ObjFile, Object, ObjectSection, ObjectSymbol, Symbol, SymbolKind, SymbolScope,
+    File as ObjFile, Object, ObjectSection, ObjectSymbol, SectionFlags, Symbol, SymbolKind,
+    SymbolScope,
 };
 use serde_json::Value;
 
@@ -24,6 +26,10 @@ Cargo will get invoked with `CARGO_ARGS` and the specified target. All output
 `compiler_builtins*.rlib` files will be checked.
 
 If TARGET is not specified, the host target is used.
+
+    check ARCHIVE_PATHS ...
+
+Run the same checks on the given set of paths, without invoking Cargo.
 ";
 
 fn main() {
@@ -33,12 +39,14 @@ fn main() {
 
     match &args_ref[1..] {
         ["build-and-check", target, "--", args @ ..] if !args.is_empty() => {
-            check_cargo_args(args);
             run_build_and_check(target, args);
         }
         ["build-and-check", "--", args @ ..] if !args.is_empty() => {
-            check_cargo_args(args);
-            run_build_and_check(&host_target(), args);
+            let target = &host_target();
+            run_build_and_check(target, args);
+        }
+        ["check", paths @ ..] if !paths.is_empty() => {
+            check_paths(paths);
         }
         _ => {
             println!("{USAGE}");
@@ -47,25 +55,29 @@ fn main() {
     }
 }
 
-/// Make sure `--target` isn't passed to avoid confusion (since it should be proivded only once,
-/// positionally).
-fn check_cargo_args(args: &[&str]) {
+fn run_build_and_check(target: &str, args: &[&str]) {
+    // Make sure `--target` isn't passed to avoid confusion (since it should be
+    // proivded only once, positionally).
     for arg in args {
         assert!(
             !arg.contains("--target"),
             "target must be passed positionally. {USAGE}"
         );
     }
-}
 
-fn run_build_and_check(target: &str, args: &[&str]) {
     let paths = exec_cargo_with_args(target, args);
+    check_paths(&paths);
+}
+
+fn check_paths<P: AsRef<Path>>(paths: &[P]) {
     for path in paths {
+        let path = path.as_ref();
         println!("Checking {}", path.display());
         let archive = Archive::from_path(&path);
 
         verify_no_duplicates(&archive);
         verify_core_symbols(&archive);
+        verify_no_exec_stack(&archive);
     }
 }
 
@@ -288,6 +300,63 @@ fn verify_core_symbols(archive: &Archive) {
     println!("    success: no undefined references to core found");
 }
 
+/// Check that all object files contain a section named `.note.GNU-stack`, indicating a
+/// nonexecutable stack.
+///
+/// Paraphrased from <https://www.man7.org/linux/man-pages/man1/ld.1.html>:
+///
+/// - A `.note.GNU-stack` section with the exe flag means this needs an executable stack
+/// - A `.note.GNU-stack` section without the exe flag means there is no executable stack needed
+/// - Without the section, behavior is target-specific and on some targets means an executable
+///   stack is required.
+fn verify_no_exec_stack(archive: &Archive) {
+    let mut problem_objfiles = Vec::new();
+
+    archive.for_each_object(|obj, member| {
+        if obj_requires_exe_stack(&obj) {
+            problem_objfiles.push(String::from_utf8_lossy(member.name()).into_owned());
+        }
+    });
+
+    if !problem_objfiles.is_empty() {
+        panic!("the following archive members require an executable stack: {problem_objfiles:#?}");
+    }
+
+    println!("    success: no writeable-executable sections found");
+}
+
+fn obj_requires_exe_stack(obj: &ObjFile) -> bool {
+    // Files other than elf likely do not use the same convention.
+    if !matches!(obj, ObjFile::Elf32(_) | ObjFile::Elf64(_)) {
+        return false;
+    }
+
+    let mut has_exe_sections = false;
+    for sec in obj.sections() {
+        let SectionFlags::Elf { sh_flags } = sec.flags() else {
+            unreachable!("only elf files are being checked");
+        };
+
+        let exe = (sh_flags & SHF_EXECINSTR as u64) != 0;
+
+        // If the magic section is present, its exe bit tells us whether or not the object
+        // file requires an executable stack.
+        if sec.name().unwrap_or_default() == ".note.GNU-stack" {
+            return exe;
+        }
+
+        // Otherwise, just keep track of whether or not we have exeuctable sections
+        has_exe_sections |= exe;
+    }
+
+    // Ignore object files that have no executable sections, like rmeta
+    if !has_exe_sections {
+        return false;
+    }
+
+    true
+}
+
 /// Thin wrapper for owning data used by `object`.
 struct Archive {
     data: Vec<u8>,
@@ -325,3 +394,11 @@ impl Archive {
         });
     }
 }
+
+#[test]
+fn check_obj() {
+    let p = option_env!("HAS_WX_OBJ").expect("this platform should support the wx test build");
+    let f = fs::read(p).unwrap();
+    let obj = ObjFile::parse(f.as_slice()).unwrap();
+    assert!(obj_requires_exe_stack(&obj));
+}