Cast pointers more carefully (#2140)

* Cast pointers more carefully

Prefer ptr::{cast, cast_const, cast_mut} over `as`.  The latter makes it
too easy to accidentally change both the type and mutability when
changing only one was intended.

This exercise caught an unintended mutability cast in one function, the
BSD version of sendfile.  In this case there's no UB because it isn't
possible for anything else to get a reference to the data that was
incorrectly cast.

There was also a type cast that wasn't guaranteed to be correct (but
probably was) due to memory layout guarantees in if_nametoindex.

* Remove unnecessary cast in UnixCredentials::groups

Author: asomers

src/fcntl.rs

@@ -1,5 +1,5 @@
 use crate::errno::Errno;
-use libc::{self, c_char, c_int, c_uint, size_t, ssize_t};
+use libc::{self, c_int, c_uint, size_t, ssize_t};
 use std::ffi::OsString;
 #[cfg(not(target_os = "redox"))]
 use std::os::raw;
@@ -306,12 +306,12 @@ fn readlink_maybe_at<P: ?Sized + NixPath>(
             Some(dirfd) => libc::readlinkat(
                 dirfd,
                 cstr.as_ptr(),
-                v.as_mut_ptr() as *mut c_char,
+                v.as_mut_ptr().cast(),
                 v.capacity() as size_t,
             ),
             None => libc::readlink(
                 cstr.as_ptr(),
-                v.as_mut_ptr() as *mut c_char,
+                v.as_mut_ptr().cast(),
                 v.capacity() as size_t,
             ),
         }
@@ -724,7 +724,7 @@ pub fn vmsplice(
     let ret = unsafe {
         libc::vmsplice(
             fd,
-            iov.as_ptr() as *const libc::iovec,
+            iov.as_ptr().cast(),
             iov.len(),
             flags.bits(),
         )

src/ifaddrs.rs

@@ -65,7 +65,7 @@ unsafe fn workaround_xnu_bug(info: &libc::ifaddrs) -> Option<SockaddrStorage> {
     // memcpy only sa_len bytes, assume the rest is zero
     std::ptr::copy_nonoverlapping(
         src_sock as *const u8,
-        dst_sock.as_mut_ptr() as *mut u8,
+        dst_sock.as_mut_ptr().cast(),
         (*src_sock).sa_len.into(),
     );
 

src/lib.rs

@@ -311,7 +311,7 @@ impl NixPath for [u8] {
         }
 
         let mut buf = MaybeUninit::<[u8; MAX_STACK_ALLOCATION]>::uninit();
-        let buf_ptr = buf.as_mut_ptr() as *mut u8;
+        let buf_ptr = buf.as_mut_ptr().cast();
 
         unsafe {
             ptr::copy_nonoverlapping(self.as_ptr(), buf_ptr, self.len());

src/mount/bsd.rs

@@ -215,7 +215,7 @@ impl<'a> Nmount<'a> {
     /// Helper function to push a slice onto the `iov` array.
     fn push_slice(&mut self, val: &'a [u8], is_owned: bool) {
         self.iov.push(libc::iovec {
-            iov_base: val.as_ptr() as *mut _,
+            iov_base: val.as_ptr().cast_mut().cast(),
             iov_len: val.len(),
         });
         self.is_owned.push(is_owned);
@@ -386,7 +386,7 @@ impl<'a> Nmount<'a> {
         // SAFETY: we are pushing a mutable iovec here, so we can't use
         //         the above method
         self.iov.push(libc::iovec {
-            iov_base: errmsg.as_mut_ptr() as *mut c_void,
+            iov_base: errmsg.as_mut_ptr().cast(),
             iov_len: errmsg.len(),
         });
 

src/mqueue.rs

@@ -35,7 +35,7 @@ use crate::NixPath;
 use crate::Result;
 
 use crate::sys::stat::Mode;
-use libc::{self, c_char, mqd_t, size_t};
+use libc::{self, mqd_t, size_t};
 use std::mem;
 #[cfg(any(
     target_os = "linux",
@@ -205,7 +205,7 @@ pub fn mq_receive(
     let res = unsafe {
         libc::mq_receive(
             mqdes.0,
-            message.as_mut_ptr() as *mut c_char,
+            message.as_mut_ptr().cast(),
             len,
             msg_prio as *mut u32,
         )
@@ -229,7 +229,7 @@ feature! {
         let res = unsafe {
             libc::mq_timedreceive(
                 mqdes.0,
-                message.as_mut_ptr() as *mut c_char,
+                message.as_mut_ptr().cast(),
                 len,
                 msg_prio as *mut u32,
                 abstime.as_ref(),
@@ -244,12 +244,7 @@ feature! {
 /// See also [`mq_send(2)`](https://pubs.opengroup.org/onlinepubs/9699919799/functions/mq_send.html)
 pub fn mq_send(mqdes: &MqdT, message: &[u8], msq_prio: u32) -> Result<()> {
     let res = unsafe {
-        libc::mq_send(
-            mqdes.0,
-            message.as_ptr() as *const c_char,
-            message.len(),
-            msq_prio,
-        )
+        libc::mq_send(mqdes.0, message.as_ptr().cast(), message.len(), msq_prio)
     };
     Errno::result(res).map(drop)
 }

src/net/if_.rs

@@ -373,6 +373,7 @@ mod if_nameindex {
     }
 
     /// A list of the network interfaces available on this system. Obtained from [`if_nameindex()`].
+    #[repr(transparent)]
     pub struct Interfaces {
         ptr: NonNull<libc::if_nameindex>,
     }
@@ -388,7 +389,7 @@ mod if_nameindex {
         /// null-terminated, so calling this calculates the length. If random access isn't needed,
         /// [`Interfaces::iter()`] should be used instead.
         pub fn to_slice(&self) -> &[Interface] {
-            let ifs = self.ptr.as_ptr() as *const Interface;
+            let ifs = self.ptr.as_ptr().cast();
             let len = self.iter().count();
             unsafe { std::slice::from_raw_parts(ifs, len) }
         }

src/poll.rs

@@ -190,11 +190,7 @@ libc_bitflags! {
 /// ready.
 pub fn poll(fds: &mut [PollFd], timeout: libc::c_int) -> Result<libc::c_int> {
     let res = unsafe {
-        libc::poll(
-            fds.as_mut_ptr() as *mut libc::pollfd,
-            fds.len() as libc::nfds_t,
-            timeout,
-        )
+        libc::poll(fds.as_mut_ptr().cast(), fds.len() as libc::nfds_t, timeout)
     };
 
     Errno::result(res)
@@ -223,7 +219,7 @@ pub fn ppoll(
     let timeout = timeout.as_ref().map_or(core::ptr::null(), |r| r.as_ref());
     let sigmask = sigmask.as_ref().map_or(core::ptr::null(), |r| r.as_ref());
     let res = unsafe {
-        libc::ppoll(fds.as_mut_ptr() as *mut libc::pollfd,
+        libc::ppoll(fds.as_mut_ptr().cast(),
                     fds.len() as libc::nfds_t,
                     timeout,
                     sigmask)

src/sys/aio.rs

@@ -35,7 +35,7 @@ use std::{
     ptr, thread,
 };
 
-use libc::{c_void, off_t};
+use libc::off_t;
 use pin_utils::unsafe_pinned;
 
 use crate::{
@@ -581,7 +581,7 @@ impl<'a> AioRead<'a> {
     ) -> Self {
         let mut aiocb = AioCb::common_init(fd, prio, sigev_notify);
         aiocb.aiocb.0.aio_nbytes = buf.len();
-        aiocb.aiocb.0.aio_buf = buf.as_mut_ptr() as *mut c_void;
+        aiocb.aiocb.0.aio_buf = buf.as_mut_ptr().cast();
         aiocb.aiocb.0.aio_lio_opcode = libc::LIO_READ;
         aiocb.aiocb.0.aio_offset = offs;
         AioRead {
@@ -702,7 +702,7 @@ impl<'a> AioReadv<'a> {
         // In vectored mode, aio_nbytes stores the length of the iovec array,
         // not the byte count.
         aiocb.aiocb.0.aio_nbytes = bufs.len();
-        aiocb.aiocb.0.aio_buf = bufs.as_mut_ptr() as *mut c_void;
+        aiocb.aiocb.0.aio_buf = bufs.as_mut_ptr().cast();
         aiocb.aiocb.0.aio_lio_opcode = libc::LIO_READV;
         aiocb.aiocb.0.aio_offset = offs;
         AioReadv {
@@ -817,7 +817,7 @@ impl<'a> AioWrite<'a> {
         // but technically its only unsafe to dereference it, not to create
         // it.  Type Safety guarantees that we'll never pass aiocb to
         // aio_read or aio_readv.
-        aiocb.aiocb.0.aio_buf = buf.as_ptr() as *mut c_void;
+        aiocb.aiocb.0.aio_buf = buf.as_ptr().cast_mut().cast();
         aiocb.aiocb.0.aio_lio_opcode = libc::LIO_WRITE;
         aiocb.aiocb.0.aio_offset = offs;
         AioWrite {
@@ -935,7 +935,7 @@ impl<'a> AioWritev<'a> {
         // but technically its only unsafe to dereference it, not to create
         // it.  Type Safety guarantees that we'll never pass aiocb to
         // aio_read or aio_readv.
-        aiocb.aiocb.0.aio_buf = bufs.as_ptr() as *mut c_void;
+        aiocb.aiocb.0.aio_buf = bufs.as_ptr().cast_mut().cast();
         aiocb.aiocb.0.aio_lio_opcode = libc::LIO_WRITEV;
         aiocb.aiocb.0.aio_offset = offs;
         AioWritev {

src/sys/epoll.rs

@@ -148,7 +148,7 @@ impl Epoll {
         let res = unsafe {
             libc::epoll_wait(
                 self.0.as_raw_fd(),
-                events.as_mut_ptr() as *mut libc::epoll_event,
+                events.as_mut_ptr().cast(),
                 events.len() as c_int,
                 timeout as c_int,
             )
@@ -240,7 +240,7 @@ pub fn epoll_wait(
     let res = unsafe {
         libc::epoll_wait(
             epfd,
-            events.as_mut_ptr() as *mut libc::epoll_event,
+            events.as_mut_ptr().cast(),
             events.len() as c_int,
             timeout_ms as c_int,
         )

src/sys/event.rs

@@ -63,9 +63,9 @@ impl Kqueue {
         let res = unsafe {
             libc::kevent(
                 self.0.as_raw_fd(),
-                changelist.as_ptr() as *const libc::kevent,
+                changelist.as_ptr().cast(),
                 changelist.len() as type_of_nchanges,
-                eventlist.as_mut_ptr() as *mut libc::kevent,
+                eventlist.as_mut_ptr().cast(),
                 eventlist.len() as type_of_nchanges,
                 if let Some(ref timeout) = timeout_opt {
                     timeout as *const timespec

src/sys/inotify.rs

@@ -202,7 +202,7 @@ impl Inotify {
                 let mut event = MaybeUninit::<libc::inotify_event>::uninit();
                 ptr::copy_nonoverlapping(
                     buffer.as_ptr().add(offset),
-                    event.as_mut_ptr() as *mut u8,
+                    event.as_mut_ptr().cast(),
                     (BUFSIZ - offset).min(header_size),
                 );
                 event.assume_init()

src/sys/ptrace/linux.rs

@@ -241,13 +241,13 @@ pub fn setregs(pid: Pid, regs: user_regs_struct) -> Result<()> {
 /// and therefore use the data field to return values. This function handles these
 /// requests.
 fn ptrace_get_data<T>(request: Request, pid: Pid) -> Result<T> {
-    let mut data = mem::MaybeUninit::uninit();
+    let mut data = mem::MaybeUninit::<T>::uninit();
     let res = unsafe {
         libc::ptrace(
             request as RequestType,
             libc::pid_t::from(pid),
             ptr::null_mut::<T>(),
-            data.as_mut_ptr() as *const _ as *const c_void,
+            data.as_mut_ptr(),
         )
     };
     Errno::result(res)?;

src/sys/quota.rs

@@ -264,7 +264,7 @@ pub fn quotactl_on<P: ?Sized + NixPath>(
 ) -> Result<()> {
     quota_file.with_nix_path(|path| {
         let mut path_copy = path.to_bytes_with_nul().to_owned();
-        let p: *mut c_char = path_copy.as_mut_ptr() as *mut c_char;
+        let p: *mut c_char = path_copy.as_mut_ptr().cast();
         quotactl(
             QuotaCmd(QuotaSubCmd::Q_QUOTAON, which),
             Some(special),
@@ -308,12 +308,12 @@ pub fn quotactl_get<P: ?Sized + NixPath>(
     special: &P,
     id: c_int,
 ) -> Result<Dqblk> {
-    let mut dqblk = mem::MaybeUninit::uninit();
+    let mut dqblk = mem::MaybeUninit::<libc::dqblk>::uninit();
     quotactl(
         QuotaCmd(QuotaSubCmd::Q_GETQUOTA, which),
         Some(special),
         id,
-        dqblk.as_mut_ptr() as *mut c_char,
+        dqblk.as_mut_ptr().cast(),
     )?;
     Ok(unsafe { Dqblk(dqblk.assume_init()) })
 }

src/sys/sendfile.rs

@@ -96,22 +96,24 @@ cfg_if! {
                 headers: Option<&'a [&'a [u8]]>,
                 trailers: Option<&'a [&'a [u8]]>
             ) -> SendfileHeaderTrailer<'a> {
-                let header_iovecs: Option<Vec<IoSlice<'_>>> =
+                let mut header_iovecs: Option<Vec<IoSlice<'_>>> =
                     headers.map(|s| s.iter().map(|b| IoSlice::new(b)).collect());
-                let trailer_iovecs: Option<Vec<IoSlice<'_>>> =
+                let mut trailer_iovecs: Option<Vec<IoSlice<'_>>> =
                     trailers.map(|s| s.iter().map(|b| IoSlice::new(b)).collect());
                 SendfileHeaderTrailer(
                     libc::sf_hdtr {
                         headers: {
                             header_iovecs
-                                .as_ref()
-                                .map_or(ptr::null(), |v| v.as_ptr()) as *mut libc::iovec
+                                .as_mut()
+                                .map_or(ptr::null_mut(), |v| v.as_mut_ptr())
+                                .cast()
                         },
                         hdr_cnt: header_iovecs.as_ref().map(|v| v.len()).unwrap_or(0) as i32,
                         trailers: {
                             trailer_iovecs
-                                .as_ref()
-                                .map_or(ptr::null(), |v| v.as_ptr()) as *mut libc::iovec
+                                .as_mut()
+                                .map_or(ptr::null_mut(), |v| v.as_mut_ptr())
+                                .cast()
                         },
                         trl_cnt: trailer_iovecs.as_ref().map(|v| v.len()).unwrap_or(0) as i32
                     },

src/sys/signalfd.rs

@@ -109,7 +109,7 @@ impl SignalFd {
 
         let size = mem::size_of_val(&buffer);
         let res = Errno::result(unsafe {
-            libc::read(self.0.as_raw_fd(), buffer.as_mut_ptr() as *mut libc::c_void, size)
+            libc::read(self.0.as_raw_fd(), buffer.as_mut_ptr().cast(), size)
         })
         .map(|r| r as usize);
         match res {