From eff26bbeb347c9cfa0d310fddbd804bd4af9824a Mon Sep 17 00:00:00 2001 From: Stephen Barber Date: Fri, 9 Aug 2019 11:09:47 -0700 Subject: [PATCH] devices: use libc::exit instead of process::exit We don't always shut down the worker threads cleanly, which can lead to a race when crosvm is exiting. Worker threads that attempt logging to stderr may fail an expect(), panic, and then panic again trying to write to stderr causing SIGILL. Work around this issue for now by using libc's exit, which won't run any rust-specific cleanup. BUG=chromium:978319,chromium:992494 TEST=crosvm shuts down without SIGILL/core dumps Change-Id: I8a99ce8a34220afdf503402d44721a9bea5ec460 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1746830 Tested-by: kokoro Tested-by: Stephen Barber Reviewed-by: Daniel Verkamp Commit-Queue: Daniel Verkamp --- devices/src/proxy.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/devices/src/proxy.rs b/devices/src/proxy.rs index dc64212e04..b97cccad43 100644 --- a/devices/src/proxy.rs +++ b/devices/src/proxy.rs @@ -6,12 +6,11 @@ use std::fmt::{self, Display}; use std::os::unix::io::{AsRawFd, RawFd}; -use std::process; use std::time::Duration; use std::{self, io}; use io_jail::{self, Minijail}; -use libc::pid_t; +use libc::{self, pid_t}; use msg_socket::{MsgOnSocket, MsgReceiver, MsgSender, MsgSocket}; use sys_util::{error, net::UnixSeqpacket}; @@ -149,8 +148,16 @@ impl ProxyDevice { 0 => { device.on_sandboxed(); child_proc(child_sock, &mut device); + + // We're explicitly not using std::process::exit here to avoid the cleanup of + // stdout/stderr globals. This can cause cascading panics and SIGILL if a worker + // thread attempts to log to stderr after at_exit handlers have been run. + // TODO(crbug.com/992494): Remove this once device shutdown ordering is clearly + // defined. + // + // exit() is trivially safe. // ! Never returns - process::exit(0); + libc::exit(0); } p => p, }