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.

Sign up for GitHub

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

Jump to bottom

Return error for current_exe on nonexistent file #69557

Closed
wants to merge 2 commits into from
Closed

Return error for current_exe on nonexistent file #69557

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52ef2af
Return error for current_exe on nonexistent file
tsurai Feb 28, 2020
f8b7ca7
Apply tidy formatting
tsurai Feb 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/libstd/sys/unix/os.rs
View file
Open in desktop
Expand Up @@ -328,6 +328,22 @@ pub fn current_exe() -> io::Result<PathBuf> {
#[cfg(any(target_os = "linux", target_os = "android", target_os = "emscripten"))]
pub fn current_exe() -> io::Result<PathBuf> {
match crate::fs::read_link("/proc/self/exe") {
Ok(path) => {
let path_str = path.to_str().ok_or(io::Error::new(
io::ErrorKind::InvalidData,
"path contains invalid UTF-8 data",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems likely to be incorrect... we shouldn't need to require a path to be UTF-8; that's the entire reason we have Path(Buf) and OsStr(ing).

))?;
unsafe {
if libc::access(path_str.as_ptr() as *const i8, libc::F_OK) == 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this FFI call require a NUL-terminated string? Rust strings are not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either way lstat is the better way to test if the path indicated exists or not; access checks if the current user can also read the path. Though this gets weird if you do create a file with the (deleted) suffix on in; this will detect it as existing, but the link is actually broken (/proc is, probably not surprisingly, magic here). I think stat("/proc/self/exe") may actually be a better solution for checking if the path exists.

I don't know; there's no good fool-proof way to do this; we're just trying to dodge the throwing knives someone is sending our way at this point.

Copy link
Member

@the8472 the8472 Mar 29, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@clarfon outlined a possible approach, but that takes at least 4 syscalls. #69343 (comment)

I'm not sure if that's worth it because under race conditions paths must be considered stale the very moment you get them from a syscall. The only race-free way to do these things is to operate on file descriptors instead of paths.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, although I will say that the number of syscalls for this seems incredibly reasonable for an API that will be called extremely infrequently (likely seconds between invocations).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that once the symlink is broken due to the target being deleted, it is always broken, so it's at least a one-way gate here. But that cannot rely on readlink followed by use of the path; it must be gotten from stat("/proc/self/exe") directly.

Ok(path)
} else {
Err(io::Error::new(
io::ErrorKind::NotFound,
"executable file does not exist anymore",
))
}
}
}
Err(ref e) if e.kind() == io::ErrorKind::NotFound => Err(io::Error::new(
io::ErrorKind::Other,
"no /proc/self/exe available. Is /proc mounted?",
Expand Down