Validate and configure tap interfaces from --tap_fd

Checks for the IFF_NO_PI and IFF_VNET_HDR flags, failing if those are
not set.
Sets the offload and vnet header sizes to the required values, instead
of trusting the values on the interface.

Bug=b/128686192

Change-Id: Ibbbfbf3cdedd6e64cdcfb446bcdfb26b4fd38395
Reviewed-on: https://chromium-review.googlesource.com/1526771
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Stephen Barber <smbarber@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

2 files changed, 74 insertions, 11 deletions

diff --git a/devices/src/virtio/net.rs b/devices/src/virtio/net.rs
index 088c2b8..ab37cf0 100644
--- a/devices/src/virtio/net.rs
+++ b/devices/src/virtio/net.rs
@@ -50,6 +50,8 @@ pub enum NetError {
     TapSetVnetHdrSize(TapError),
     /// Enabling tap interface failed.
     TapEnable(TapError),
+    /// Validating tap interface failed.
+    TapValidate(String),
     /// Error while polling for events.
     PollError(SysError),
 }
@@ -69,6 +71,7 @@ impl Display for NetError {
             TapSetOffload(e) => write!(f, "failed to set tap interface offload flags: {}", e),
             TapSetVnetHdrSize(e) => write!(f, "failed to set vnet header size: {}", e),
             TapEnable(e) => write!(f, "failed to enable tap interface: {}", e),
+            TapValidate(s) => write!(f, "failed to validate tap interface: {}", s),
             PollError(e) => write!(f, "error while polling for events: {}", e),
         }
     }
@@ -338,17 +341,6 @@ where
         tap.set_mac_address(mac_addr)
             .map_err(NetError::TapSetMacAddress)?;
 
-        // 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)
-            .map_err(NetError::TapSetVnetHdrSize)?;
-
         tap.enable().map_err(NetError::TapEnable)?;
 
         Net::from(tap)
@@ -357,6 +349,11 @@ where
     /// Creates a new virtio network device from a tap device that has already been
     /// configured.
     pub fn from(tap: T) -> Result<Net<T>, NetError> {
+        // This would also validate a tap created by Self::new(), but that's a good thing as it
+        // would ensure that any changes in the creation procedure are matched in the validation.
+        // Plus we still need to set the offload and vnet_hdr_size values.
+        validate_and_configure_tap(&tap)?;
+
         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
@@ -376,6 +373,48 @@ where
     }
 }
 
+// Ensure that the tap interface has the correct flags and sets the offload and VNET header size
+// to the appropriate values.
+fn validate_and_configure_tap<T: TapT>(tap: &T) -> Result<(), NetError> {
+    let flags = tap.if_flags();
+    let required_flags = [
+        (net_sys::IFF_TAP, "IFF_TAP"),
+        (net_sys::IFF_NO_PI, "IFF_NO_PI"),
+        (net_sys::IFF_VNET_HDR, "IFF_VNET_HDR"),
+    ];
+    let missing_flags = required_flags
+        .iter()
+        .filter_map(
+            |(value, name)| {
+                if value & flags == 0 {
+                    Some(name)
+                } else {
+                    None
+                }
+            },
+        )
+        .collect::<Vec<_>>();
+
+    if !missing_flags.is_empty() {
+        return Err(NetError::TapValidate(format!(
+            "Missing flags: {:?}",
+            missing_flags
+        )));
+    }
+
+    // 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)?;
+
+    let vnet_hdr_size = mem::size_of::<virtio_net_hdr_v1>() as i32;
+    tap.set_vnet_hdr_size(vnet_hdr_size)
+        .map_err(NetError::TapSetVnetHdrSize)?;
+
+    Ok(())
+}
+
 impl<T> Drop for Net<T>
 where
     T: TapT,
diff --git a/net_util/src/lib.rs b/net_util/src/lib.rs
index 066b7d8..456608c 100644
--- a/net_util/src/lib.rs
+++ b/net_util/src/lib.rs
@@ -170,6 +170,7 @@ impl Display for MacAddress {
 pub struct Tap {
     tap_file: File,
     if_name: [u8; 16usize],
+    if_flags: ::std::os::raw::c_short,
 }
 
 impl Tap {
@@ -187,6 +188,7 @@ impl Tap {
         Ok(Tap {
             tap_file,
             if_name: ifreq.ifr_ifrn.ifrn_name.as_ref().clone(),
+            if_flags: ifreq.ifr_ifru.ifru_flags.as_ref().clone(),
         })
     }
 }
@@ -225,6 +227,9 @@ pub trait TapT: Read + Write + AsRawFd + Send + Sized {
     fn set_vnet_hdr_size(&self, size: c_int) -> Result<()>;
 
     fn get_ifreq(&self) -> net_sys::ifreq;
+
+    /// Get the interface flags
+    fn if_flags(&self) -> u32;
 }
 
 impl TapT for Tap {
@@ -278,6 +283,7 @@ impl TapT for Tap {
         Ok(Tap {
             tap_file: tuntap,
             if_name: unsafe { *ifreq.ifr_ifrn.ifrn_name.as_ref() },
+            if_flags: unsafe { *ifreq.ifr_ifru.ifru_flags.as_ref() },
         })
     }
 
@@ -465,8 +471,22 @@ impl TapT for Tap {
             ifrn_name.clone_from_slice(&self.if_name);
         }
 
+        // This sets the flags with which the interface was created, which is the only entry we set
+        // on the second union.
+        unsafe {
+            ifreq
+                .ifr_ifru
+                .ifru_flags
+                .as_mut()
+                .clone_from(&self.if_flags);
+        }
+
         ifreq
     }
+
+    fn if_flags(&self) -> u32 {
+        self.if_flags as u32
+    }
 }
 
 impl Read for Tap {
@@ -554,6 +574,10 @@ pub mod fakes {
             let ifreq: net_sys::ifreq = Default::default();
             ifreq
         }
+
+        fn if_flags(&self) -> u32 {
+            net_sys::IFF_TAP
+        }
     }
 
     impl Drop for FakeTap {