diff --git a/changelog/2000.changed.md b/changelog/2000.changed.md new file mode 100644 index 0000000000..613e9dfef7 --- /dev/null +++ b/changelog/2000.changed.md @@ -0,0 +1 @@ +`mmap`, `mmap_anonymous`, `munmap`, `mremap`, `madvise`, `msync`, `mprotect`, `munlock` and `mlock` updated to use `NonNull`. \ No newline at end of file diff --git a/src/sys/mman.rs b/src/sys/mman.rs index c2299e9648..1e4536ae8c 100644 --- a/src/sys/mman.rs +++ b/src/sys/mman.rs @@ -8,10 +8,8 @@ use crate::Result; #[cfg(feature = "fs")] use crate::{fcntl::OFlag, sys::stat::Mode}; use libc::{self, c_int, c_void, off_t, size_t}; -use std::{ - num::NonZeroUsize, - os::unix::io::{AsFd, AsRawFd}, -}; +use std::{num::NonZeroUsize, os::unix::io::{AsRawFd, AsFd}}; +use std::ptr::NonNull; libc_bitflags! { /// Desired memory protection of a memory mapping. @@ -377,8 +375,8 @@ libc_bitflags! { /// `addr` must meet all the requirements described in the [`mlock(2)`] man page. /// /// [`mlock(2)`]: https://man7.org/linux/man-pages/man2/mlock.2.html -pub unsafe fn mlock(addr: *const c_void, length: size_t) -> Result<()> { - Errno::result(libc::mlock(addr, length)).map(drop) +pub unsafe fn mlock(addr: NonNull, length: size_t) -> Result<()> { + Errno::result(libc::mlock(addr.as_ptr(), length)).map(drop) } /// Unlocks all memory pages that contain part of the address range with @@ -390,8 +388,8 @@ pub unsafe fn mlock(addr: *const c_void, length: size_t) -> Result<()> { /// page. /// /// [`munlock(2)`]: https://man7.org/linux/man-pages/man2/munlock.2.html -pub unsafe fn munlock(addr: *const c_void, length: size_t) -> Result<()> { - Errno::result(libc::munlock(addr, length)).map(drop) +pub unsafe fn munlock(addr: NonNull, length: size_t) -> Result<()> { + Errno::result(libc::munlock(addr.as_ptr(), length)).map(drop) } /// Locks all memory pages mapped into this process' address space. @@ -430,7 +428,7 @@ pub unsafe fn mmap( flags: MapFlags, f: F, offset: off_t, -) -> Result<*mut c_void> { +) -> Result> { let ptr = addr.map_or(std::ptr::null_mut(), |a| a.get() as *mut c_void); let fd = f.as_fd().as_raw_fd(); @@ -439,9 +437,12 @@ pub unsafe fn mmap( if ret == libc::MAP_FAILED { Err(Errno::last()) - } else { - Ok(ret) } + else { + // SAFETY: `libc::mmap` returns a valid non-null pointer or `libc::MAP_FAILED`, thus `ret` + // will be non-null here. + Ok(unsafe { NonNull::new_unchecked(ret) }) + } } /// Create an anonymous memory mapping. @@ -459,7 +460,7 @@ pub unsafe fn mmap_anonymous( length: NonZeroUsize, prot: ProtFlags, flags: MapFlags, -) -> Result<*mut c_void> { +) -> Result> { let ptr = addr.map_or(std::ptr::null_mut(), |a| a.get() as *mut c_void); let flags = MapFlags::MAP_ANONYMOUS | flags; @@ -468,7 +469,9 @@ pub unsafe fn mmap_anonymous( if ret == libc::MAP_FAILED { Err(Errno::last()) } else { - Ok(ret) + // SAFETY: `libc::mmap` returns a valid non-null pointer or `libc::MAP_FAILED`, thus `ret` + // will be non-null here. + Ok(unsafe { NonNull::new_unchecked(ret) }) } } @@ -481,25 +484,25 @@ pub unsafe fn mmap_anonymous( /// detailed requirements. #[cfg(any(target_os = "linux", target_os = "netbsd"))] pub unsafe fn mremap( - addr: *mut c_void, + addr: NonNull, old_size: size_t, new_size: size_t, flags: MRemapFlags, - new_address: Option<*mut c_void>, -) -> Result<*mut c_void> { + new_address: Option>, +) -> Result> { #[cfg(target_os = "linux")] let ret = libc::mremap( - addr, + addr.as_ptr(), old_size, new_size, flags.bits(), - new_address.unwrap_or(std::ptr::null_mut()), + new_address.map(NonNull::as_ptr).unwrap_or(std::ptr::null_mut()), ); #[cfg(target_os = "netbsd")] let ret = libc::mremap( - addr, + addr.as_ptr(), old_size, - new_address.unwrap_or(std::ptr::null_mut()), + new_address.map(NonNull::as_ptr).unwrap_or(std::ptr::null_mut()), new_size, flags.bits(), ); @@ -507,7 +510,9 @@ pub unsafe fn mremap( if ret == libc::MAP_FAILED { Err(Errno::last()) } else { - Ok(ret) + // SAFETY: `libc::mremap` returns a valid non-null pointer or `libc::MAP_FAILED`, thus `ret` + // will be non-null here. + Ok(unsafe { NonNull::new_unchecked(ret) }) } } @@ -519,8 +524,8 @@ pub unsafe fn mremap( /// page. /// /// [`munmap(2)`]: https://man7.org/linux/man-pages/man2/munmap.2.html -pub unsafe fn munmap(addr: *mut c_void, len: size_t) -> Result<()> { - Errno::result(libc::munmap(addr, len)).map(drop) +pub unsafe fn munmap(addr: NonNull, len: size_t) -> Result<()> { + Errno::result(libc::munmap(addr.as_ptr(), len)).map(drop) } /// give advice about use of memory @@ -532,11 +537,11 @@ pub unsafe fn munmap(addr: *mut c_void, len: size_t) -> Result<()> { /// /// [`madvise(2)`]: https://man7.org/linux/man-pages/man2/madvise.2.html pub unsafe fn madvise( - addr: *mut c_void, + addr: NonNull, length: size_t, advise: MmapAdvise, ) -> Result<()> { - Errno::result(libc::madvise(addr, length, advise as i32)).map(drop) + Errno::result(libc::madvise(addr.as_ptr(), length, advise as i32)).map(drop) } /// Set protection of memory mapping. @@ -560,18 +565,18 @@ pub unsafe fn madvise( /// let mem = mmap_anonymous(None, one_k_non_zero, ProtFlags::PROT_NONE, MapFlags::MAP_PRIVATE) /// .unwrap(); /// mprotect(mem, ONE_K, ProtFlags::PROT_READ | ProtFlags::PROT_WRITE).unwrap(); -/// std::slice::from_raw_parts_mut(mem as *mut u8, ONE_K) +/// std::slice::from_raw_parts_mut(mem.as_ptr().cast(), ONE_K) /// }; /// assert_eq!(slice[0], 0x00); /// slice[0] = 0xFF; /// assert_eq!(slice[0], 0xFF); /// ``` pub unsafe fn mprotect( - addr: *mut c_void, + addr: NonNull, length: size_t, prot: ProtFlags, ) -> Result<()> { - Errno::result(libc::mprotect(addr, length, prot.bits())).map(drop) + Errno::result(libc::mprotect(addr.as_ptr(), length, prot.bits())).map(drop) } /// synchronize a mapped region @@ -583,11 +588,11 @@ pub unsafe fn mprotect( /// /// [`msync(2)`]: https://man7.org/linux/man-pages/man2/msync.2.html pub unsafe fn msync( - addr: *mut c_void, + addr: NonNull, length: size_t, flags: MsFlags, ) -> Result<()> { - Errno::result(libc::msync(addr, length, flags.bits())).map(drop) + Errno::result(libc::msync(addr.as_ptr(), length, flags.bits())).map(drop) } #[cfg(not(target_os = "android"))] diff --git a/test/sys/test_mman.rs b/test/sys/test_mman.rs index 585b6d6363..3689f642be 100644 --- a/test/sys/test_mman.rs +++ b/test/sys/test_mman.rs @@ -1,19 +1,22 @@ +#![allow(clippy::redundant_slicing)] + use nix::sys::mman::{mmap_anonymous, MapFlags, ProtFlags}; use std::num::NonZeroUsize; #[test] fn test_mmap_anonymous() { unsafe { - let ptr = mmap_anonymous( + let mut ptr = mmap_anonymous( None, NonZeroUsize::new(1).unwrap(), ProtFlags::PROT_READ | ProtFlags::PROT_WRITE, MapFlags::MAP_PRIVATE, ) - .unwrap() as *mut u8; - assert_eq!(*ptr, 0x00u8); - *ptr = 0xffu8; - assert_eq!(*ptr, 0xffu8); + .unwrap() + .cast::(); + assert_eq!(*ptr.as_ref(), 0x00u8); + *ptr.as_mut() = 0xffu8; + assert_eq!(*ptr.as_ref(), 0xffu8); } } @@ -22,6 +25,7 @@ fn test_mmap_anonymous() { fn test_mremap_grow() { use nix::libc::size_t; use nix::sys::mman::{mremap, MRemapFlags}; + use std::ptr::NonNull; const ONE_K: size_t = 1024; let one_k_non_zero = NonZeroUsize::new(ONE_K).unwrap(); @@ -34,7 +38,7 @@ fn test_mremap_grow() { MapFlags::MAP_PRIVATE, ) .unwrap(); - std::slice::from_raw_parts_mut(mem as *mut u8, ONE_K) + std::slice::from_raw_parts_mut(mem.as_ptr().cast(), ONE_K) }; assert_eq!(slice[ONE_K - 1], 0x00); slice[ONE_K - 1] = 0xFF; @@ -43,7 +47,7 @@ fn test_mremap_grow() { let slice: &mut [u8] = unsafe { #[cfg(target_os = "linux")] let mem = mremap( - slice.as_mut_ptr().cast(), + NonNull::from(&mut slice[..]).cast(), ONE_K, 10 * ONE_K, MRemapFlags::MREMAP_MAYMOVE, @@ -52,14 +56,14 @@ fn test_mremap_grow() { .unwrap(); #[cfg(target_os = "netbsd")] let mem = mremap( - slice.as_mut_ptr().cast(), + NonNull::from(&mut slice[..]).cast(), ONE_K, 10 * ONE_K, MRemapFlags::MAP_REMAPDUP, None, ) .unwrap(); - std::slice::from_raw_parts_mut(mem as *mut u8, 10 * ONE_K) + std::slice::from_raw_parts_mut(mem.cast().as_ptr(), 10 * ONE_K) }; // The first KB should still have the old data in it. @@ -79,6 +83,7 @@ fn test_mremap_shrink() { use nix::libc::size_t; use nix::sys::mman::{mremap, MRemapFlags}; use std::num::NonZeroUsize; + use std::ptr::NonNull; const ONE_K: size_t = 1024; let ten_one_k = NonZeroUsize::new(10 * ONE_K).unwrap(); @@ -90,7 +95,7 @@ fn test_mremap_shrink() { MapFlags::MAP_PRIVATE, ) .unwrap(); - std::slice::from_raw_parts_mut(mem as *mut u8, ONE_K) + std::slice::from_raw_parts_mut(mem.as_ptr().cast(), ONE_K) }; assert_eq!(slice[ONE_K - 1], 0x00); slice[ONE_K - 1] = 0xFF; @@ -98,7 +103,7 @@ fn test_mremap_shrink() { let slice: &mut [u8] = unsafe { let mem = mremap( - slice.as_mut_ptr().cast(), + NonNull::from(&mut slice[..]).cast(), ten_one_k.into(), ONE_K, MRemapFlags::empty(), @@ -107,8 +112,8 @@ fn test_mremap_shrink() { .unwrap(); // Since we didn't supply MREMAP_MAYMOVE, the address should be the // same. - assert_eq!(mem, slice.as_mut_ptr().cast()); - std::slice::from_raw_parts_mut(mem as *mut u8, ONE_K) + assert_eq!(mem.as_ptr(), NonNull::from(&mut slice[..]).cast().as_ptr()); + std::slice::from_raw_parts_mut(mem.as_ptr().cast(), ONE_K) }; // The first KB should still be accessible and have the old data in it.