Skip to content

Commit 3858946

Browse files
Revise dl_iterate_phdr callback to be sound-ish
We - no longer pretend the entire Unixverse is GNU/Linux - no longer pretend static linking does not exist - no longer dereference pointers without checking them, except for the info object itself, where it's libc's responsibility to be sound This passed CI on locally running rustc's arm-android Docker image.
1 parent fc37b22 commit 3858946

File tree

2 files changed

+41
-19
lines changed

2 files changed

+41
-19
lines changed

Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/symbolize/gimli/libs_dl_iterate_phdr.rs

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,31 +35,53 @@ fn infer_current_exe(base_addr: usize) -> OsString {
3535
env::current_exe().map(|e| e.into()).unwrap_or_default()
3636
}
3737

38-
// `info` should be a valid pointers.
39-
// `vec` should be a valid pointer to a `std::Vec`.
38+
/// # Safety
39+
/// `info` must be a valid pointer.
40+
/// `vec` must be a valid pointer to `Vec<Library>`
41+
#[forbid(unsafe_op_in_unsafe_fn)]
4042
unsafe extern "C" fn callback(
4143
info: *mut libc::dl_phdr_info,
4244
_size: libc::size_t,
4345
vec: *mut libc::c_void,
4446
) -> libc::c_int {
45-
let info = &*info;
46-
let libs = &mut *vec.cast::<Vec<Library>>();
47-
let is_main_prog = info.dlpi_name.is_null() || *info.dlpi_name == 0;
48-
let name = if is_main_prog {
49-
// The man page for dl_iterate_phdr says that the first object visited by
50-
// callback is the main program; so the first time we encounter a
51-
// nameless entry, we can assume its the main program and try to infer its path.
52-
// After that, we cannot continue that assumption, and we use an empty string.
53-
if libs.is_empty() {
54-
infer_current_exe(info.dlpi_addr as usize)
55-
} else {
47+
// SAFETY: We are guaranteed these fields:
48+
let libc::dl_phdr_info {
49+
dlpi_addr,
50+
dlpi_name,
51+
dlpi_phdr,
52+
dlpi_phnum,
53+
..
54+
} = unsafe { *info };
55+
// SAFETY: We assured this.
56+
let libs = unsafe { &mut *vec.cast::<Vec<Library>>() };
57+
// most implementations give us the main program first
58+
let is_main = libs.is_empty();
59+
// we may be statically linked, which means we are main and mostly one big blob of code
60+
let is_static = dlpi_addr == 0;
61+
// sometimes we get a null or 0-len CStr, based on libc's whims, but these mean the same thing
62+
let no_given_name = dlpi_name.is_null()
63+
// SAFETY: we just checked for null
64+
|| unsafe { *dlpi_name == 0 };
65+
let name = if is_static {
66+
// don't try to look up our name from /proc/self/maps, it'll get silly
67+
env::current_exe().unwrap_or_default().into_os_string()
68+
} else if is_main && no_given_name {
69+
infer_current_exe(dlpi_addr as usize)
70+
} else {
71+
// this fallback works even if we are main, because some platforms give the name anyways
72+
if dlpi_name.is_null() {
5673
OsString::new()
74+
} else {
75+
// SAFETY: we just checked for nullness
76+
OsStr::from_bytes(unsafe { CStr::from_ptr(dlpi_name) }.to_bytes()).to_owned()
5777
}
78+
};
79+
let headers = if dlpi_phdr.is_null() || dlpi_phnum == 0 {
80+
&[]
5881
} else {
59-
let bytes = CStr::from_ptr(info.dlpi_name).to_bytes();
60-
OsStr::from_bytes(bytes).to_owned()
82+
// SAFETY: We just checked for nullness or 0-len slices
83+
unsafe { slice::from_raw_parts(dlpi_phdr, dlpi_phnum as usize) }
6184
};
62-
let headers = slice::from_raw_parts(info.dlpi_phdr, info.dlpi_phnum as usize);
6385
libs.push(Library {
6486
name,
6587
segments: headers
@@ -69,7 +91,7 @@ unsafe extern "C" fn callback(
6991
stated_virtual_memory_address: (*header).p_vaddr as usize,
7092
})
7193
.collect(),
72-
bias: info.dlpi_addr as usize,
94+
bias: dlpi_addr as usize,
7395
});
7496
0
7597
}

0 commit comments

Comments
 (0)