From f651357433b8a9824796f545f63edbefa905b23f Mon Sep 17 00:00:00 2001 From: Zach Reizner Date: Wed, 31 May 2017 09:47:23 -0700 Subject: [PATCH] crosvm: use sys_util::clone_process to create proxy device The `clone_process` function was created to safely encapsulate fork/clone usage for the proxy device. This patch changes proxy device to do utilize that. TEST=cargo run -- -u ... BUG=None Change-Id: I2d9f1794be61be31f3aae21037c7df14b7691172 Reviewed-on: https://chromium-review.googlesource.com/518935 Commit-Ready: Stephen Barber Tested-by: Zach Reizner Reviewed-by: Dylan Reid --- Cargo.toml | 1 - src/hw/proxy.rs | 46 +++++++++++++--------------------------------- src/main.rs | 1 - 3 files changed, 13 insertions(+), 35 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 137ccbdd7b..b24346d842 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,6 @@ x86_64 = { path = "x86_64" } kernel_loader = { path = "kernel_loader" } libc = "0.2.21" byteorder = "1" -syscall_defines = { path = "syscall_defines" } [dependencies.clap] version = "*" diff --git a/src/hw/proxy.rs b/src/hw/proxy.rs index efd9734715..001a86bcee 100644 --- a/src/hw/proxy.rs +++ b/src/hw/proxy.rs @@ -4,18 +4,14 @@ //! Runs hardware devices in child processes. -use std::process; -use std::io::{Error, Result}; +use std::io::{Error, ErrorKind, Result}; use std::os::unix::net::UnixDatagram; use std::time::Duration; -use libc; -use libc::pid_t; - use byteorder::{NativeEndian, ByteOrder}; use hw::BusDevice; -use syscall_defines::linux::LinuxSyscall::SYS_clone; +use sys_util::{clone_process, CloneNamespace}; const SOCKET_TIMEOUT_MS: u64 = 2000; const MSG_SIZE: usize = 24; @@ -27,7 +23,7 @@ enum Command { Shutdown = 2, } -fn child_proc(sock: UnixDatagram, device: &mut BusDevice) -> ! { +fn child_proc(sock: UnixDatagram, device: &mut BusDevice) { let mut running = true; let res = handle_eintr!(sock.send(&CHILD_SIGNATURE)); @@ -75,23 +71,6 @@ fn child_proc(sock: UnixDatagram, device: &mut BusDevice) -> ! { break; } } - - // ! Never returns - process::exit(0); -} - -unsafe fn do_clone() -> Result { - // Forking is unsafe, this function must be unsafe as there is no way to - // guarantee saftey without more context about the state of the program. - let pid = libc::syscall(SYS_clone as i64, - libc::CLONE_NEWUSER | libc::CLONE_NEWPID | - libc::SIGCHLD as i32, - 0); - if pid < 0 { - Err(Error::last_os_error()) - } else { - Ok(pid as pid_t) - } } /// Wraps an inner `hw::BusDevice` that is run inside a child process via fork. @@ -107,20 +86,21 @@ impl ProxyDevice { /// /// The forked process will automatically be terminated when this is dropped, so be sure to keep /// a reference. - /// `post_clone_cb` - Called after forking the child process, passed the - /// child end of the pipe that must be kep open. + /// + /// # Arguments + /// * `device` - The device to isolate to another process. + /// * `post_clone_cb` - Called after forking the child process, passed the child end of the pipe + /// that must be kept open. pub fn new(mut device: D, post_clone_cb: F) -> Result - where F: FnOnce(&UnixDatagram) { + where F: FnOnce(&UnixDatagram) + { let (child_sock, parent_sock) = UnixDatagram::pair()?; - // Forking a new process is unsafe, we must ensure no resources required - // by the other side are freed after the two processes start. - let ret = unsafe { do_clone()? }; - if ret == 0 { + clone_process(CloneNamespace::NewUserPid, move || { post_clone_cb(&child_sock); - // ! Never returns child_proc(child_sock, &mut device); - } + }) + .map_err(|e| Error::new(ErrorKind::Other, format!("{:?}", e)))?; let mut buf = [0; MSG_SIZE]; parent_sock diff --git a/src/main.rs b/src/main.rs index aea8808298..1e76072796 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,7 +12,6 @@ extern crate x86_64; extern crate kernel_loader; extern crate byteorder; #[macro_use] extern crate sys_util; -extern crate syscall_defines; use std::ffi::{CString, CStr}; use std::fmt;