Fix some possible UB in std::sys::windows

This commit is contained in:
Thom Chiovoloni 2022-08-29 15:36:01 -07:00
parent fcc2bddd26
commit 1b8025a24c
No known key found for this signature in database
GPG Key ID: D7733D1D7A775F0A
4 changed files with 71 additions and 16 deletions

View File

@ -503,6 +503,8 @@ pub struct FILE_END_OF_FILE_INFO {
pub EndOfFile: LARGE_INTEGER, pub EndOfFile: LARGE_INTEGER,
} }
/// NB: Use carefully! In general using this as a reference is likely to get the
/// provenance wrong for the `rest` field!
#[repr(C)] #[repr(C)]
pub struct REPARSE_DATA_BUFFER { pub struct REPARSE_DATA_BUFFER {
pub ReparseTag: c_uint, pub ReparseTag: c_uint,
@ -511,6 +513,8 @@ pub struct REPARSE_DATA_BUFFER {
pub rest: (), pub rest: (),
} }
/// NB: Use carefully! In general using this as a reference is likely to get the
/// provenance wrong for the `PathBuffer` field!
#[repr(C)] #[repr(C)]
pub struct SYMBOLIC_LINK_REPARSE_BUFFER { pub struct SYMBOLIC_LINK_REPARSE_BUFFER {
pub SubstituteNameOffset: c_ushort, pub SubstituteNameOffset: c_ushort,
@ -521,6 +525,8 @@ pub struct SYMBOLIC_LINK_REPARSE_BUFFER {
pub PathBuffer: WCHAR, pub PathBuffer: WCHAR,
} }
/// NB: Use carefully! In general using this as a reference is likely to get the
/// provenance wrong for the `PathBuffer` field!
#[repr(C)] #[repr(C)]
pub struct MOUNT_POINT_REPARSE_BUFFER { pub struct MOUNT_POINT_REPARSE_BUFFER {
pub SubstituteNameOffset: c_ushort, pub SubstituteNameOffset: c_ushort,

View File

@ -11,7 +11,7 @@ use crate::slice;
use crate::sync::Arc; use crate::sync::Arc;
use crate::sys::handle::Handle; use crate::sys::handle::Handle;
use crate::sys::time::SystemTime; use crate::sys::time::SystemTime;
use crate::sys::{c, cvt}; use crate::sys::{c, cvt, AlignedAs};
use crate::sys_common::{AsInner, FromInner, IntoInner}; use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::thread; use crate::thread;
@ -47,6 +47,9 @@ pub struct ReadDir {
first: Option<c::WIN32_FIND_DATAW>, first: Option<c::WIN32_FIND_DATAW>,
} }
type AlignedReparseBuf =
AlignedAs<c::REPARSE_DATA_BUFFER, [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]>;
struct FindNextFileHandle(c::HANDLE); struct FindNextFileHandle(c::HANDLE);
unsafe impl Send for FindNextFileHandle {} unsafe impl Send for FindNextFileHandle {}
@ -326,9 +329,9 @@ impl File {
cvt(c::GetFileInformationByHandle(self.handle.as_raw_handle(), &mut info))?; cvt(c::GetFileInformationByHandle(self.handle.as_raw_handle(), &mut info))?;
let mut reparse_tag = 0; let mut reparse_tag = 0;
if info.dwFileAttributes & c::FILE_ATTRIBUTE_REPARSE_POINT != 0 { if info.dwFileAttributes & c::FILE_ATTRIBUTE_REPARSE_POINT != 0 {
let mut b = [0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]; let mut b = AlignedReparseBuf::new([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
if let Ok((_, buf)) = self.reparse_point(&mut b) { if let Ok((_, buf)) = self.reparse_point(&mut b) {
reparse_tag = buf.ReparseTag; reparse_tag = (*buf).ReparseTag;
} }
} }
Ok(FileAttr { Ok(FileAttr {
@ -389,7 +392,7 @@ impl File {
attr.file_size = info.AllocationSize as u64; attr.file_size = info.AllocationSize as u64;
attr.number_of_links = Some(info.NumberOfLinks); attr.number_of_links = Some(info.NumberOfLinks);
if attr.file_type().is_reparse_point() { if attr.file_type().is_reparse_point() {
let mut b = [0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]; let mut b = AlignedReparseBuf::new([0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
if let Ok((_, buf)) = self.reparse_point(&mut b) { if let Ok((_, buf)) = self.reparse_point(&mut b) {
attr.reparse_tag = buf.ReparseTag; attr.reparse_tag = buf.ReparseTag;
} }
@ -458,10 +461,13 @@ impl File {
Ok(Self { handle: self.handle.try_clone()? }) Ok(Self { handle: self.handle.try_clone()? })
} }
fn reparse_point<'a>( // NB: returned pointer is derived from `space`, and has provenance to
// match. A raw pointer is returned rather than a reference in order to
// avoid narrowing provenance to the actual `REPARSE_DATA_BUFFER`.
fn reparse_point(
&self, &self,
space: &'a mut [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE], space: &mut AlignedReparseBuf,
) -> io::Result<(c::DWORD, &'a c::REPARSE_DATA_BUFFER)> { ) -> io::Result<(c::DWORD, *const c::REPARSE_DATA_BUFFER)> {
unsafe { unsafe {
let mut bytes = 0; let mut bytes = 0;
cvt({ cvt({
@ -470,26 +476,27 @@ impl File {
c::FSCTL_GET_REPARSE_POINT, c::FSCTL_GET_REPARSE_POINT,
ptr::null_mut(), ptr::null_mut(),
0, 0,
space.as_mut_ptr() as *mut _, space.value.as_mut_ptr() as *mut _,
space.len() as c::DWORD, space.value.len() as c::DWORD,
&mut bytes, &mut bytes,
ptr::null_mut(), ptr::null_mut(),
) )
})?; })?;
Ok((bytes, &*(space.as_ptr() as *const c::REPARSE_DATA_BUFFER))) Ok((bytes, space.value.as_ptr().cast::<c::REPARSE_DATA_BUFFER>()))
} }
} }
fn readlink(&self) -> io::Result<PathBuf> { fn readlink(&self) -> io::Result<PathBuf> {
let mut space = [0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]; let mut space = AlignedReparseBuf::new([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
let (_bytes, buf) = self.reparse_point(&mut space)?; let (_bytes, buf) = self.reparse_point(&mut space)?;
unsafe { unsafe {
let (path_buffer, subst_off, subst_len, relative) = match buf.ReparseTag { let (path_buffer, subst_off, subst_len, relative) = match (*buf).ReparseTag {
c::IO_REPARSE_TAG_SYMLINK => { c::IO_REPARSE_TAG_SYMLINK => {
let info: *const c::SYMBOLIC_LINK_REPARSE_BUFFER = let info: *const c::SYMBOLIC_LINK_REPARSE_BUFFER =
&buf.rest as *const _ as *const _; ptr::addr_of!((*buf).rest).cast();
assert!(info.is_aligned());
( (
&(*info).PathBuffer as *const _ as *const u16, ptr::addr_of!((*info).PathBuffer).cast::<u16>(),
(*info).SubstituteNameOffset / 2, (*info).SubstituteNameOffset / 2,
(*info).SubstituteNameLength / 2, (*info).SubstituteNameLength / 2,
(*info).Flags & c::SYMLINK_FLAG_RELATIVE != 0, (*info).Flags & c::SYMLINK_FLAG_RELATIVE != 0,
@ -497,9 +504,10 @@ impl File {
} }
c::IO_REPARSE_TAG_MOUNT_POINT => { c::IO_REPARSE_TAG_MOUNT_POINT => {
let info: *const c::MOUNT_POINT_REPARSE_BUFFER = let info: *const c::MOUNT_POINT_REPARSE_BUFFER =
&buf.rest as *const _ as *const _; ptr::addr_of!((*buf).rest).cast();
assert!(info.is_aligned());
( (
&(*info).PathBuffer as *const _ as *const u16, ptr::addr_of!((*info).PathBuffer).cast::<u16>(),
(*info).SubstituteNameOffset / 2, (*info).SubstituteNameOffset / 2,
(*info).SubstituteNameLength / 2, (*info).SubstituteNameLength / 2,
false, false,

View File

@ -329,3 +329,26 @@ pub fn abort_internal() -> ! {
} }
crate::intrinsics::abort(); crate::intrinsics::abort();
} }
/// Used for some win32 buffers which are stack allocated, for example:
/// `AlignedAs<c::REPARSE_DATA_BUFFER, [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]>`
#[repr(C)]
#[derive(Copy, Clone)]
pub struct AlignedAs<Aligner, Alignee: ?Sized> {
/// Use `[Aligner; 0]` as a sort of `PhantomAlignNextField<Aligner>`. This
/// is a bit of a hack, and could break (in a way that's caught by tests) if
/// #81996 is fixed.
aligner: [Aligner; 0],
/// The aligned value. Public rather than exposed via accessors so that if
/// needed it can be used with `addr_of` and such (also, this is less code).
pub value: Alignee,
}
impl<Aligner, Alignee> AlignedAs<Aligner, Alignee> {
// This is frequently used with large stack buffers, so force-inline to
// try and avoid using 2x as much stack space in debug builds.
#[inline(always)]
pub const fn new(value: Alignee) -> Self {
Self { aligner: [], value }
}
}

View File

@ -11,3 +11,21 @@ fn ntstatus_error() {
.contains("FormatMessageW() returned error") .contains("FormatMessageW() returned error")
); );
} }
#[test]
fn smoketest_aligned_as() {
use crate::{
mem::{align_of, size_of},
ptr::addr_of,
sys::{c, AlignedAs},
};
type AlignedReparseBuf =
AlignedAs<c::REPARSE_DATA_BUFFER, [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]>;
assert!(size_of::<AlignedReparseBuf>() >= c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE);
assert_eq!(align_of::<AlignedReparseBuf>(), align_of::<c::REPARSE_DATA_BUFFER>());
let a = AlignedReparseBuf::new([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
// Quick and dirty offsetof check.
assert_eq!(addr_of!(a).cast::<u8>(), addr_of!(a.value).cast::<u8>());
// Smoke check that it's actually aligned.
assert!(addr_of!(a.value).is_aligned_to(align_of::<c::REPARSE_DATA_BUFFER>()));
}