summary refs log tree commit diff
diff options
context:
space:
mode:
authorChirantan Ekbote <chirantan@chromium.org>2018-05-31 15:31:31 -0700
committerchrome-bot <chrome-bot@chromium.org>2018-06-12 00:36:27 -0700
commit5f787217cc48e526c6244cd39db4f944fadfcd88 (patch)
tree8d3160c83c19c3ef3356502a3b18603b1de05ec3
parent92068bca00d7499dc789527b90efdf3daed2e927 (diff)
downloadcrosvm-5f787217cc48e526c6244cd39db4f944fadfcd88.tar
crosvm-5f787217cc48e526c6244cd39db4f944fadfcd88.tar.gz
crosvm-5f787217cc48e526c6244cd39db4f944fadfcd88.tar.bz2
crosvm-5f787217cc48e526c6244cd39db4f944fadfcd88.tar.lz
crosvm-5f787217cc48e526c6244cd39db4f944fadfcd88.tar.xz
crosvm-5f787217cc48e526c6244cd39db4f944fadfcd88.tar.zst
crosvm-5f787217cc48e526c6244cd39db4f944fadfcd88.zip
net: Allow passing in a configured tap fd on the command line
Allow the process that spawned crosvm to pass in a configured tap file
descriptor for networking.  If this option is provided then crosvm will
ignore the other networking related command line flags (like mac
address, netmask, etc).

Passing in a configured tap device allows us to run crosvm without
having to give it CAP_NET_ADMIN.

BUG=none
TEST=Start a container and verify that networking still works

Change-Id: I70b9e6ae030d66c4882e4e48804dc2f29d9874ba
Signed-off-by: Chirantan Ekbote <chirantan@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1081394
Reviewed-by: Zach Reizner <zachr@chromium.org>
-rw-r--r--devices/src/virtio/net.rs42
-rw-r--r--net_util/src/lib.rs9
-rw-r--r--src/linux.rs60
-rw-r--r--src/main.rs21
4 files changed, 96 insertions, 36 deletions
diff --git a/devices/src/virtio/net.rs b/devices/src/virtio/net.rs
index 8360158..96f7199 100644
--- a/devices/src/virtio/net.rs
+++ b/devices/src/virtio/net.rs
@@ -295,22 +295,22 @@ where
 {
     /// Create a new virtio network device with the given IP address and
     /// netmask.
-    pub fn new(ip_addr: Ipv4Addr,
-               netmask: Ipv4Addr,
-               mac_addr: MacAddress) -> Result<Net<T>, NetError> {
-        let kill_evt = EventFd::new().map_err(NetError::CreateKillEventFd)?;
-
+    pub fn new(
+        ip_addr: Ipv4Addr,
+        netmask: Ipv4Addr,
+        mac_addr: MacAddress,
+    ) -> Result<Net<T>, NetError> {
         let tap: T = T::new(true).map_err(NetError::TapOpen)?;
         tap.set_ip_addr(ip_addr).map_err(NetError::TapSetIp)?;
-        tap.set_netmask(netmask)
-            .map_err(NetError::TapSetNetmask)?;
+        tap.set_netmask(netmask).map_err(NetError::TapSetNetmask)?;
         tap.set_mac_address(mac_addr)
             .map_err(NetError::TapSetMacAddress)?;
 
-        // Set offload flags to match the virtio features below.
-        tap.set_offload(net_sys::TUN_F_CSUM | net_sys::TUN_F_UFO | net_sys::TUN_F_TSO4 |
-                         net_sys::TUN_F_TSO6)
-            .map_err(NetError::TapSetOffload)?;
+        // Set offload flags to match the virtio features below.  If you make any
+        // changes to this set, also change the corresponding feature set in vm_concierge.
+        tap.set_offload(
+            net_sys::TUN_F_CSUM | net_sys::TUN_F_UFO | net_sys::TUN_F_TSO4 | net_sys::TUN_F_TSO6,
+        ).map_err(NetError::TapSetOffload)?;
 
         let vnet_hdr_size = mem::size_of::<virtio_net_hdr_v1>() as i32;
         tap.set_vnet_hdr_size(vnet_hdr_size)
@@ -318,13 +318,21 @@ where
 
         tap.enable().map_err(NetError::TapEnable)?;
 
-        let avail_features =
-            1 << virtio_net::VIRTIO_NET_F_GUEST_CSUM | 1 << virtio_net::VIRTIO_NET_F_CSUM |
-                1 << virtio_net::VIRTIO_NET_F_GUEST_TSO4 |
-                1 << virtio_net::VIRTIO_NET_F_GUEST_UFO |
-                1 << virtio_net::VIRTIO_NET_F_HOST_TSO4 |
-                1 << virtio_net::VIRTIO_NET_F_HOST_UFO | 1 << vhost::VIRTIO_F_VERSION_1;
+        Net::from(tap)
+    }
 
+    /// Creates a new virtio network device from a tap device that has already been
+    /// configured.
+    pub fn from(tap: T) -> Result<Net<T>, NetError> {
+        let avail_features = 1 << virtio_net::VIRTIO_NET_F_GUEST_CSUM
+            | 1 << virtio_net::VIRTIO_NET_F_CSUM
+            | 1 << virtio_net::VIRTIO_NET_F_GUEST_TSO4
+            | 1 << virtio_net::VIRTIO_NET_F_GUEST_UFO
+            | 1 << virtio_net::VIRTIO_NET_F_HOST_TSO4
+            | 1 << virtio_net::VIRTIO_NET_F_HOST_UFO
+            | 1 << vhost::VIRTIO_F_VERSION_1;
+
+        let kill_evt = EventFd::new().map_err(NetError::CreateKillEventFd)?;
         Ok(Net {
             workers_kill_evt: Some(kill_evt.try_clone().map_err(NetError::CloneKillEventFd)?),
             kill_evt: kill_evt,
diff --git a/net_util/src/lib.rs b/net_util/src/lib.rs
index 273c1ca..0826729 100644
--- a/net_util/src/lib.rs
+++ b/net_util/src/lib.rs
@@ -449,6 +449,15 @@ impl AsRawFd for Tap {
     }
 }
 
+impl FromRawFd for Tap {
+    unsafe fn from_raw_fd(fd: RawFd) -> Tap {
+        Tap {
+            tap_file: File::from_raw_fd(fd),
+            if_name: [0; 16usize],
+        }
+    }
+}
+
 pub mod fakes {
     use super::*;
     use std::fs::OpenOptions;
diff --git a/src/linux.rs b/src/linux.rs
index eb85659..8950dfc 100644
--- a/src/linux.rs
+++ b/src/linux.rs
@@ -191,6 +191,27 @@ impl Drop for UnlinkUnixDatagram {
     }
 }
 
+// Verifies that |raw_fd| is actually owned by this process and duplicates it to ensure that
+// we have a unique handle to it.
+fn validate_raw_fd(raw_fd: RawFd) -> Result<RawFd> {
+    // Checking that close-on-exec isn't set helps filter out FDs that were opened by
+    // crosvm as all crosvm FDs are close on exec.
+    // Safe because this doesn't modify any memory and we check the return value.
+    let flags = unsafe { libc::fcntl(raw_fd, libc::F_GETFD) };
+    if flags < 0 || (flags & libc::FD_CLOEXEC) != 0 {
+        return Err(Error::FailedCLOEXECCheck);
+    }
+
+    // Duplicate the fd to ensure that we don't accidentally close an fd previously
+    // opened by another subsystem.  Safe because this doesn't modify any memory and
+    // we check the return value.
+    let dup_fd = unsafe { libc::fcntl(raw_fd, libc::F_DUPFD_CLOEXEC, 0) };
+    if dup_fd < 0 {
+        return Err(Error::FailedToDupFd);
+    }
+    Ok(dup_fd as RawFd)
+}
+
 fn create_base_minijail(root: &Path, seccomp_policy: &Path) -> Result<Minijail> {
     // All child jails run in a new user namespace without any users mapped,
     // they run as nobody unless otherwise configured.
@@ -248,24 +269,8 @@ fn setup_mmio_bus(cfg: &Config,
                 .and_then(|fd_osstr| fd_osstr.to_str())
                 .and_then(|fd_str| fd_str.parse::<c_int>().ok())
                 .ok_or(Error::InvalidFdPath)?;
-            unsafe {
-                // The FD is valid and this process owns it because it exists in /proc/self/fd.
-                // Ensure |raw_image| is the only owner by first duping it then closing the
-                // original.
-                // Checking that close-on-exec isn't set helps filter out FDs that were opened by
-                // crosvm as all crosvm FDs are close on exec.
-                let flags = libc::fcntl(raw_fd, libc::F_GETFD);
-                if flags < 0 || (flags & libc::FD_CLOEXEC) != 0 {
-                    return Err(Error::FailedCLOEXECCheck);
-                }
-
-                let dup_fd = libc::fcntl(raw_fd, libc::F_DUPFD_CLOEXEC, 0) as RawFd;
-                if dup_fd < 0 {
-                    return Err(Error::FailedToDupFd);
-                }
-                libc::close(raw_fd);
-                File::from_raw_fd(dup_fd)
-            }
+            // Safe because we will validate |raw_fd|.
+            unsafe { File::from_raw_fd(validate_raw_fd(raw_fd)?) }
         } else {
             OpenOptions::new()
                 .read(true)
@@ -329,7 +334,24 @@ fn setup_mmio_bus(cfg: &Config,
         .map_err(Error::RegisterBalloon)?;
 
     // We checked above that if the IP is defined, then the netmask is, too.
-    if let Some(host_ip) = cfg.host_ip {
+    if let Some(tap_fd) = cfg.tap_fd {
+        // Safe because we ensure that we get a unique handle to the fd.
+        let tap = unsafe { Tap::from_raw_fd(validate_raw_fd(tap_fd)?) };
+        let net_box = Box::new(devices::virtio::Net::from(tap)
+            .map_err(|e| Error::NetDeviceNew(e))?);
+
+        let jail = if cfg.multiprocess {
+            let policy_path: PathBuf = cfg.seccomp_policy_dir.join("net_device.policy");
+
+            Some(create_base_minijail(empty_root_path, &policy_path)?)
+        } else {
+            None
+        };
+
+        device_manager
+            .register_mmio(net_box, jail, cmdline)
+            .map_err(Error::RegisterNet)?;
+    } else if let Some(host_ip) = cfg.host_ip {
         if let Some(netmask) = cfg.netmask {
             if let Some(mac_address) = cfg.mac_address {
                 let net_box: Box<devices::virtio::VirtioDevice> = if cfg.vhost_net {
diff --git a/src/main.rs b/src/main.rs
index 724fc7a..35f4cf0 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -38,6 +38,7 @@ pub mod linux;
 pub mod plugin;
 
 use std::net;
+use std::os::unix::io::RawFd;
 use std::os::unix::net::UnixDatagram;
 use std::path::PathBuf;
 use std::string::String;
@@ -72,6 +73,7 @@ pub struct Config {
     netmask: Option<net::Ipv4Addr>,
     mac_address: Option<net_util::MacAddress>,
     vhost_net: bool,
+    tap_fd: Option<RawFd>,
     wayland_socket_path: Option<PathBuf>,
     wayland_dmabuf: bool,
     socket_path: Option<PathBuf>,
@@ -94,6 +96,7 @@ impl Default for Config {
             netmask: None,
             mac_address: None,
             vhost_net: false,
+            tap_fd: None,
             wayland_socket_path: None,
             wayland_dmabuf: false,
             socket_path: None,
@@ -335,6 +338,17 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument::
         "vhost-net" => {
             cfg.vhost_net = true
         },
+        "tap-fd" => {
+            if cfg.tap_fd.is_some() {
+                return Err(argument::Error::TooManyArguments("`tap-fd` alread given".to_owned()));
+            }
+            cfg.tap_fd = Some(value.unwrap().parse().map_err(|_| {
+                argument::Error::InvalidValue {
+                    value: value.unwrap().to_owned(),
+                    expected: "this value for `tap-fd` must be an unsigned integer",
+                }
+            })?);
+        }
         "help" => return Err(argument::Error::PrintHelp),
         _ => unreachable!(),
     }
@@ -385,6 +399,9 @@ fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> {
           Argument::value("plugin", "PATH", "Absolute path to plugin process to run under crosvm."),
           Argument::value("plugin-root", "PATH", "Absolute path to a directory that will become root filesystem for the plugin process."),
           Argument::flag("vhost-net", "Use vhost for networking."),
+          Argument::value("tap-fd",
+                          "fd",
+                          "File descriptor for configured tap device.  Mutually exclusive with `host_ip`, `netmask`, and `mac`."),
           Argument::short_flag('h', "help", "Print help message.")];
 
     let mut cfg = Config::default();
@@ -406,6 +423,10 @@ fn run_vm(args: std::env::Args) -> std::result::Result<(), ()> {
         if cfg.plugin_root.is_some() && cfg.plugin.is_none() {
             return Err(argument::Error::ExpectedArgument("`plugin-root` requires `plugin`".to_owned()));
         }
+        if cfg.tap_fd.is_some() && (cfg.host_ip.is_some() || cfg.netmask.is_some() || cfg.mac_address.is_some()) {
+            return Err(argument::Error::TooManyArguments(
+                "`tap_fd` and any of `host_ip`, `netmask`, or `mac` are mutually exclusive".to_owned()));
+        }
         Ok(())
     });