Skip to content

Commit 48b8755

Browse files
committed
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
1 parent 41b5e34 commit 48b8755

File tree

4 files changed

+165
-24
lines changed

4 files changed

+165
-24
lines changed

crates/symbol-check/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,6 @@ serde_json = "1.0.140"
1111

1212
[features]
1313
wasm = ["object/wasm"]
14+
15+
[build-dependencies]
16+
cc = "1.2.25"

crates/symbol-check/build.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
fn main() {
2+
let intermediates = cc::Build::new()
3+
.file("has_wx.c")
4+
.try_compile_intermediates();
5+
if let Ok(list) = intermediates {
6+
let [obj] = list.as_slice() else { panic!() };
7+
println!("cargo::rustc-env=HAS_WX_OBJ={}", obj.display());
8+
}
9+
}

crates/symbol-check/has_wx.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
void intermediate(void (*)(int, int), int);
2+
3+
int hack(int *array, int size) {
4+
void store (int index, int value) {
5+
array[index] = value;
6+
}
7+
8+
intermediate(store, size);
9+
}

crates/symbol-check/src/main.rs

Lines changed: 144 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,12 @@ use std::io::{BufRead, BufReader};
77
use std::path::{Path, PathBuf};
88
use std::process::{Command, Stdio};
99

10+
use object::elf::SHF_EXECINSTR;
1011
use object::read::archive::{ArchiveFile, ArchiveMember};
11-
use object::{Object, ObjectSymbol, Symbol, SymbolKind, SymbolScope, SymbolSection};
12+
use object::{
13+
File as ObjFile, Object, ObjectSection, ObjectSymbol, SectionFlags, Symbol, SymbolKind,
14+
SymbolScope, SymbolSection,
15+
};
1216
use serde_json::Value;
1317

1418
const CHECK_LIBRARIES: &[&str] = &["compiler_builtins", "builtins_test_intrinsics"];
@@ -28,13 +32,11 @@ fn main() {
2832
let args_ref = args.iter().map(String::as_str).collect::<Vec<_>>();
2933

3034
match &args_ref[1..] {
31-
["build-and-check", rest @ ..] if !rest.is_empty() => {
32-
let paths = exec_cargo_with_args(rest);
33-
for path in paths {
34-
println!("Checking {}", path.display());
35-
verify_no_duplicates(&path);
36-
verify_core_symbols(&path);
37-
}
35+
["build-and-check", "--target", target, args @ ..] if !args.is_empty() => {
36+
run_build_and_check(Some(target), args);
37+
}
38+
["build-and-check", args @ ..] if !args.is_empty() => {
39+
run_build_and_check(None, args);
3840
}
3941
_ => {
4042
println!("{USAGE}");
@@ -43,12 +45,43 @@ fn main() {
4345
}
4446
}
4547

48+
fn run_build_and_check(target: Option<&str>, args: &[&str]) {
49+
let paths = exec_cargo_with_args(target, args);
50+
for path in paths {
51+
println!("Checking {}", path.display());
52+
let archive = Archive::from_path(&path);
53+
54+
verify_no_duplicates(&archive);
55+
verify_core_symbols(&archive);
56+
verify_no_exec_stack(&archive);
57+
}
58+
}
59+
60+
fn host_target() -> String {
61+
let out = Command::new("rustc")
62+
.arg("--version")
63+
.arg("--verbose")
64+
.output()
65+
.unwrap();
66+
assert!(out.status.success());
67+
let out = String::from_utf8(out.stdout).unwrap();
68+
out.lines()
69+
.find_map(|s| s.strip_prefix("host: "))
70+
.unwrap()
71+
.to_owned()
72+
}
73+
4674
/// Run `cargo build` with the provided additional arguments, collecting the list of created
4775
/// libraries.
48-
fn exec_cargo_with_args(args: &[&str]) -> Vec<PathBuf> {
76+
fn exec_cargo_with_args(target: Option<&str>, args: &[&str]) -> Vec<PathBuf> {
77+
let mut host = String::new();
78+
let target = target.unwrap_or_else(|| {
79+
host = host_target();
80+
host.as_str()
81+
});
82+
4983
let mut cmd = Command::new("cargo");
50-
cmd.arg("build")
51-
.arg("--message-format=json")
84+
cmd.args(["build", "--target", &target, "--message-format=json"])
5285
.args(args)
5386
.stdout(Stdio::piped());
5487

@@ -133,12 +166,12 @@ impl SymInfo {
133166
/// Note that this will also locate cases where a symbol is weakly defined in more than one place.
134167
/// Technically there are no linker errors that will come from this, but it keeps our binary more
135168
/// straightforward and saves some distribution size.
136-
fn verify_no_duplicates(path: &Path) {
169+
fn verify_no_duplicates(archive: &Archive) {
137170
let mut syms = BTreeMap::<String, SymInfo>::new();
138171
let mut dups = Vec::new();
139172
let mut found_any = false;
140173

141-
for_each_symbol(path, |symbol, member| {
174+
archive.for_each_symbol(|symbol, member| {
142175
// Only check defined globals
143176
if !symbol.is_global() || symbol.is_undefined() {
144177
return;
@@ -185,12 +218,12 @@ fn verify_no_duplicates(path: &Path) {
185218
}
186219

187220
/// Ensure that there are no references to symbols from `core` that aren't also (somehow) defined.
188-
fn verify_core_symbols(path: &Path) {
221+
fn verify_core_symbols(archive: &Archive) {
189222
let mut defined = BTreeSet::new();
190223
let mut undefined = Vec::new();
191224
let mut has_symbols = false;
192225

193-
for_each_symbol(path, |symbol, member| {
226+
archive.for_each_symbol(|symbol, member| {
194227
has_symbols = true;
195228

196229
// Find only symbols from `core`
@@ -219,14 +252,101 @@ fn verify_core_symbols(path: &Path) {
219252
println!(" success: no undefined references to core found");
220253
}
221254

222-
/// For a given archive path, do something with each symbol.
223-
fn for_each_symbol(path: &Path, mut f: impl FnMut(Symbol, &ArchiveMember)) {
224-
let data = fs::read(path).expect("reading file failed");
225-
let archive = ArchiveFile::parse(data.as_slice()).expect("archive parse failed");
226-
for member in archive.members() {
227-
let member = member.expect("failed to access member");
228-
let obj_data = member.data(&*data).expect("failed to access object");
229-
let obj = object::File::parse(obj_data).expect("failed to parse object");
230-
obj.symbols().for_each(|sym| f(sym, &member));
255+
/// Check that all object files contain a section named `.note.GNU-stack`, indicating a
256+
/// nonexecutable stack.
257+
fn verify_no_exec_stack(archive: &Archive) {
258+
let mut problem_objfiles = Vec::new();
259+
260+
archive.for_each_object(|obj, member| {
261+
if obj_has_exe_stack(&obj) {
262+
problem_objfiles.push(String::from_utf8_lossy(member.name()).into_owned());
263+
}
264+
});
265+
266+
if !problem_objfiles.is_empty() {
267+
panic!(
268+
"the following archive members have executable sections but no \
269+
`.note.GNU-stack` section: {problem_objfiles:#?}"
270+
);
271+
}
272+
273+
println!(" success: no writeable-executable sections found");
274+
}
275+
276+
fn obj_has_exe_stack(obj: &ObjFile) -> bool {
277+
// Files other than elf likely do not use the same convention.
278+
if !matches!(obj, ObjFile::Elf32(_) | ObjFile::Elf64(_)) {
279+
return false;
231280
}
281+
282+
let mut has_exe_sections = false;
283+
for sec in obj.sections() {
284+
let SectionFlags::Elf { sh_flags } = sec.flags() else {
285+
unreachable!("only elf files are being checked");
286+
};
287+
288+
let exe = (sh_flags & SHF_EXECINSTR as u64) != 0;
289+
has_exe_sections |= exe;
290+
291+
// Located a GNU-stack section, nothing else to do
292+
if sec.name().unwrap_or_default() == ".note.GNU-stack" {
293+
return false;
294+
}
295+
}
296+
297+
// Ignore object files that have no executable sections, like rmeta
298+
if !has_exe_sections {
299+
return false;
300+
}
301+
302+
true
303+
}
304+
305+
/// Thin wrapper for owning data used by `object`.
306+
struct Archive {
307+
data: Vec<u8>,
308+
}
309+
310+
impl Archive {
311+
fn from_path(path: &Path) -> Self {
312+
Self {
313+
data: fs::read(path).expect("reading file failed"),
314+
}
315+
}
316+
317+
fn file(&self) -> ArchiveFile {
318+
ArchiveFile::parse(self.data.as_slice()).expect("archive parse failed")
319+
}
320+
321+
/// For a given archive, do something with each object file.
322+
fn for_each_object(&self, mut f: impl FnMut(ObjFile, &ArchiveMember)) {
323+
let archive = self.file();
324+
325+
for member in archive.members() {
326+
let member = member.expect("failed to access member");
327+
let obj_data = member
328+
.data(self.data.as_slice())
329+
.expect("failed to access object");
330+
let obj = ObjFile::parse(obj_data).expect("failed to parse object");
331+
f(obj, &member);
332+
}
333+
}
334+
335+
/// For a given archive, do something with each symbol.
336+
fn for_each_symbol(&self, mut f: impl FnMut(Symbol, &ArchiveMember)) {
337+
self.for_each_object(|obj, member| {
338+
obj.symbols().for_each(|sym| f(sym, member));
339+
});
340+
}
341+
}
342+
343+
#[test]
344+
fn check_obj() {
345+
let Some(p) = option_env!("HAS_WX_OBJ") else {
346+
return;
347+
};
348+
349+
let f = fs::read(p).unwrap();
350+
let obj = ObjFile::parse(f.as_slice()).unwrap();
351+
assert!(obj_has_exe_stack(&obj));
232352
}

0 commit comments

Comments
 (0)