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.

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

Reopen standard file descriptors when they are missing on Unix #75295

Merged
merged 1 commit into from
Sep 28, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Reopen standard streams when they are closed on Unix
The syscalls returning a new file descriptors generally use
lowest-numbered file descriptor not currently opened, without any
exceptions for those corresponding to the standard streams.

Previously when any of standard streams has been closed before starting
the application, operations on std::io::{stderr,stdin,stdout} objects
were likely to operate on other logically unrelated file resources
opened afterwards.

Avoid the issue by reopening the standard streams when they are closed.
tmiasko committed Sep 27, 2020
commit 7d98d2207a20d95e731bfc742d89389a67d6618d
62 changes: 62 additions & 0 deletions library/std/src/sys/unix/mod.rs
Original file line number Diff line number Diff line change
@@ -75,6 +75,13 @@ pub use crate::sys_common::os_str_bytes as os_str;

#[cfg(not(test))]
pub fn init() {
// The standard streams might be closed on application startup. To prevent
// std::io::{stdin, stdout,stderr} objects from using other unrelated file
// resources opened later, we reopen standards streams when they are closed.
unsafe {
sanitize_standard_fds();
}

// By default, some platforms will send a *signal* when an EPIPE error
// would otherwise be delivered. This runtime doesn't install a SIGPIPE
// handler, causing it to kill the program, which isn't exactly what we
@@ -86,6 +93,61 @@ pub fn init() {
reset_sigpipe();
}

// In the case when all file descriptors are open, the poll has been
// observed to perform better than fcntl (on GNU/Linux).
#[cfg(not(any(
miri,
target_os = "emscripten",
target_os = "fuchsia",
// The poll on Darwin doesn't set POLLNVAL for closed fds.
target_os = "macos",
target_os = "ios",
target_os = "redox",
)))]
unsafe fn sanitize_standard_fds() {
use crate::sys::os::errno;
let pfds: &mut [_] = &mut [
libc::pollfd { fd: 0, events: 0, revents: 0 },
libc::pollfd { fd: 1, events: 0, revents: 0 },
libc::pollfd { fd: 2, events: 0, revents: 0 },
];
while libc::poll(pfds.as_mut_ptr(), 3, 0) == -1 {
if errno() == libc::EINTR {
continue;
}
libc::abort();
}
for pfd in pfds {
if pfd.revents & libc::POLLNVAL == 0 {
continue;
}
if libc::open("/dev/null\0".as_ptr().cast(), libc::O_RDWR, 0) == -1 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if libc::open("/dev/null\0".as_ptr().cast(), libc::O_RDWR, 0) == -1 {
if libc::open(b"/dev/null\0".as_ptr().cast(), libc::O_RDWR, 0) == -1 {

// If the stream is closed but we failed to reopen it, abort the
// process. Otherwise we wouldn't preserve the safety of
// operations on the corresponding Rust object Stdin, Stdout, or
// Stderr.
libc::abort();
}
}
}
#[cfg(any(target_os = "macos", target_os = "ios", target_os = "redox"))]
unsafe fn sanitize_standard_fds() {
use crate::sys::os::errno;
for fd in 0..3 {
if libc::fcntl(fd, libc::F_GETFD) == -1 && errno() == libc::EBADF {
if libc::open("/dev/null\0".as_ptr().cast(), libc::O_RDWR, 0) == -1 {
libc::abort();
}
}
}
}
#[cfg(any(
// The standard fds are always available in Miri.
miri,
target_os = "emscripten",
target_os = "fuchsia"))]
unsafe fn sanitize_standard_fds() {}

#[cfg(not(any(target_os = "emscripten", target_os = "fuchsia")))]
unsafe fn reset_sigpipe() {
assert!(signal(libc::SIGPIPE, libc::SIG_IGN) != libc::SIG_ERR);
22 changes: 20 additions & 2 deletions src/test/ui/no-stdio.rs
Original file line number Diff line number Diff line change
@@ -36,6 +36,16 @@ unsafe fn without_stdio<R, F: FnOnce() -> R>(f: F) -> R {
return r
}

#[cfg(unix)]
fn assert_fd_is_valid(fd: libc::c_int) {
if unsafe { libc::fcntl(fd, libc::F_GETFD) == -1 } {
panic!("file descriptor {} is not valid: {}", fd, io::Error::last_os_error());
}
}

#[cfg(windows)]
fn assert_fd_is_valid(_fd: libc::c_int) {}

#[cfg(windows)]
unsafe fn without_stdio<R, F: FnOnce() -> R>(f: F) -> R {
type DWORD = u32;
@@ -77,10 +87,18 @@ unsafe fn without_stdio<R, F: FnOnce() -> R>(f: F) -> R {

fn main() {
if env::args().len() > 1 {
// Writing to stdout & stderr should not panic.
println!("test");
assert!(io::stdout().write(b"test\n").is_ok());
assert!(io::stderr().write(b"test\n").is_ok());

// Stdin should be at EOF.
assert_eq!(io::stdin().read(&mut [0; 10]).unwrap(), 0);

// Standard file descriptors should be valid on UNIX:
assert_fd_is_valid(0);
assert_fd_is_valid(1);
assert_fd_is_valid(2);
return
}

@@ -109,12 +127,12 @@ fn main() {
.stdout(Stdio::null())
.stderr(Stdio::null())
.status().unwrap();
assert!(status.success(), "{:?} isn't a success", status);
assert!(status.success(), "{} isn't a success", status);

// Finally, close everything then spawn a child to make sure everything is
// *still* ok.
let status = unsafe {
without_stdio(|| Command::new(&me).arg("next").status())
}.unwrap();
assert!(status.success(), "{:?} isn't a success", status);
assert!(status.success(), "{} isn't a success", status);
}