Skip to content

Commit

Permalink
Correctly handle spaces in /proc/pid/maps pathnames (#553)
Browse files Browse the repository at this point in the history
Prior to this change, pathnames were parsed from `/proc/pid/maps` using `s.split(' ')`. This is incorrect, because pathnames may contain spaces.

In practice, on the relevant operating systems, this resulted in truncation of pathnames containing spaces. This lead to a failure resolve any symbols or offsets, which in turn causes the stack trace to be printed containing only `<unknown>`s to the end user.

This PR corrects the parsing code, resolving the issue.
  • Loading branch information
workingjubilee committed Aug 18, 2023
2 parents 40410b7 + aa3186a commit d008d39
Showing 1 changed file with 72 additions and 9 deletions.
81 changes: 72 additions & 9 deletions src/symbolize/gimli/parse_running_mmaps_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,37 @@ impl FromStr for MapsEntry {
// e.g.: "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsyscall]"
// e.g.: "7f5985f46000-7f5985f48000 rw-p 00039000 103:06 76021795 /usr/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2"
// e.g.: "35b1a21000-35b1a22000 rw-p 00000000 00:00 0"
//
// Note that paths may contain spaces, so we can't use `str::split` for parsing (until
// Split::remainder is stabilized #77998).
fn from_str(s: &str) -> Result<Self, Self::Err> {
let mut parts = s
.split(' ') // space-separated fields
.filter(|s| s.len() > 0); // multiple spaces implies empty strings that need to be skipped.
let range_str = parts.next().ok_or("Couldn't find address")?;
let perms_str = parts.next().ok_or("Couldn't find permissions")?;
let offset_str = parts.next().ok_or("Couldn't find offset")?;
let dev_str = parts.next().ok_or("Couldn't find dev")?;
let inode_str = parts.next().ok_or("Couldn't find inode")?;
let pathname_str = parts.next().unwrap_or(""); // pathname may be omitted.
let (range_str, s) = s.trim_start().split_once(' ').unwrap_or((s, ""));
if range_str.is_empty() {
return Err("Couldn't find address");
}

let (perms_str, s) = s.trim_start().split_once(' ').unwrap_or((s, ""));
if perms_str.is_empty() {
return Err("Couldn't find permissions");
}

let (offset_str, s) = s.trim_start().split_once(' ').unwrap_or((s, ""));
if offset_str.is_empty() {
return Err("Couldn't find offset");
}

let (dev_str, s) = s.trim_start().split_once(' ').unwrap_or((s, ""));
if dev_str.is_empty() {
return Err("Couldn't find dev");
}

let (inode_str, s) = s.trim_start().split_once(' ').unwrap_or((s, ""));
if inode_str.is_empty() {
return Err("Couldn't find inode");
}

// Pathname may be omitted in which case it will be empty
let pathname_str = s.trim_start();

let hex = |s| usize::from_str_radix(s, 16).map_err(|_| "Couldn't parse hex number");
let address = if let Some((start, limit)) = range_str.split_once('-') {
Expand Down Expand Up @@ -229,4 +250,46 @@ fn check_maps_entry_parsing_32bit() {
pathname: Default::default(),
}
);
assert_eq!(
"b7c79000-b7e02000 r--p 00000000 08:01 60662705 \
/executable/path/with some spaces"
.parse::<MapsEntry>()
.unwrap(),
MapsEntry {
address: (0xb7c79000, 0xb7e02000),
perms: ['r', '-', '-', 'p'],
offset: 0x00000000,
dev: (0x08, 0x01),
inode: 0x60662705,
pathname: "/executable/path/with some spaces".into(),
}
);
assert_eq!(
"b7c79000-b7e02000 r--p 00000000 08:01 60662705 \
/executable/path/with multiple-continuous spaces "
.parse::<MapsEntry>()
.unwrap(),
MapsEntry {
address: (0xb7c79000, 0xb7e02000),
perms: ['r', '-', '-', 'p'],
offset: 0x00000000,
dev: (0x08, 0x01),
inode: 0x60662705,
pathname: "/executable/path/with multiple-continuous spaces ".into(),
}
);
assert_eq!(
" b7c79000-b7e02000 r--p 00000000 08:01 60662705 \
/executable/path/starts-with-spaces"
.parse::<MapsEntry>()
.unwrap(),
MapsEntry {
address: (0xb7c79000, 0xb7e02000),
perms: ['r', '-', '-', 'p'],
offset: 0x00000000,
dev: (0x08, 0x01),
inode: 0x60662705,
pathname: "/executable/path/starts-with-spaces".into(),
}
);
}

0 comments on commit d008d39

Please sign in to comment.