From 96e26c2681ee73b6a1e1d2d99aa53a1d21fa494b Mon Sep 17 00:00:00 2001 From: "Jorge E. Moreira" Date: Fri, 15 Mar 2019 18:07:01 -0700 Subject: 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 Tested-by: kokoro Reviewed-by: Stephen Barber Reviewed-by: Chirantan Ekbote --- devices/src/virtio/net.rs | 61 ++++++++++++++++++++++++++++++++++++++--------- net_util/src/lib.rs | 24 +++++++++++++++++++ 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::() 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, 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(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::>(); + + 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::() as i32; + tap.set_vnet_hdr_size(vnet_hdr_size) + .map_err(NetError::TapSetVnetHdrSize)?; + + Ok(()) +} + impl Drop for Net 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 { -- cgit 1.4.1