devices: don't jail in ProxyDevice constructor

Jail functionality has been moved into a new JailedDevice struct,
which wraps ProxyDevice.

Doing this allows ProxyDevice to be much more generic and reusable.

1 files changed, 5 insertions, 141 deletions

diff --git a/devices/src/proxy.rs b/devices/src/proxy.rs
index cb7ebd6..cc31550 100644
--- a/devices/src/proxy.rs
+++ b/devices/src/proxy.rs
@@ -4,41 +4,13 @@
 
 //! Runs hardware devices in child processes.
 
-use std::fmt::{self, Display};
-use std::os::unix::io::{AsRawFd, RawFd};
-use std::time::Duration;
-use std::{self, io};
-
-use io_jail::{self, Minijail};
-use libc::{self, pid_t};
 use msg_socket::{MsgOnSocket, MsgReceiver, MsgSender, MsgSocket};
-use sys_util::{error, net::UnixSeqpacket};
+use sys_util::error;
 
 use crate::BusDevice;
 
-/// Errors for proxy devices.
-#[derive(Debug)]
-pub enum Error {
-    ForkingJail(io_jail::Error),
-    Io(io::Error),
-}
-pub type Result<T> = std::result::Result<T, Error>;
-
-impl Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        use self::Error::*;
-
-        match self {
-            ForkingJail(e) => write!(f, "Failed to fork jail process: {}", e),
-            Io(e) => write!(f, "IO error configuring proxy device {}.", e),
-        }
-    }
-}
-
-const SOCKET_TIMEOUT_MS: u64 = 2000;
-
 #[derive(Debug, MsgOnSocket)]
-enum Command {
+pub enum Command {
     Read {
         len: u32,
         offset: u64,
@@ -59,128 +31,20 @@ enum Command {
 }
 
 #[derive(MsgOnSocket)]
-enum CommandResult {
+pub enum CommandResult {
     Ok,
     ReadResult([u8; 8]),
     ReadConfigResult(u32),
 }
 
-fn child_proc<D: BusDevice>(sock: UnixSeqpacket, device: &mut D) {
-    let mut running = true;
-    let sock = MsgSocket::<CommandResult, Command>::new(sock);
-
-    while running {
-        let cmd = match sock.recv() {
-            Ok(cmd) => cmd,
-            Err(err) => {
-                error!("child device process failed recv: {}", err);
-                break;
-            }
-        };
-
-        let res = match cmd {
-            Command::Read { len, offset } => {
-                let mut buffer = [0u8; 8];
-                device.read(offset, &mut buffer[0..len as usize]);
-                sock.send(&CommandResult::ReadResult(buffer))
-            }
-            Command::Write { len, offset, data } => {
-                let len = len as usize;
-                device.write(offset, &data[0..len]);
-                // Command::Write does not have a result.
-                Ok(())
-            }
-            Command::ReadConfig(idx) => {
-                let val = device.config_register_read(idx as usize);
-                sock.send(&CommandResult::ReadConfigResult(val))
-            }
-            Command::WriteConfig {
-                reg_idx,
-                offset,
-                len,
-                data,
-            } => {
-                let len = len as usize;
-                device.config_register_write(reg_idx as usize, offset as u64, &data[0..len]);
-                // Command::WriteConfig does not have a result.
-                Ok(())
-            }
-            Command::Shutdown => {
-                running = false;
-                sock.send(&CommandResult::Ok)
-            }
-        };
-        if let Err(e) = res {
-            error!("child device process failed send: {}", e);
-        }
-    }
-}
-
-/// Wraps an inner `BusDevice` that is run inside a child process via fork.
-///
-/// Because forks are very unfriendly to destructors and all memory mappings and file descriptors
-/// are inherited, this should be used as early as possible in the main process.
 pub struct ProxyDevice {
     sock: MsgSocket<Command, CommandResult>,
-    pid: pid_t,
     debug_label: String,
 }
 
 impl ProxyDevice {
-    /// Takes the given device and isolates it into another process via fork before returning.
-    ///
-    /// The forked process will automatically be terminated when this is dropped, so be sure to keep
-    /// a reference.
-    ///
-    /// # Arguments
-    /// * `device` - The device to isolate to another process.
-    /// * `jail` - The jail to use for isolating the given device.
-    /// * `keep_fds` - File descriptors that will be kept open in the child.
-    pub fn new<D: BusDevice>(
-        mut device: D,
-        jail: &Minijail,
-        mut keep_fds: Vec<RawFd>,
-    ) -> Result<ProxyDevice> {
-        let debug_label = device.debug_label();
-        let (child_sock, parent_sock) = UnixSeqpacket::pair().map_err(Error::Io)?;
-
-        keep_fds.push(child_sock.as_raw_fd());
-        // Forking here is safe as long as the program is still single threaded.
-        let pid = unsafe {
-            match jail.fork(Some(&keep_fds)).map_err(Error::ForkingJail)? {
-                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
-                    libc::exit(0);
-                }
-                p => p,
-            }
-        };
-
-        parent_sock
-            .set_write_timeout(Some(Duration::from_millis(SOCKET_TIMEOUT_MS)))
-            .map_err(Error::Io)?;
-        parent_sock
-            .set_read_timeout(Some(Duration::from_millis(SOCKET_TIMEOUT_MS)))
-            .map_err(Error::Io)?;
-        Ok(ProxyDevice {
-            sock: MsgSocket::<Command, CommandResult>::new(parent_sock),
-            pid,
-            debug_label,
-        })
-    }
-
-    pub fn pid(&self) -> pid_t {
-        self.pid
+    pub fn new(sock: MsgSocket<Command, CommandResult>, debug_label: String) -> Self {
+        Self { sock, debug_label }
     }
 
     /// Send a command that does not expect a response from the child device process.