Fix UB from unsafe uninitialized buffer slices

Creating &mut [u8] (and &[u8]) for unitialized memory is undefined
behaviour even when not actually reading the data.

Use bytes::BufMut instead, and advance buffer in recv functions.
High level functions (without flags param) don't need to return length
of read, as it was used to advance the buffer - only low-level read
might return larger length than the buffer (PEEK + TRUNC flags).

This also bumps netlink-sys; the other crates already got bumped.

ethtool/Cargo.toml

@@ -30,7 +30,7 @@ netlink-packet-core = "0.3.0"
 netlink-packet-generic = "0.2.0"
 netlink-packet-utils = "0.4.1"
 netlink-proto = { default-features = false, version = "0.8.0" }
-netlink-sys = "0.7.0"
+netlink-sys = "0.8.0"
 thiserror = "1.0.29"
 tokio = { version = "1.0.1", features = ["rt"], optional = true}
 

netlink-packet-generic/Cargo.toml

@@ -18,4 +18,4 @@ netlink-packet-core = "0.3"
 netlink-packet-utils = "0.4"
 
 [dev-dependencies]
-netlink-sys = { path = "../netlink-sys", version = "0.7" }
+netlink-sys = { path = "../netlink-sys", version = "0.8" }

netlink-packet-generic/tests/query_family_id.rs

@@ -26,8 +26,7 @@ fn query_family_id() {
 
     socket.send(&txbuf, 0).unwrap();
 
-    let mut rxbuf = vec![0u8; 2048];
-    socket.recv(&mut rxbuf, 0).unwrap();
+    let (rxbuf, _addr) = socket.recv_from_full().unwrap();
     let rx_packet = <NetlinkMessage<GenlMessage<GenlCtrl>>>::deserialize(&rxbuf).unwrap();
 
     if let NetlinkPayload::InnerMessage(genlmsg) = rx_packet.payload {

netlink-packet-route/Cargo.toml

@@ -26,7 +26,7 @@ name = "dump_links"
 criterion = "0.3.0"
 pcap-file = "1.1.1"
 lazy_static = "1.4.0"
-netlink-sys = "0.7"
+netlink-sys = "0.8"
 
 [[bench]]
 name = "link_message"

netlink-packet-route/examples/dump_links.rs

@@ -37,7 +37,7 @@ fn main() {
 
     // we set the NLM_F_DUMP flag so we expect a multipart rx_packet in response.
     loop {
-        let size = socket.recv(&mut receive_buffer[..], 0).unwrap();
+        let size = socket.recv(&mut &mut receive_buffer[..], 0).unwrap();
 
         loop {
             let bytes = &receive_buffer[offset..];

netlink-packet-route/examples/dump_neighbours.rs

@@ -38,7 +38,7 @@ fn main() {
     let mut offset = 0;
 
     'outer: loop {
-        let size = socket.recv(&mut receive_buffer[..], 0).unwrap();
+        let size = socket.recv(&mut &mut receive_buffer[..], 0).unwrap();
 
         loop {
             let bytes = &receive_buffer[offset..];

netlink-packet-route/examples/dump_rules.rs

@@ -41,7 +41,7 @@ fn main() {
     let mut offset = 0;
 
     // we set the NLM_F_DUMP flag so we expect a multipart rx_packet in response.
-    while let Ok(size) = socket.recv(&mut receive_buffer[..], 0) {
+    while let Ok(size) = socket.recv(&mut &mut receive_buffer[..], 0) {
         loop {
             let bytes = &receive_buffer[offset..];
             let rx_packet = <NetlinkMessage<RtnlMessage>>::deserialize(bytes).unwrap();

netlink-packet-sock-diag/Cargo.toml

@@ -22,4 +22,4 @@ smallvec = "1.4.2"
 
 [dev-dependencies]
 lazy_static = "1.4.0"
-netlink-sys = "0.7"
+netlink-sys = "0.8"

netlink-packet-sock-diag/examples/dump_ipv4.rs

@@ -46,7 +46,7 @@ fn main() {
 
     let mut receive_buffer = vec![0; 4096];
     let mut offset = 0;
-    while let Ok(size) = socket.recv(&mut receive_buffer[..], 0) {
+    while let Ok(size) = socket.recv(&mut &mut receive_buffer[..], 0) {
         loop {
             let bytes = &receive_buffer[offset..];
             let rx_packet = <NetlinkMessage<SockDiagMessage>>::deserialize(bytes).unwrap();

netlink-proto/Cargo.toml

@@ -17,7 +17,7 @@ log = "0.4.8"
 futures = "0.3"
 tokio = { version = "1.0", default-features = false, features = ["io-util"] }
 netlink-packet-core = "0.3"
-netlink-sys = { default-features = false, version = "0.7" }
+netlink-sys = { default-features = false, version = "0.8" }
 
 [features]
 default = ["tokio_socket"]

netlink-proto/src/framed.rs

@@ -1,10 +1,9 @@
-use bytes::{BufMut, BytesMut};
+use bytes::BytesMut;
 use std::{
     fmt::Debug,
     io,
     marker::PhantomData,
     pin::Pin,
-    slice,
     task::{Context, Poll},
 };
 
@@ -56,23 +55,11 @@ where
             reader.clear();
             reader.reserve(INITIAL_READER_CAPACITY);
 
-            *in_addr = unsafe {
-                // Read into the buffer without having to initialize the memory.
-                //
-                // safety: we know poll_recv_from never reads from the
-                // memory during a recv so it's fine to turn &mut
-                // [<MaybeUninitialized<u8>>] into &mut[u8]
-                let bytes = reader.chunk_mut();
-                let bytes = slice::from_raw_parts_mut(bytes.as_mut_ptr(), bytes.len());
-                match ready!(socket.poll_recv_from(cx, bytes)) {
-                    Ok((n, addr)) => {
-                        reader.advance_mut(n);
-                        addr
-                    }
-                    Err(e) => {
-                        error!("failed to read from netlink socket: {:?}", e);
-                        return Poll::Ready(None);
-                    }
+            *in_addr = match ready!(socket.poll_recv_from(cx, reader)) {
+                Ok(addr) => addr,
+                Err(e) => {
+                    error!("failed to read from netlink socket: {:?}", e);
+                    return Poll::Ready(None);
                 }
             };
         }

netlink-sys/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 authors = ["Corentin Henry <[email protected]>"]
 name = "netlink-sys"
-version = "0.7.0"
+version = "0.8.0"  # TODO: drop this comment - already bumped version for trait changes
 edition = "2018"
 
 homepage = "https://github.com/little-dude/netlink"
@@ -12,6 +12,7 @@ repository = "https://github.com/little-dude/netlink"
 description = "netlink sockets, with optional integration with tokio"
 
 [dependencies]
+bytes = "1.0"
 libc = "0.2.66"
 log = "0.4.8"
 

netlink-sys/examples/audit_events_tokio.rs

@@ -50,20 +50,22 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
         .await
         .unwrap();
 
-    let mut buf = vec![0; 1024 * 8];
+    let mut buf = bytes::BytesMut::with_capacity(1024 * 8);
     loop {
-        let (n, _addr) = socket.recv_from(&mut buf).await.unwrap();
+        buf.clear();
+        let _addr = socket.recv_from(&mut buf).await.unwrap();
         // This dance with the NetlinkBuffer should not be
         // necessary. It is here to work around a netlink bug. See:
         // https://github.com/mozilla/libaudit-go/issues/24
         // https://github.com/linux-audit/audit-userspace/issues/78
         {
-            let mut nl_buf = NetlinkBuffer::new(&mut buf[0..n]);
+            let n = buf.len();
+            let mut nl_buf = NetlinkBuffer::new(&mut buf);
             if n != nl_buf.length() as usize {
                 nl_buf.set_length(n as u32);
             }
         }
-        let parsed = NetlinkMessage::<AuditMessage>::deserialize(&buf[0..n]).unwrap();
+        let parsed = NetlinkMessage::<AuditMessage>::deserialize(&buf).unwrap();
         println!("<<< {:?}", parsed);
     }
 }

netlink-sys/src/smol.rs

@@ -43,23 +43,32 @@ impl SmolSocket {
             .await
     }
 
-    pub async fn recv(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-        self.0.read_with_mut(|sock| sock.recv(buf, 0)).await
+    pub async fn recv(&mut self, buf: &mut B) -> io::Result<()>
+    where
+        B: bytes::BufMut,
+    {
+        self.0.read_with_mut(|sock| sock.recv(buf, 0).map(|_len| ())).await
     }
 
-    pub async fn recv_from(&mut self, buf: &mut [u8]) -> io::Result<(usize, SocketAddr)> {
-        self.0.read_with_mut(|sock| sock.recv_from(buf, 0)).await
+    pub async fn recv_from(&mut self, buf: &mut B) -> io::Result<SocketAddr>
+    where
+        B: bytes::BufMut,
+    {
+        self.0.read_with_mut(|sock| sock.recv_from(buf, 0).map(|(_len, addr)| addr)).await
     }
 
     pub async fn recv_from_full(&mut self) -> io::Result<(Vec<u8>, SocketAddr)> {
         self.0.read_with_mut(|sock| sock.recv_from_full()).await
     }
 
-    pub fn poll_recv_from(
+    pub fn poll_recv_from<B>(
         &mut self,
         cx: &mut Context,
-        buf: &mut [u8],
-    ) -> Poll<io::Result<(usize, SocketAddr)>> {
+        buf: &mut B,
+    ) -> Poll<io::Result<SocketAddr>>
+    where
+        B: bytes::BufMut,
+    {
         loop {
             trace!("poll_recv_from called");
             let _guard = ready!(self.0.poll_readable(cx))?;
@@ -72,7 +81,7 @@ impl SmolSocket {
                 }
                 x => {
                     trace!("poll_recv_from {:?} bytes read", x);
-                    return Poll::Ready(x);
+                    return Poll::Ready(x.map(|(_len, addr)| addr));
                 }
             }
         }

netlink-sys/src/socket.rs

@@ -36,7 +36,7 @@ use crate::SocketAddr;
 /// let mut buf = vec![0; 4096];
 /// loop {
 ///     // receive a datagram
-///     let (n_received, sender_addr) = socket.recv_from(&mut buf[..], 0).unwrap();
+///     let (n_received, sender_addr) = socket.recv_from(&mut &mut buf[..], 0).unwrap();
 ///     assert_eq!(sender_addr, kernel_addr);
 ///     println!("received datagram {:?}", &buf[..n_received]);
 ///     if buf[4] == 2 && buf[5] == 0 {
@@ -165,7 +165,7 @@ impl Socket {
     /// // buffer for receiving the response
     /// let mut buf = vec![0; 4096];
     /// loop {
-    ///     let mut n_received = socket.recv(&mut buf[..], 0).unwrap();
+    ///     let mut n_received = socket.recv(&mut &mut buf[..], 0).unwrap();
     ///     println!("received {:?}", &buf[..n_received]);
     ///     if buf[4] == 2 && buf[5] == 0 {
     ///         println!("the kernel responded with an error");
@@ -224,7 +224,14 @@ impl Socket {
     /// In datagram oriented protocols, `recv` and `recvfrom` receive normally only ONE datagram, but this seems not to
     /// be always true for netlink sockets: with some protocols like `NETLINK_AUDIT`, multiple netlink packets can be
     /// read with a single call.
-    pub fn recv_from(&self, buf: &mut [u8], flags: libc::c_int) -> Result<(usize, SocketAddr)> {
+    pub fn recv_from<B>(
+        &self,
+        buf: &mut B,
+        flags: libc::c_int,
+    ) -> Result<(usize, SocketAddr)>
+    where
+        B: bytes::BufMut,
+    {
         // Create an empty storage for the address. Note that Rust standard library create a
         // sockaddr_storage so that it works for any address family, but here, we already know that
         // we'll have a Netlink address, so we can create the appropriate storage.
@@ -252,34 +259,46 @@ impl Socket {
         // a pointer to it.
         let addrlen_ptr = &mut addrlen as *mut usize as *mut libc::socklen_t;
 
-        //                      Cast the *mut u8 into *mut void.
-        //               This is equivalent to casting a *char into *void
-        //                                 See [thread]
-        //                                       ^
-        //           Create a *mut u8            |
-        //                   ^                   |
-        //                   |                   |
-        //             +-----+-----+    +--------+-------+
-        //            /             \  /                  \
-        let buf_ptr = buf.as_mut_ptr() as *mut libc::c_void;
-        let buf_len = buf.len() as libc::size_t;
+        let chunk = buf.chunk_mut();
+        //                        Cast the *mut u8 into *mut void.
+        //                 This is equivalent to casting a *char into *void
+        //                                   See [thread]
+        //                                         ^
+        //             Create a *mut u8            |
+        //                    ^                    |
+        //                    |                    |
+        //             +------+-------+   +--------+-------+
+        //            /                \ /                  \
+        let buf_ptr = chunk.as_mut_ptr() as *mut libc::c_void;
+        let buf_len = chunk.len() as libc::size_t;
 
         let res = unsafe { libc::recvfrom(self.0, buf_ptr, buf_len, flags, addr_ptr, addrlen_ptr) };
         if res < 0 {
             return Err(Error::last_os_error());
+        } else {
+            let written = std::cmp::min(buf_len, res as usize);
+            unsafe { buf.advance_mut(written); }
         }
         Ok((res as usize, SocketAddr(addr)))
     }
 
     /// For a connected socket, `recv` reads a datagram from the socket. The sender is the remote peer the socket is
     /// connected to (see [`Socket::connect`]). See also [`Socket::recv_from`]
-    pub fn recv(&self, buf: &mut [u8], flags: libc::c_int) -> Result<usize> {
-        let buf_ptr = buf.as_mut_ptr() as *mut libc::c_void;
-        let buf_len = buf.len() as libc::size_t;
+    pub fn recv<B>(&self, buf: &mut B, flags: libc::c_int) -> Result<usize>
+    where
+        B: bytes::BufMut,
+    {
+        let chunk = buf.chunk_mut();
+        let buf_ptr = chunk.as_mut_ptr() as *mut libc::c_void;
+        let buf_len = chunk.len() as libc::size_t;
 
         let res = unsafe { libc::recv(self.0, buf_ptr, buf_len, flags) };
         if res < 0 {
             return Err(Error::last_os_error());
+        } else {
+            // with `MSG_PEEK | MSG_TRUNC` `res` might exceed `buf_len`
+            let written = std::cmp::min(buf_len, res as usize);
+            unsafe { buf.advance_mut(written); }
         }
         Ok(res as usize)
     }
@@ -288,13 +307,14 @@ impl Socket {
     /// buffer passed as argument, this method always reads a whole message, no matter its size.
     pub fn recv_from_full(&self) -> Result<(Vec<u8>, SocketAddr)> {
         // Peek
-        let mut buf = Vec::<u8>::new();
-        let (rlen, _) = self.recv_from(&mut buf, libc::MSG_PEEK | libc::MSG_TRUNC)?;
+        let mut buf: Vec<u8> = Vec::new();
+        let (peek_len, _) = self.recv_from(&mut buf, libc::MSG_PEEK | libc::MSG_TRUNC)?;
 
         // Receive
-        let mut buf = vec![0; rlen as usize];
-        let (_, addr) = self.recv_from(&mut buf, 0)?;
-
+        buf.clear();
+        buf.reserve(peek_len);
+        let (rlen, addr) = self.recv_from(&mut buf, 0)?;
+        assert_eq!(rlen, peek_len);
         Ok((buf, addr))
     }
 

netlink-sys/src/tokio.rs

@@ -71,7 +71,10 @@ impl TokioSocket {
         }
     }
 
-    pub async fn recv(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+    pub async fn recv<B>(&mut self, buf: &mut B) -> io::Result<()>
+    where
+        B: bytes::BufMut,
+    {
         poll_fn(|cx| loop {
             // Check if the socket is readable. If not,
             // AsyncFd::poll_read_ready would have arranged for the
@@ -80,26 +83,35 @@ impl TokioSocket {
             let mut guard = ready!(self.0.poll_read_ready(cx))?;
 
             match guard.try_io(|inner| inner.get_ref().recv(buf, 0)) {
-                Ok(x) => return Poll::Ready(x),
+                Ok(x) => return Poll::Ready(x.map(|_len| ())),
                 Err(_would_block) => continue,
             }
         })
         .await
     }
 
-    pub async fn recv_from(&mut self, buf: &mut [u8]) -> io::Result<(usize, SocketAddr)> {
+    pub async fn recv_from<B>(
+        &mut self,
+        buf: &mut B,
+    ) -> io::Result<SocketAddr>
+    where
+        B: bytes::BufMut,
+    {
         poll_fn(|cx| self.poll_recv_from(cx, buf)).await
     }
 
     pub async fn recv_from_full(&mut self) -> io::Result<(Vec<u8>, SocketAddr)> {
         poll_fn(|cx| self.poll_recv_from_full(cx)).await
     }
 
-    pub fn poll_recv_from(
+    pub fn poll_recv_from<B>(
         &mut self,
         cx: &mut Context,
-        buf: &mut [u8],
-    ) -> Poll<io::Result<(usize, SocketAddr)>> {
+        buf: &mut B,
+    ) -> Poll<io::Result<SocketAddr>>
+    where
+        B: bytes::BufMut,
+    {
         loop {
             trace!("poll_recv_from called");
             let mut guard = ready!(self.0.poll_read_ready(cx))?;
@@ -108,7 +120,7 @@ impl TokioSocket {
             match guard.try_io(|inner| inner.get_ref().recv_from(buf, 0)) {
                 Ok(x) => {
                     trace!("poll_recv_from {:?} bytes read", x);
-                    return Poll::Ready(x);
+                    return Poll::Ready(x.map(|(_len, addr)| addr));
                 }
                 Err(_would_block) => {
                     trace!("poll_recv_from socket would block");