diff --git a/CHANGELOG.md b/CHANGELOG.md index 481a1a2c44..42ba93b956 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). (#[1252](https://github.com/nix-rust/nix/pull/1252)) - Added support for `Ipv4PacketInfo` and `Ipv6PacketInfo` to `ControlMessage`. (#[1222](https://github.com/nix-rust/nix/pull/1222)) +- `CpuSet` and `UnixCredentials` now implement `Default`. + (#[1244](https://github.com/nix-rust/nix/pull/1244)) ### Changed - Changed `fallocate` return type from `c_int` to `()` (#[1201](https://github.com/nix-rust/nix/pull/1201)) @@ -44,6 +46,15 @@ This project adheres to [Semantic Versioning](http://semver.org/). (#[1245](https://github.com/nix-rust/nix/pull/1245)) - `execv`, `execve`, `execvp` and `execveat` in `::nix::unistd` and `reboot` in `::nix::sys::reboot` now return `Result` instead of `Result` (#[1239](https://github.com/nix-rust/nix/pull/1239)) +- `sys::socket::sockaddr_storage_to_addr` is no longer `unsafe`. So is + `offset_of!`. +- `sys::socket::sockaddr_storage_to_addr`, `offset_of!`, and `Errno::clear` are + no longer `unsafe`. +- `SockAddr::as_ffi_pair`,`sys::socket::sockaddr_storage_to_addr`, `offset_of!`, + and `Errno::clear` are no longer `unsafe`. + (#[1244](https://github.com/nix-rust/nix/pull/1244)) +- Several `Inotify` methods now take `self` by value instead of by reference + (#[1244](https://github.com/nix-rust/nix/pull/1244)) ### Fixed @@ -61,6 +72,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). correctness's sake across all architectures and compilers, though now bugs have been reported so far. (#[1243](https://github.com/nix-rust/nix/pull/1243)) +- Fixed unaligned pointer read in `Inotify::read_events`. + (#[1244](https://github.com/nix-rust/nix/pull/1244)) ### Removed diff --git a/src/errno.rs b/src/errno.rs index 1fb66ac5f7..eda097d883 100644 --- a/src/errno.rs +++ b/src/errno.rs @@ -46,8 +46,11 @@ cfg_if! { } /// Sets the platform-specific errno to no-error -unsafe fn clear() { - *errno_location() = 0; +fn clear() { + // Safe because errno is a thread-local variable + unsafe { + *errno_location() = 0; + } } /// Returns the platform-specific value of errno @@ -70,7 +73,7 @@ impl Errno { from_i32(err) } - pub unsafe fn clear() { + pub fn clear() { clear() } diff --git a/src/fcntl.rs b/src/fcntl.rs index ea0364270c..1581d3a702 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -161,6 +161,8 @@ libc_bitflags!( } ); +// The conversion is not identical on all operating systems. +#[allow(clippy::identity_conversion)] pub fn open(path: &P, oflag: OFlag, mode: Mode) -> Result { let fd = path.with_nix_path(|cstr| { unsafe { libc::open(cstr.as_ptr(), oflag.bits(), mode.bits() as c_uint) } @@ -169,6 +171,8 @@ pub fn open(path: &P, oflag: OFlag, mode: Mode) -> Result( dirfd: RawFd, diff --git a/src/macros.rs b/src/macros.rs index 8b08b7d910..feb02ea745 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -208,7 +208,11 @@ macro_rules! libc_enum { /// offset of `field` within struct `ty` #[cfg(not(target_os = "redox"))] macro_rules! offset_of { - ($ty:ty, $field:ident) => { - &(*(ptr::null() as *const $ty)).$field as *const _ as usize - } + ($ty:ty, $field:ident) => {{ + // Safe because we don't actually read from the dereferenced pointer + #[allow(unused_unsafe)] // for when the macro is used in an unsafe block + unsafe { + &(*(ptr::null() as *const $ty)).$field as *const _ as usize + } + }} } diff --git a/src/sched.rs b/src/sched.rs index 064deb8c0c..3b48b4adf6 100644 --- a/src/sched.rs +++ b/src/sched.rs @@ -80,7 +80,8 @@ mod sched_linux_like { if field >= CpuSet::count() { Err(Error::Sys(Errno::EINVAL)) } else { - Ok(unsafe { libc::CPU_SET(field, &mut self.cpu_set) }) + unsafe { libc::CPU_SET(field, &mut self.cpu_set); } + Ok(()) } } @@ -90,7 +91,8 @@ mod sched_linux_like { if field >= CpuSet::count() { Err(Error::Sys(Errno::EINVAL)) } else { - Ok(unsafe { libc::CPU_CLR(field, &mut self.cpu_set) }) + unsafe { libc::CPU_CLR(field, &mut self.cpu_set);} + Ok(()) } } @@ -100,6 +102,12 @@ mod sched_linux_like { } } + impl Default for CpuSet { + fn default() -> Self { + Self::new() + } + } + /// `sched_setaffinity` set a thread's CPU affinity mask /// ([`sched_setaffinity(2)`](http://man7.org/linux/man-pages/man2/sched_setaffinity.2.html)) /// @@ -181,8 +189,8 @@ mod sched_linux_like { let res = unsafe { let combined = flags.bits() | signal.unwrap_or(0); - let ptr = stack.as_mut_ptr().offset(stack.len() as isize); - let ptr_aligned = ptr.offset((ptr as usize % 16) as isize * -1); + let ptr = stack.as_mut_ptr().add(stack.len()); + let ptr_aligned = ptr.sub(ptr as usize % 16); libc::clone( mem::transmute( callback as extern "C" fn(*mut Box isize>) -> i32, diff --git a/src/sys/inotify.rs b/src/sys/inotify.rs index 078015b6fd..4880a4a514 100644 --- a/src/sys/inotify.rs +++ b/src/sys/inotify.rs @@ -29,8 +29,9 @@ use libc::{ }; use std::ffi::{OsString,OsStr,CStr}; use std::os::unix::ffi::OsStrExt; -use std::mem::size_of; +use std::mem::{MaybeUninit, size_of}; use std::os::unix::io::{RawFd,AsRawFd,FromRawFd}; +use std::ptr; use crate::unistd::read; use crate::Result; use crate::NixPath; @@ -130,7 +131,7 @@ impl Inotify { /// Returns a watch descriptor. This is not a File Descriptor! /// /// For more information see, [inotify_add_watch(2)](http://man7.org/linux/man-pages/man2/inotify_add_watch.2.html). - pub fn add_watch(&self, + pub fn add_watch(self, path: &P, mask: AddWatchFlags) -> Result @@ -151,14 +152,14 @@ impl Inotify { /// /// For more information see, [inotify_rm_watch(2)](http://man7.org/linux/man-pages/man2/inotify_rm_watch.2.html). #[cfg(target_os = "linux")] - pub fn rm_watch(&self, wd: WatchDescriptor) -> Result<()> { + pub fn rm_watch(self, wd: WatchDescriptor) -> Result<()> { let res = unsafe { libc::inotify_rm_watch(self.fd, wd.wd) }; Errno::result(res).map(drop) } #[cfg(target_os = "android")] - pub fn rm_watch(&self, wd: WatchDescriptor) -> Result<()> { + pub fn rm_watch(self, wd: WatchDescriptor) -> Result<()> { let res = unsafe { libc::inotify_rm_watch(self.fd, wd.wd as u32) }; Errno::result(res).map(drop) @@ -170,9 +171,10 @@ impl Inotify { /// /// Returns as many events as available. If the call was non blocking and no /// events could be read then the EAGAIN error is returned. - pub fn read_events(&self) -> Result> { + pub fn read_events(self) -> Result> { let header_size = size_of::(); - let mut buffer = [0u8; 4096]; + const BUFSIZ: usize = 4096; + let mut buffer = [0u8; BUFSIZ]; let mut events = Vec::new(); let mut offset = 0; @@ -180,11 +182,13 @@ impl Inotify { while (nread - offset) >= header_size { let event = unsafe { - &*( - buffer - .as_ptr() - .offset(offset as isize) as *const libc::inotify_event - ) + let mut event = MaybeUninit::::uninit(); + ptr::copy_nonoverlapping( + buffer.as_ptr().add(offset), + event.as_mut_ptr() as *mut u8, + (BUFSIZ - offset).min(header_size) + ); + event.assume_init() }; let name = match event.len { @@ -193,7 +197,7 @@ impl Inotify { let ptr = unsafe { buffer .as_ptr() - .offset(offset as isize + header_size as isize) + .add(offset + header_size) as *const c_char }; let cstr = unsafe { CStr::from_ptr(ptr) }; diff --git a/src/sys/mman.rs b/src/sys/mman.rs index 5a5dd89e06..b2bed6e4ca 100644 --- a/src/sys/mman.rs +++ b/src/sys/mman.rs @@ -260,20 +260,37 @@ libc_bitflags!{ } } -/// Locks all memory pages that contain part of the address range with `length` bytes starting at -/// `addr`. Locked pages never move to the swap area. +/// Locks all memory pages that contain part of the address range with `length` +/// bytes starting at `addr`. +/// +/// Locked pages never move to the swap area. +/// +/// # Safety +/// +/// `addr` must meet all the requirements described in the `mlock(2)` man page. pub unsafe fn mlock(addr: *const c_void, length: size_t) -> Result<()> { Errno::result(libc::mlock(addr, length)).map(drop) } -/// Unlocks all memory pages that contain part of the address range with `length` bytes starting at -/// `addr`. +/// Unlocks all memory pages that contain part of the address range with +/// `length` bytes starting at `addr`. +/// +/// # Safety +/// +/// `addr` must meet all the requirements described in the `munlock(2)` man +/// page. pub unsafe fn munlock(addr: *const c_void, length: size_t) -> Result<()> { Errno::result(libc::munlock(addr, length)).map(drop) } -/// Locks all memory pages mapped into this process' address space. Locked pages never move to the -/// swap area. +/// Locks all memory pages mapped into this process' address space. +/// +/// Locked pages never move to the swap area. +/// +/// # Safety +/// +/// `addr` must meet all the requirements described in the `mlockall(2)` man +/// page. pub fn mlockall(flags: MlockAllFlags) -> Result<()> { unsafe { Errno::result(libc::mlockall(flags.bits())) }.map(drop) } @@ -283,8 +300,11 @@ pub fn munlockall() -> Result<()> { unsafe { Errno::result(libc::munlockall()) }.map(drop) } -/// Calls to mmap are inherently unsafe, so they must be made in an unsafe block. Typically -/// a higher-level abstraction will hide the unsafe interactions with the mmap'd region. +/// allocate memory, or map files or devices into memory +/// +/// # Safety +/// +/// See the `mmap(2)` man page for detailed requirements. pub unsafe fn mmap(addr: *mut c_void, length: size_t, prot: ProtFlags, flags: MapFlags, fd: RawFd, offset: off_t) -> Result<*mut c_void> { let ret = libc::mmap(addr, length, prot.bits(), flags.bits(), fd, offset); @@ -295,10 +315,22 @@ pub unsafe fn mmap(addr: *mut c_void, length: size_t, prot: ProtFlags, flags: Ma } } +/// remove a mapping +/// +/// # Safety +/// +/// `addr` must meet all the requirements described in the `munmap(2)` man +/// page. pub unsafe fn munmap(addr: *mut c_void, len: size_t) -> Result<()> { Errno::result(libc::munmap(addr, len)).map(drop) } +/// give advice about use of memory +/// +/// # Safety +/// +/// See the `madvise(2)` man page. Take special care when using +/// `MmapAdvise::MADV_FREE`. pub unsafe fn madvise(addr: *mut c_void, length: size_t, advise: MmapAdvise) -> Result<()> { Errno::result(libc::madvise(addr, length, advise as i32)).map(drop) } @@ -332,6 +364,12 @@ pub unsafe fn mprotect(addr: *mut c_void, length: size_t, prot: ProtFlags) -> Re Errno::result(libc::mprotect(addr, length, prot.bits())).map(drop) } +/// synchronize a mapped region +/// +/// # Safety +/// +/// `addr` must meet all the requirements described in the `msync(2)` man +/// page. pub unsafe fn msync(addr: *mut c_void, length: size_t, flags: MsFlags) -> Result<()> { Errno::result(libc::msync(addr, length, flags.bits())).map(drop) } diff --git a/src/sys/socket/addr.rs b/src/sys/socket/addr.rs index cdfa704deb..24b2310270 100644 --- a/src/sys/socket/addr.rs +++ b/src/sys/socket/addr.rs @@ -768,39 +768,60 @@ impl SockAddr { /// with the size of the actual data type. sockaddr is commonly used as a proxy for /// a superclass as C doesn't support inheritance, so many functions that take /// a sockaddr * need to take the size of the underlying type as well and then internally cast it back. - pub unsafe fn as_ffi_pair(&self) -> (&libc::sockaddr, libc::socklen_t) { + pub fn as_ffi_pair(&self) -> (&libc::sockaddr, libc::socklen_t) { match *self { SockAddr::Inet(InetAddr::V4(ref addr)) => ( - &*(addr as *const libc::sockaddr_in as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(addr as *const libc::sockaddr_in as *const libc::sockaddr) + }, mem::size_of_val(addr) as libc::socklen_t ), SockAddr::Inet(InetAddr::V6(ref addr)) => ( - &*(addr as *const libc::sockaddr_in6 as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(addr as *const libc::sockaddr_in6 as *const libc::sockaddr) + }, mem::size_of_val(addr) as libc::socklen_t ), SockAddr::Unix(UnixAddr(ref addr, len)) => ( - &*(addr as *const libc::sockaddr_un as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(addr as *const libc::sockaddr_un as *const libc::sockaddr) + }, (len + offset_of!(libc::sockaddr_un, sun_path)) as libc::socklen_t ), #[cfg(any(target_os = "android", target_os = "linux"))] SockAddr::Netlink(NetlinkAddr(ref sa)) => ( - &*(sa as *const libc::sockaddr_nl as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(sa as *const libc::sockaddr_nl as *const libc::sockaddr) + }, mem::size_of_val(sa) as libc::socklen_t ), #[cfg(any(target_os = "android", target_os = "linux"))] SockAddr::Alg(AlgAddr(ref sa)) => ( - &*(sa as *const libc::sockaddr_alg as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(sa as *const libc::sockaddr_alg as *const libc::sockaddr) + }, mem::size_of_val(sa) as libc::socklen_t ), #[cfg(any(target_os = "ios", target_os = "macos"))] SockAddr::SysControl(SysControlAddr(ref sa)) => ( - &*(sa as *const libc::sockaddr_ctl as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(sa as *const libc::sockaddr_ctl as *const libc::sockaddr) + }, mem::size_of_val(sa) as libc::socklen_t ), #[cfg(any(target_os = "android", target_os = "linux"))] SockAddr::Link(LinkAddr(ref addr)) => ( - &*(addr as *const libc::sockaddr_ll as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(addr as *const libc::sockaddr_ll as *const libc::sockaddr) + }, mem::size_of_val(addr) as libc::socklen_t ), #[cfg(any(target_os = "dragonfly", @@ -810,12 +831,18 @@ impl SockAddr { target_os = "netbsd", target_os = "openbsd"))] SockAddr::Link(LinkAddr(ref addr)) => ( - &*(addr as *const libc::sockaddr_dl as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(addr as *const libc::sockaddr_dl as *const libc::sockaddr) + }, mem::size_of_val(addr) as libc::socklen_t ), #[cfg(target_os = "linux")] SockAddr::Vsock(VsockAddr(ref sa)) => ( - &*(sa as *const libc::sockaddr_vm as *const libc::sockaddr), + // This cast is always allowed in C + unsafe { + &*(sa as *const libc::sockaddr_vm as *const libc::sockaddr) + }, mem::size_of_val(sa) as libc::socklen_t ), } diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs index b972835f89..aafa849cb8 100644 --- a/src/sys/socket/mod.rs +++ b/src/sys/socket/mod.rs @@ -221,6 +221,12 @@ cfg_if! { } } + impl Default for UnixCredentials { + fn default() -> Self { + Self::new() + } + } + impl From for UnixCredentials { fn from(cred: libc::ucred) -> Self { UnixCredentials(cred) @@ -1245,7 +1251,7 @@ fn pack_mhdr_to_send<'a, I, C>( // Next encode the sending address, if provided let (name, namelen) = match addr { Some(addr) => { - let (x, y) = unsafe { addr.as_ffi_pair() }; + let (x, y) = addr.as_ffi_pair(); (x as *const _, y) }, None => (ptr::null(), 0), @@ -1606,16 +1612,19 @@ pub fn getsockname(fd: RawFd) -> Result { } } -/// Return the appropriate `SockAddr` type from a `sockaddr_storage` of a certain -/// size. In C this would usually be done by casting. The `len` argument +/// Return the appropriate `SockAddr` type from a `sockaddr_storage` of a +/// certain size. +/// +/// In C this would usually be done by casting. The `len` argument /// should be the number of bytes in the `sockaddr_storage` that are actually /// allocated and valid. It must be at least as large as all the useful parts /// of the structure. Note that in the case of a `sockaddr_un`, `len` need not /// include the terminating null. -pub unsafe fn sockaddr_storage_to_addr( +pub fn sockaddr_storage_to_addr( addr: &sockaddr_storage, len: usize) -> Result { + assert!(len <= mem::size_of::()); if len < mem::size_of_val(&addr.ss_family) { return Err(Error::Sys(Errno::ENOTCONN)); } @@ -1623,32 +1632,48 @@ pub unsafe fn sockaddr_storage_to_addr( match c_int::from(addr.ss_family) { libc::AF_INET => { assert_eq!(len as usize, mem::size_of::()); - let ret = *(addr as *const _ as *const sockaddr_in); - Ok(SockAddr::Inet(InetAddr::V4(ret))) + let sin = unsafe { + *(addr as *const sockaddr_storage as *const sockaddr_in) + }; + Ok(SockAddr::Inet(InetAddr::V4(sin))) } libc::AF_INET6 => { assert_eq!(len as usize, mem::size_of::()); - Ok(SockAddr::Inet(InetAddr::V6(*(addr as *const _ as *const sockaddr_in6)))) + let sin6 = unsafe { + *(addr as *const _ as *const sockaddr_in6) + }; + Ok(SockAddr::Inet(InetAddr::V6(sin6))) } libc::AF_UNIX => { - let sun = *(addr as *const _ as *const sockaddr_un); let pathlen = len - offset_of!(sockaddr_un, sun_path); + let sun = unsafe { + *(addr as *const _ as *const sockaddr_un) + }; Ok(SockAddr::Unix(UnixAddr(sun, pathlen))) } #[cfg(any(target_os = "android", target_os = "linux"))] libc::AF_NETLINK => { use libc::sockaddr_nl; - Ok(SockAddr::Netlink(NetlinkAddr(*(addr as *const _ as *const sockaddr_nl)))) + let snl = unsafe { + *(addr as *const _ as *const sockaddr_nl) + }; + Ok(SockAddr::Netlink(NetlinkAddr(snl))) } #[cfg(any(target_os = "android", target_os = "linux"))] libc::AF_ALG => { use libc::sockaddr_alg; - Ok(SockAddr::Alg(AlgAddr(*(addr as *const _ as *const sockaddr_alg)))) + let salg = unsafe { + *(addr as *const _ as *const sockaddr_alg) + }; + Ok(SockAddr::Alg(AlgAddr(salg))) } #[cfg(target_os = "linux")] libc::AF_VSOCK => { use libc::sockaddr_vm; - Ok(SockAddr::Vsock(VsockAddr(*(addr as *const _ as *const sockaddr_vm)))) + let svm = unsafe { + *(addr as *const _ as *const sockaddr_vm) + }; + Ok(SockAddr::Vsock(VsockAddr(svm))) } af => panic!("unexpected address family {}", af), } diff --git a/src/ucontext.rs b/src/ucontext.rs index 9c1e8d2ace..a5b8cc75cb 100644 --- a/src/ucontext.rs +++ b/src/ucontext.rs @@ -3,6 +3,7 @@ use libc; use crate::Result; #[cfg(not(target_env = "musl"))] use crate::errno::Errno; +#[cfg(not(target_env = "musl"))] use std::mem; use crate::sys::signal::SigSet; @@ -30,10 +31,14 @@ impl UContext { } pub fn sigmask_mut(&mut self) -> &mut SigSet { - unsafe { mem::transmute(&mut self.context.uc_sigmask) } + unsafe { + &mut *(&mut self.context.uc_sigmask as *mut libc::sigset_t as *mut SigSet) + } } pub fn sigmask(&self) -> &SigSet { - unsafe { mem::transmute(&self.context.uc_sigmask) } + unsafe { + &*(&self.context.uc_sigmask as *const libc::sigset_t as *const SigSet) + } } } diff --git a/src/unistd.rs b/src/unistd.rs index 375ae82e3c..f2902bb45a 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -1465,9 +1465,7 @@ pub fn getgroups() -> Result> { /// # Ok(()) /// # } /// # -/// # fn main() { -/// # try_main().unwrap(); -/// # } +/// # try_main().unwrap(); /// ``` #[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "redox")))] pub fn setgroups(groups: &[Gid]) -> Result<()> { @@ -1589,9 +1587,7 @@ pub fn getgrouplist(user: &CStr, group: Gid) -> Result> { /// # Ok(()) /// # } /// # -/// # fn main() { -/// # try_main().unwrap(); -/// # } +/// # try_main().unwrap(); /// ``` #[cfg(not(any(target_os = "ios", target_os = "macos", target_os = "redox")))] pub fn initgroups(user: &CStr, group: Gid) -> Result<()> {