From f3bb34b1088f6dc8f07f37942c1e62b40010f955 Mon Sep 17 00:00:00 2001 From: Chris Denton Date: Mon, 15 Jul 2024 13:38:23 +0000 Subject: [PATCH] Deny more windows unsafe_op_in_unsafe_fn --- std/src/sys/os_str/wtf8.rs | 5 ++- std/src/sys/pal/windows/alloc.rs | 2 - std/src/sys/pal/windows/fs.rs | 23 ++++++----- std/src/sys/pal/windows/handle.rs | 63 +++++++++++++++++-------------- std/src/sys/pal/windows/os.rs | 17 ++++++--- 5 files changed, 63 insertions(+), 47 deletions(-) diff --git a/std/src/sys/os_str/wtf8.rs b/std/src/sys/os_str/wtf8.rs index edb923a47501c..cc00fcaf13d8b 100644 --- a/std/src/sys/os_str/wtf8.rs +++ b/std/src/sys/os_str/wtf8.rs @@ -1,5 +1,6 @@ //! The underlying OsString/OsStr implementation on Windows is a //! wrapper around the "WTF-8" encoding; see the `wtf8` module for more. +#![deny(unsafe_op_in_unsafe_fn)] use crate::borrow::Cow; use crate::collections::TryReserveError; @@ -71,7 +72,7 @@ impl Buf { #[inline] pub unsafe fn from_encoded_bytes_unchecked(s: Vec) -> Self { - Self { inner: Wtf8Buf::from_bytes_unchecked(s) } + unsafe { Self { inner: Wtf8Buf::from_bytes_unchecked(s) } } } pub fn with_capacity(capacity: usize) -> Buf { @@ -190,7 +191,7 @@ impl Slice { #[inline] pub unsafe fn from_encoded_bytes_unchecked(s: &[u8]) -> &Slice { - mem::transmute(Wtf8::from_bytes_unchecked(s)) + unsafe { mem::transmute(Wtf8::from_bytes_unchecked(s)) } } #[track_caller] diff --git a/std/src/sys/pal/windows/alloc.rs b/std/src/sys/pal/windows/alloc.rs index 9f0194492b0a3..987be6b69eec9 100644 --- a/std/src/sys/pal/windows/alloc.rs +++ b/std/src/sys/pal/windows/alloc.rs @@ -1,5 +1,3 @@ -#![deny(unsafe_op_in_unsafe_fn)] - use crate::alloc::{GlobalAlloc, Layout, System}; use crate::ffi::c_void; use crate::ptr; diff --git a/std/src/sys/pal/windows/fs.rs b/std/src/sys/pal/windows/fs.rs index 85fd9153d5370..48c39392047f0 100644 --- a/std/src/sys/pal/windows/fs.rs +++ b/std/src/sys/pal/windows/fs.rs @@ -1,4 +1,3 @@ -#![allow(unsafe_op_in_unsafe_fn)] use core::ptr::addr_of; use crate::os::windows::prelude::*; @@ -795,10 +794,12 @@ impl<'a> Iterator for DirBuffIter<'a> { } unsafe fn from_maybe_unaligned<'a>(p: *const u16, len: usize) -> Cow<'a, [u16]> { - if p.is_aligned() { - Cow::Borrowed(crate::slice::from_raw_parts(p, len)) - } else { - Cow::Owned((0..len).map(|i| p.add(i).read_unaligned()).collect()) + unsafe { + if p.is_aligned() { + Cow::Borrowed(crate::slice::from_raw_parts(p, len)) + } else { + Cow::Owned((0..len).map(|i| p.add(i).read_unaligned()).collect()) + } } } @@ -897,7 +898,9 @@ impl IntoRawHandle for File { impl FromRawHandle for File { unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { - Self { handle: FromInner::from_inner(FromRawHandle::from_raw_handle(raw_handle)) } + unsafe { + Self { handle: FromInner::from_inner(FromRawHandle::from_raw_handle(raw_handle)) } + } } } @@ -1427,10 +1430,12 @@ pub fn copy(from: &Path, to: &Path) -> io::Result { _hDestinationFile: c::HANDLE, lpData: *const c_void, ) -> u32 { - if dwStreamNumber == 1 { - *(lpData as *mut i64) = StreamBytesTransferred; + unsafe { + if dwStreamNumber == 1 { + *(lpData as *mut i64) = StreamBytesTransferred; + } + c::PROGRESS_CONTINUE } - c::PROGRESS_CONTINUE } let pfrom = maybe_verbatim(from)?; let pto = maybe_verbatim(to)?; diff --git a/std/src/sys/pal/windows/handle.rs b/std/src/sys/pal/windows/handle.rs index ae9ea8ff584ef..e63f5c9dec298 100644 --- a/std/src/sys/pal/windows/handle.rs +++ b/std/src/sys/pal/windows/handle.rs @@ -1,5 +1,4 @@ #![unstable(issue = "none", feature = "windows_handle")] -#![allow(unsafe_op_in_unsafe_fn)] #[cfg(test)] mod tests; @@ -73,7 +72,7 @@ impl IntoRawHandle for Handle { impl FromRawHandle for Handle { unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { - Self(FromRawHandle::from_raw_handle(raw_handle)) + unsafe { Self(FromRawHandle::from_raw_handle(raw_handle)) } } } @@ -142,19 +141,23 @@ impl Handle { buf: &mut [u8], overlapped: *mut c::OVERLAPPED, ) -> io::Result> { - let len = cmp::min(buf.len(), u32::MAX as usize) as u32; - let mut amt = 0; - let res = - cvt(c::ReadFile(self.as_raw_handle(), buf.as_mut_ptr(), len, &mut amt, overlapped)); - match res { - Ok(_) => Ok(Some(amt as usize)), - Err(e) => { - if e.raw_os_error() == Some(c::ERROR_IO_PENDING as i32) { - Ok(None) - } else if e.raw_os_error() == Some(c::ERROR_BROKEN_PIPE as i32) { - Ok(Some(0)) - } else { - Err(e) + // SAFETY: We have exclusive access to the buffer and it's up to the caller to + // ensure the OVERLAPPED pointer is valid for the lifetime of this function. + unsafe { + let len = cmp::min(buf.len(), u32::MAX as usize) as u32; + let mut amt = 0; + let res = + cvt(c::ReadFile(self.as_raw_handle(), buf.as_mut_ptr(), len, &mut amt, overlapped)); + match res { + Ok(_) => Ok(Some(amt as usize)), + Err(e) => { + if e.raw_os_error() == Some(c::ERROR_IO_PENDING as i32) { + Ok(None) + } else if e.raw_os_error() == Some(c::ERROR_BROKEN_PIPE as i32) { + Ok(Some(0)) + } else { + Err(e) + } } } } @@ -230,20 +233,24 @@ impl Handle { // The length is clamped at u32::MAX. let len = cmp::min(len, u32::MAX as usize) as u32; - let status = c::NtReadFile( - self.as_handle(), - ptr::null_mut(), - None, - ptr::null_mut(), - &mut io_status, - buf, - len, - offset.map(|n| n as _).as_ref(), - None, - ); + // SAFETY: It's up to the caller to ensure `buf` is writeable up to + // the provided `len`. + let status = unsafe { + c::NtReadFile( + self.as_handle(), + ptr::null_mut(), + None, + ptr::null_mut(), + &mut io_status, + buf, + len, + offset.map(|n| n as _).as_ref(), + None, + ) + }; let status = if status == c::STATUS_PENDING { - c::WaitForSingleObject(self.as_raw_handle(), c::INFINITE); + unsafe { c::WaitForSingleObject(self.as_raw_handle(), c::INFINITE) }; io_status.status() } else { status @@ -261,7 +268,7 @@ impl Handle { status if c::nt_success(status) => Ok(io_status.Information), status => { - let error = c::RtlNtStatusToDosError(status); + let error = unsafe { c::RtlNtStatusToDosError(status) }; Err(io::Error::from_raw_os_error(error as _)) } } diff --git a/std/src/sys/pal/windows/os.rs b/std/src/sys/pal/windows/os.rs index 0a9279e50ba15..f1f4d3a5d26ef 100644 --- a/std/src/sys/pal/windows/os.rs +++ b/std/src/sys/pal/windows/os.rs @@ -1,7 +1,6 @@ //! Implementation of `std::os` functionality for Windows. #![allow(nonstandard_style)] -#![allow(unsafe_op_in_unsafe_fn)] #[cfg(test)] mod tests; @@ -305,15 +304,21 @@ pub fn getenv(k: &OsStr) -> Option { } pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> { - let k = to_u16s(k)?; - let v = to_u16s(v)?; + // SAFETY: We ensure that k and v are null-terminated wide strings. + unsafe { + let k = to_u16s(k)?; + let v = to_u16s(v)?; - cvt(c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr())).map(drop) + cvt(c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr())).map(drop) + } } pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> { - let v = to_u16s(n)?; - cvt(c::SetEnvironmentVariableW(v.as_ptr(), ptr::null())).map(drop) + // SAFETY: We ensure that v is a null-terminated wide strings. + unsafe { + let v = to_u16s(n)?; + cvt(c::SetEnvironmentVariableW(v.as_ptr(), ptr::null())).map(drop) + } } pub fn temp_dir() -> PathBuf {