netlink: clean all clippy warnings

Clean up all clippy warning.
Also fix a bug in dealing with IFLA_IFNAME attribute.
nlh.addattr_var(IFLA_IFNAME, name.as_ptr() as *const u8, name.len() + 1);
The `name` is a rust String, which doesn't including the trailing '\0',
so name.len() + 1 may cause invalid memory access.

Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

src/agent/netlink/src/agent_handler.rs

@@ -72,11 +72,7 @@ impl super::RtnlHandle {
 
             // if str is null terminated, use addattr_var.
             // otherwise, use addattr_str
-            nlh.addattr_var(
+            nlh.addattr_var(IFLA_IFNAME, iface.name.as_ref());
-                IFLA_IFNAME,
-                iface.name.as_ptr() as *const u8,
-                iface.name.len(),
-            );
         }
 
         self.rtnl_talk(v.as_mut_slice(), false)?;
@@ -192,10 +188,11 @@ impl super::RtnlHandle {
 
                     // fill address field of Interface
                     let mut one: IPAddress = IPAddress::default();
-                    let mut tattr: *const rtattr = addrs[IFA_LOCAL as usize];
+                    let tattr: *const rtattr = if !addrs[IFA_ADDRESS as usize].is_null() {
-                    if !addrs[IFA_ADDRESS as usize].is_null() {
+                        addrs[IFA_ADDRESS as usize]
-                        tattr = addrs[IFA_ADDRESS as usize];
+                    } else {
-                    }
+                        addrs[IFA_LOCAL as usize]
+                    };
 
                     one.mask = format!("{}", ifa.ifa_prefixlen);
                     one.family = IPFamily::v4;
@@ -221,7 +218,7 @@ impl super::RtnlHandle {
         Ok(ifaces)
     }
 
-    pub fn update_routes(&mut self, rt: &Vec<Route>) -> Result<Vec<Route>> {
+    pub fn update_routes(&mut self, rt: &[Route]) -> Result<Vec<Route>> {
         let rs = self.get_all_routes()?;
         self.delete_all_routes(&rs)?;
 
@@ -245,7 +242,7 @@ impl super::RtnlHandle {
             }
         }
 
-        Ok(rt.clone())
+        Ok(rt.to_owned())
     }
 
     pub fn list_routes(&mut self) -> Result<Vec<Route>> {
@@ -342,7 +339,7 @@ impl super::RtnlHandle {
 
                     rte.device = self
                         .get_name_by_index(*data)
-                        .unwrap_or("unknown".to_string());
+                        .unwrap_or_else(|_| "unknown".to_string());
                 }
             }
 
@@ -352,7 +349,7 @@ impl super::RtnlHandle {
         Ok(rs)
     }
 
-    pub fn add_arp_neighbors(&mut self, neighs: &Vec<ARPNeighbor>) -> Result<()> {
+    pub fn add_arp_neighbors(&mut self, neighs: &[ARPNeighbor]) -> Result<()> {
         for neigh in neighs {
             self.add_one_arp_neighbor(&neigh)?;
         }
@@ -390,13 +387,13 @@ impl super::RtnlHandle {
             let llabuf = parser::parse_mac_addr(&neigh.lladdr)?;
 
             // Safe because we have allocated enough buffer space.
-            unsafe { nlh.addattr_var(NDA_LLADDR, llabuf.as_ptr() as *const u8, llabuf.len()) };
+            unsafe { nlh.addattr_var(NDA_LLADDR, llabuf.as_ref()) };
         }
 
         let (family, ip_data) = parser::parse_ip_addr_with_family(&to_ip)?;
         ndm.ndm_family = family;
         // Safe because we have allocated enough buffer space.
-        unsafe { nlh.addattr_var(NDA_DST, ip_data.as_ptr() as *const u8, ip_data.len()) };
+        unsafe { nlh.addattr_var(NDA_DST, ip_data.as_ref()) };
 
         // process state
         if neigh.state != 0 {
@@ -471,14 +468,6 @@ impl TryFrom<Route> for RtRoute {
             Some(parser::parse_ip_addr(r.gateway.as_str())?)
         };
 
-        /*
-                let (dest, dst_len) = if gateway.is_some() {
-                    (vec![0 as u8; 4], 0)
-                } else {
-                    (tdest, tdst_len)
-                };
-        */
-
         Ok(Self {
             dest,
             source,

src/agent/netlink/src/lib.rs

@@ -11,6 +11,18 @@
 //! - all fields have been correctly aligned
 
 #![allow(non_camel_case_types)]
+// NOTE!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+// By default, there are many warnings for incorrect alignment like:
+//      casting from `*mut u8` to a more-strictly-aligned pointer (`*mut nlmsghdr`) (1 < 4 bytes)
+//
+// The root cause is that we use a Vec<u8> buffer to receive netlink message from the kernel.
+// The data buffer for a Vec<u8> may be 1-byte aligned, which doesn't match the alignment
+// requirement for netlink message fields. The reason it works without failure now is that
+// rust memory allocation is 16-byte aligned by default.
+//
+// But please do pay attention here, it's no guarantee that it will be 16-byte aligned, just works
+// in this way. For safety, we should use something like the FamStruct from the vmm-sys-util crate.
+#![allow(clippy::cast_ptr_alignment)]
 
 extern crate libc;
 extern crate nix;
@@ -28,6 +40,7 @@ extern crate slog_scope;
 
 use nix::errno::Errno;
 use std::borrow::Borrow;
+use std::ffi::CString;
 use std::fmt;
 use std::mem;
 use std::os::unix::io::RawFd;
@@ -246,7 +259,7 @@ pub const RT_TABLE_COMPAT: libc::c_uint = 252;
 pub const RT_TABLE_DEFAULT: libc::c_uint = 253;
 pub const RT_TABLE_MAIN: libc::c_uint = 254;
 pub const RT_TABLE_LOCAL: libc::c_uint = 255;
-pub const RT_TABLE_MAX: libc::c_uint = 0xffffffff;
+pub const RT_TABLE_MAX: libc::c_uint = 0xffff_ffff;
 
 // rat_type c_ushort
 pub const RTA_UNSPEC: libc::c_ushort = 0;
@@ -456,10 +469,10 @@ pub const RTAX_FASTOPEN_NO_COOKIE: libc::c_ushort = 17;
 pub const __RTAX_MAX: libc::c_ushort = 18;
 
 pub const RTAX_MAX: libc::c_ushort = __RTAX_MAX - 1;
-pub const RTAX_FEATURE_ECN: libc::c_ushort = 1 << 0;
+pub const RTAX_FEATURE_ECN: libc::c_ushort = 0x1;
-pub const RTAX_FEATURE_SACK: libc::c_ushort = 1 << 1;
+pub const RTAX_FEATURE_SACK: libc::c_ushort = 0x2;
-pub const RTAX_FEATURE_TIMESTAMP: libc::c_ushort = 1 << 2;
+pub const RTAX_FEATURE_TIMESTAMP: libc::c_ushort = 0x4;
-pub const RTAX_FEATURE_ALLFRAG: libc::c_ushort = 1 << 3;
+pub const RTAX_FEATURE_ALLFRAG: libc::c_ushort = 0x8;
 pub const RTAX_FEATURE_MASK: libc::c_ushort =
     RTAX_FEATURE_ECN | RTAX_FEATURE_SACK | RTAX_FEATURE_TIMESTAMP | RTAX_FEATURE_ALLFRAG;
 
@@ -1005,10 +1018,10 @@ pub const NLMSG_OVERRUN: __u16 = 0x4;
 pub const NLMSG_MIN_TYPE: __u16 = 0x10;
 
 // IFLA_EXT_MASK
-pub const RTEXT_FILTER_VF: __u32 = 1 << 0;
+pub const RTEXT_FILTER_VF: __u32 = 0x1;
-pub const RTEXT_FILTER_BRVLAN: __u32 = 1 << 1;
+pub const RTEXT_FILTER_BRVLAN: __u32 = 0x2;
-pub const RTEXT_FILTER_BRVLAN_COMPRESSED: __u32 = 1 << 2;
+pub const RTEXT_FILTER_BRVLAN_COMPRESSED: __u32 = 0x4;
-pub const RTEXT_FILTER_SKIP_STATS: __u32 = 1 << 3;
+pub const RTEXT_FILTER_SKIP_STATS: __u32 = 0x8;
 
 // IFLA attr
 pub const IFLA_UNSPEC: __u16 = 0;
@@ -1302,7 +1315,7 @@ impl RtnlHandle {
         Ok(Self {
             fd,
             local: sa,
-            seq: unsafe { libc::time(0 as *mut libc::time_t) } as __u32,
+            seq: unsafe { libc::time(std::ptr::null_mut::<libc::time_t>()) } as __u32,
             dump: 0,
         })
     }
@@ -1412,7 +1425,7 @@ impl RtnlHandle {
         let mut sa: libc::sockaddr_nl = unsafe { mem::zeroed::<libc::sockaddr_nl>() };
         let mut h = unsafe { mem::zeroed::<libc::msghdr>() };
         let mut iov = libc::iovec {
-            iov_base: 0 as *mut libc::c_void,
+            iov_base: std::ptr::null_mut::<libc::c_void>(),
             iov_len: 0 as libc::size_t,
         };
 
@@ -1649,7 +1662,11 @@ impl RtnlHandle {
 
         // Safe because the data buffer should be big enough.
         unsafe {
-            nlh.addattr_var(IFLA_IFNAME, name.as_ptr() as *const u8, name.len() + 1);
+            if let Ok(cname) = CString::new(name.as_bytes()) {
+                nlh.addattr_var(IFLA_IFNAME, cname.as_bytes());
+            } else {
+                return nix_errno(Errno::EINVAL);
+            }
             nlh.addattr32(IFLA_EXT_MASK, RTEXT_FILTER_VF | RTEXT_FILTER_SKIP_STATS);
         }
 
@@ -1761,11 +1778,7 @@ impl RtnlHandle {
 
         // Safe because we have allocated a big enough data buffer.
         unsafe {
-            nlh.addattr_var(
+            nlh.addattr_var(IFA_ADDRESS, addr.addr.as_ref());
-                IFA_ADDRESS,
-                addr.addr.as_ptr() as *const u8,
-                addr.addr.len(),
-            );
         }
 
         // ignore EADDRNOTAVAIL here..
@@ -1774,7 +1787,7 @@ impl RtnlHandle {
         Ok(())
     }
 
-    pub fn delete_all_addrs(&mut self, ifinfo: &ifinfomsg, addrs: &Vec<RtIPAddr>) -> Result<()> {
+    pub fn delete_all_addrs(&mut self, ifinfo: &ifinfomsg, addrs: &[RtIPAddr]) -> Result<()> {
         for a in addrs {
             self.delete_one_addr(ifinfo, a)?;
         }
@@ -1799,9 +1812,9 @@ impl RtnlHandle {
 
         // Safe because we have allocated a big enough data buffer.
         unsafe {
-            nlh.addattr_var(IFA_ADDRESS, ip.addr.as_ptr() as *const u8, ip.addr.len());
+            nlh.addattr_var(IFA_ADDRESS, ip.addr.as_ref());
             // don't know why need IFA_LOCAL, without it kernel returns -EINVAL...
-            nlh.addattr_var(IFA_LOCAL, ip.addr.as_ptr() as *const u8, ip.addr.len());
+            nlh.addattr_var(IFA_LOCAL, ip.addr.as_ref());
         }
 
         self.rtnl_talk(v.as_mut_slice(), false)?;
@@ -1950,7 +1963,7 @@ impl RtnlHandle {
         Ok(rs)
     }
 
-    pub fn delete_all_routes(&mut self, rs: &Vec<RtRoute>) -> Result<()> {
+    pub fn delete_all_routes(&mut self, rs: &[RtRoute]) -> Result<()> {
         for r in rs {
             let name = self.get_name_by_index(r.index)?;
             if name.as_str().contains("lo") || name.as_str().contains("::1") {
@@ -1991,22 +2004,19 @@ impl RtnlHandle {
 
         unsafe {
             if let Some(source) = r.source.as_ref() {
-                let len = source.len();
                 if r.src_len > 0 {
-                    nlh.addattr_var(RTA_SRC, source.as_ptr() as *const u8, len);
+                    nlh.addattr_var(RTA_SRC, source.as_ref());
                 } else {
-                    nlh.addattr_var(RTA_PREFSRC, source.as_ptr() as *const u8, len);
+                    nlh.addattr_var(RTA_PREFSRC, source.as_ref());
                 }
             }
 
             if let Some(dest) = r.dest.as_ref() {
-                let len = dest.len();
+                nlh.addattr_var(RTA_DST, dest.as_ref());
-                nlh.addattr_var(RTA_DST, dest.as_ptr() as *const u8, len);
             }
 
             if let Some(gateway) = r.gateway.as_ref() {
-                let len = gateway.len();
+                nlh.addattr_var(RTA_GATEWAY, gateway.as_ref());
-                nlh.addattr_var(RTA_GATEWAY, gateway.as_ptr() as *const u8, len);
             }
 
             nlh.addattr32(RTA_OIF, r.index as u32);
@@ -2040,22 +2050,19 @@ impl RtnlHandle {
         // Safe because we have allocated a big enough data buffer.
         unsafe {
             if let Some(source) = r.source.as_ref() {
-                let len = source.len();
                 if r.src_len > 0 {
-                    nlh.addattr_var(RTA_SRC, source.as_ptr() as *const u8, len);
+                    nlh.addattr_var(RTA_SRC, source.as_ref());
                 } else {
-                    nlh.addattr_var(RTA_PREFSRC, source.as_ptr() as *const u8, len);
+                    nlh.addattr_var(RTA_PREFSRC, source.as_ref());
                 }
             }
 
             if let Some(dest) = r.dest.as_ref() {
-                let len = dest.len();
+                nlh.addattr_var(RTA_DST, dest.as_ref());
-                nlh.addattr_var(RTA_DST, dest.as_ptr() as *const u8, len);
             }
 
             if let Some(gateway) = r.gateway.as_ref() {
-                let len = gateway.len();
+                nlh.addattr_var(RTA_GATEWAY, gateway.as_ref());
-                nlh.addattr_var(RTA_GATEWAY, gateway.as_ptr() as *const u8, len);
             }
 
             nlh.addattr32(RTA_OIF, r.index as u32);
@@ -2092,8 +2099,8 @@ impl Drop for RtnlHandle {
 
 /// Parse netlink attributes from raw buffer.
 ///
-/// Safety:
+/// # Safety
-/// Caller needs to ensure rta and rtalen are valid.
+/// Caller needs to ensure that rta and rtalen are valid.
 pub unsafe fn parse_attrs(
     mut rta: *const rtattr,
     mut rtalen: u32,
@@ -2115,24 +2122,32 @@ pub unsafe fn parse_attrs(
 }
 
 impl nlmsghdr {
-    pub unsafe fn addattr_var(&mut self, cat: u16, data: *const u8, len: usize) {
+    /// Add an variable attribute to the netlink message.
+    ///
+    /// # Safety
+    /// Caller needs to ensure that there are enough space after `self` to store the attribute.
+    pub unsafe fn addattr_var(&mut self, cat: u16, data: &[u8]) {
         let mut rta: *mut rtattr = NLMSG_TAIL!(self) as *mut rtattr;
-        let alen = RTA_LENGTH!(len) as u16;
+        let alen = RTA_LENGTH!(data.len()) as u16;
 
         (*rta).rta_type = cat;
         (*rta).rta_len = alen;
 
-        if len > 0 {
+        if !data.is_empty() {
             libc::memcpy(
                 RTA_DATA!(rta) as *mut libc::c_void,
-                data as *const libc::c_void,
+                data.as_ptr() as *const libc::c_void,
-                len,
+                data.len(),
             );
         }
 
         self.nlmsg_len = NLMSG_ALIGN!(self.nlmsg_len) + RTA_ALIGN!(alen);
     }
 
+    /// Add a string attribute to the netlink message.
+    ///
+    /// # Safety
+    /// Caller needs to ensure that there are enough space after `self` to store the attribute.
     pub unsafe fn addattr_str(&mut self, cat: u16, data: &str) {
         let mut rta: *mut rtattr = NLMSG_TAIL!(self) as *mut rtattr;
         let len = data.len();
@@ -2151,6 +2166,10 @@ impl nlmsghdr {
         self.nlmsg_len = NLMSG_ALIGN!(self.nlmsg_len) + RTA_ALIGN!(alen);
     }
 
+    /// Add an 1/2/4/8 bytes attribute to the netlink message.
+    ///
+    /// # Safety
+    /// Caller needs to ensure that there are enough space after `self` to store the attribute.
     pub unsafe fn addattr_size(&mut self, cat: u16, val: u64, size: u8) {
         assert_eq!(size == 1 || size == 2 || size == 4 || size == 8, true);
 
@@ -2190,32 +2209,32 @@ impl nlmsghdr {
 
     /// Add a 8-bit attribute.
     ///
-    /// Safety:
+    /// # Safety
-    /// Caller needs to ensure there's enough space to store the attribute.
+    /// Caller needs to ensure that there are enough space after `self` to store the attribute.
     pub unsafe fn addattr8(&mut self, cat: u16, val: u8) {
         self.addattr_size(cat, val as u64, 1);
     }
 
     /// Add a 16-bit attribute.
     ///
-    /// Safety:
+    /// # Safety
-    /// Caller needs to ensure there's enough space to store the attribute.
+    /// Caller needs to ensure that there are enough space after `self` to store the attribute.
     pub unsafe fn addattr16(&mut self, cat: u16, val: u16) {
         self.addattr_size(cat, val as u64, 2);
     }
 
     /// Add a 32-bit attribute.
     ///
-    /// Safety:
+    /// # Safety
-    /// Caller needs to ensure there's enough space to store the attribute.
+    /// Caller needs to ensure that there are enough space after `self` to store the attribute.
     pub unsafe fn addattr32(&mut self, cat: u16, val: u32) {
         self.addattr_size(cat, val as u64, 4);
     }
 
     /// Add a 64-bit attribute.
     ///
-    /// Safety:
+    /// # Safety
-    /// Caller needs to ensure there's enough space to store the attribute.
+    /// Caller needs to ensure that there are enough space after `self` to store the attribute.
     pub unsafe fn addattr64(&mut self, cat: u16, val: u64) {
         self.addattr_size(cat, val, 8);
     }
@@ -2249,30 +2268,50 @@ unsafe fn getattr_size(rta: *const rtattr) -> u64 {
     panic!("impossible!");
 }
 
+/// Get a 8-bit attribute.
+///
+/// # Safety
+/// Caller needs to ensure that there are enough space after `rta` to read the attribute.
 pub unsafe fn getattr8(rta: *const rtattr) -> u8 {
     let alen = RTA_PAYLOAD!(rta);
-    assert!(alen == 1);
+    assert_eq!(alen, 1);
     getattr_size(rta) as u8
 }
 
+/// Get a 16-bit attribute.
+///
+/// # Safety
+/// Caller needs to ensure that there are enough space after `rta` to read the attribute.
 pub unsafe fn getattr16(rta: *const rtattr) -> u16 {
     let alen = RTA_PAYLOAD!(rta);
-    assert!(alen == 2);
+    assert_eq!(alen, 2);
     getattr_size(rta) as u16
 }
 
+/// Get a 32-bit attribute.
+///
+/// # Safety
+/// Caller needs to ensure that there are enough space after `rta` to read the attribute.
 pub unsafe fn getattr32(rta: *const rtattr) -> u32 {
     let alen = RTA_PAYLOAD!(rta);
-    assert!(alen == 4);
+    assert_eq!(alen, 4);
     getattr_size(rta) as u32
 }
 
+/// Get a 64-bit attribute.
+///
+/// # Safety
+/// Caller needs to ensure that there are enough space after `rta` to read the attribute.
 pub unsafe fn getattr64(rta: *const rtattr) -> u64 {
     let alen = RTA_PAYLOAD!(rta);
-    assert!(alen == 8);
+    assert_eq!(alen, 8);
     getattr_size(rta)
 }
 
+/// Get a variable length attribute.
+///
+/// # Safety
+/// Caller needs to ensure that there are enough space after `rta` to read the attribute.
 pub unsafe fn getattr_var(rta: *const rtattr) -> Vec<u8> {
     assert_ne!(rta as i64, 0);
     let data: *const libc::c_void = RTA_DATA!(rta) as *const libc::c_void;

src/agent/netlink/src/parser.rs

@@ -37,12 +37,12 @@ pub fn parse_ip_addr_with_family(ip_address: &str) -> Result<(__u8, Vec<u8>)> {
     if let Ok(v6) = Ipv6Addr::from_str(ip_address) {
         Ok((libc::AF_INET6 as __u8, Vec::from(v6.octets().as_ref())))
     } else {
-        parse_ipv4_addr(ip_address).map(|v| ((libc::AF_INET as __u8, v)))
+        parse_ipv4_addr(ip_address).map(|v| (libc::AF_INET as __u8, v))
     }
 }
 
 pub fn parse_ipv4_cidr(s: &str) -> Result<(Vec<u8>, u8)> {
-    let fields: Vec<&str> = s.split("/").collect();
+    let fields: Vec<&str> = s.split('/').collect();
 
     if fields.len() != 2 {
         nix_errno(Errno::EINVAL)
@@ -52,7 +52,7 @@ pub fn parse_ipv4_cidr(s: &str) -> Result<(Vec<u8>, u8)> {
 }
 
 pub fn parse_cidr(s: &str) -> Result<(Vec<u8>, u8)> {
-    let fields: Vec<&str> = s.split("/").collect();
+    let fields: Vec<&str> = s.split('/').collect();
 
     if fields.len() != 2 {
         nix_errno(Errno::EINVAL)
@@ -62,7 +62,7 @@ pub fn parse_cidr(s: &str) -> Result<(Vec<u8>, u8)> {
 }
 
 pub fn parse_mac_addr(hwaddr: &str) -> Result<Vec<u8>> {
-    let fields: Vec<&str> = hwaddr.split(":").collect();
+    let fields: Vec<&str> = hwaddr.split(':').collect();
 
     if fields.len() != 6 {
         nix_errno(Errno::EINVAL)
@@ -80,8 +80,8 @@ pub fn parse_mac_addr(hwaddr: &str) -> Result<Vec<u8>> {
 
 /// Format an IPv4/IPv6/MAC address.
 ///
-/// Safety:
+/// # Safety
-/// Caller needs to ensure addr and len are valid.
+/// Caller needs to ensure that addr and len are valid.
 pub unsafe fn format_address(addr: *const u8, len: u32) -> Result<String> {
     let mut a: String;
     if len == 4 {