Skip to content

Commit

Permalink
Cast pointers more carefully (#2140)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
asomers committed Oct 1, 2023
1 parent 57663c0 commit e5b2df6
Show file tree
Hide file tree
Showing 22 changed files with 109 additions and 117 deletions.
8 changes: 4 additions & 4 deletions src/fcntl.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
),
}
Expand Down Expand Up @@ -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(),
)
Expand Down
2 changes: 1 addition & 1 deletion src/ifaddrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);

Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
4 changes: 2 additions & 2 deletions src/mount/bsd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(),
});

Expand Down
13 changes: 4 additions & 9 deletions src/mqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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,
)
Expand All @@ -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(),
Expand All @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion src/net/if_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}
Expand All @@ -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) }
}
Expand Down
8 changes: 2 additions & 6 deletions src/poll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions src/sys/aio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/sys/epoll.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
4 changes: 2 additions & 2 deletions src/sys/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/sys/inotify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions src/sys/ptrace/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down
6 changes: 3 additions & 3 deletions src/sys/quota.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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()) })
}
Expand Down
14 changes: 8 additions & 6 deletions src/sys/sendfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
2 changes: 1 addition & 1 deletion src/sys/signalfd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit e5b2df6

Please sign in to comment.