nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-04-16 14:07:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

Rollup merge of #101171 - thomcc:fix-winfs-ub, r=ChrisDenton

Browse Source 
Fix UB from misalignment and provenance widening in `std::sys::windows`

This fixes two types of UB:

1. Reading past the end of a reference in types like `&c::REPARSE_DATA_BUFFER` (see https://github.com/rust-lang/unsafe-code-guidelines/issues/256). This is fixed by using `addr_of!`. I think there are probably a couple more cases where we do this for other structures, and will look into it in a bit.

2. Failing to ensure that a `[u8; N]` on the stack is sufficiently aligned to convert to a `REPARSE_DATA_BUFFER`. ~~This was done by introducing a new `AlignedAs` struct that allows aligning one type to the alignment of another type. I expect there are other places where we have this issue too, or I wouldn't introduce this type, but will get to them after this lands.~~

    ~~Worth noting, it *is* implemented in a way that can cause problems depending on how we fix #81996, but this would be caught by the test I added (and presumably if we decide to fix that in a way that would break this code, we'd also introduce a `#[repr(simple)]` or `#[repr(linear)]` as a replacement for this usage of `#[repr(C)]`).~~

    Edit: None of that is still in the code, I just went with a `Align8` since that's all we'll need for almost everything we want to call.

These are more or less "potential UB" since it's likely at the moment everything works fine, although the alignment not causing issues might just be down to luck (and x86 being forgiving).

~~NB: I've only ensured this check builds, but will run tests soon.~~ All tests pass, including stage2 compiler tests.

r? ``@ChrisDenton``
This commit is contained in:
Matthias Krüger 2022-08-31 07:58:01 +02:00 committed by GitHub
commit 0ed046f44d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 55 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
library/std/src/sys/windows/c.rs
View File

@ -501,6 +501,8 @@ pub struct FILE_END_OF_FILE_INFO {
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)]
pub struct REPARSE_DATA_BUFFER {
pub ReparseTag: c_uint,
@ -509,6 +511,8 @@ pub struct REPARSE_DATA_BUFFER {
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)]
pub struct SYMBOLIC_LINK_REPARSE_BUFFER {
pub SubstituteNameOffset: c_ushort,
@ -519,6 +523,8 @@ pub struct SYMBOLIC_LINK_REPARSE_BUFFER {
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)]
pub struct MOUNT_POINT_REPARSE_BUFFER {
pub SubstituteNameOffset: c_ushort,

69
library/std/src/sys/windows/fs.rs
View File

@ -11,7 +11,7 @@ use crate::slice;
use crate::sync::Arc;
use crate::sys::handle::Handle;
use crate::sys::time::SystemTime;
use crate::sys::{c, cvt};
use crate::sys::{c, cvt, Align8};
use crate::sys_common::{AsInner, FromInner, IntoInner};
use crate::thread;
@ -326,9 +326,9 @@ impl File {
cvt(c::GetFileInformationByHandle(self.handle.as_raw_handle(), &mut info))?;
let mut reparse_tag = 0;
if info.dwFileAttributes & c::FILE_ATTRIBUTE_REPARSE_POINT != 0 {
let mut b = [0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut b = Align8([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
if let Ok((_, buf)) = self.reparse_point(&mut b) {
reparse_tag = buf.ReparseTag;
reparse_tag = (*buf).ReparseTag;
}
}
Ok(FileAttr {
@ -389,9 +389,9 @@ impl File {
attr.file_size = info.AllocationSize as u64;
attr.number_of_links = Some(info.NumberOfLinks);
if attr.file_type().is_reparse_point() {
let mut b = [0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut b = Align8([0; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
if let Ok((_, buf)) = self.reparse_point(&mut b) {
attr.reparse_tag = buf.ReparseTag;
attr.reparse_tag = (*buf).ReparseTag;
}
}
Ok(attr)
@ -458,38 +458,46 @@ impl File {
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,
space: &'a mut [u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE],
) -> io::Result<(c::DWORD, &'a c::REPARSE_DATA_BUFFER)> {
space: &mut Align8<[u8]>,
) -> io::Result<(c::DWORD, *const c::REPARSE_DATA_BUFFER)> {
unsafe {
let mut bytes = 0;
cvt({
// Grab this in advance to avoid it invalidating the pointer
// we get from `space.0.as_mut_ptr()`.
let len = space.0.len();
c::DeviceIoControl(
self.handle.as_raw_handle(),
c::FSCTL_GET_REPARSE_POINT,
ptr::null_mut(),
0,
space.as_mut_ptr() as *mut _,
space.len() as c::DWORD,
space.0.as_mut_ptr().cast(),
len as c::DWORD,
&mut bytes,
ptr::null_mut(),
)
})?;
Ok((bytes, &*(space.as_ptr() as *const c::REPARSE_DATA_BUFFER)))
const _: () = assert!(core::mem::align_of::<c::REPARSE_DATA_BUFFER>() <= 8);
Ok((bytes, space.0.as_ptr().cast::<c::REPARSE_DATA_BUFFER>()))
}
}
fn readlink(&self) -> io::Result<PathBuf> {
let mut space = [0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let mut space = Align8([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
let (_bytes, buf) = self.reparse_point(&mut space)?;
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 => {
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).SubstituteNameLength / 2,
(*info).Flags & c::SYMLINK_FLAG_RELATIVE != 0,
@ -497,9 +505,10 @@ impl File {
}
c::IO_REPARSE_TAG_MOUNT_POINT => {
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).SubstituteNameLength / 2,
false,
@ -649,18 +658,18 @@ impl File {
/// A buffer for holding directory entries.
struct DirBuff {
buffer: Vec<u8>,
buffer: Box<Align8<[u8; Self::BUFFER_SIZE]>>,
}
impl DirBuff {
const BUFFER_SIZE: usize = 1024;
fn new() -> Self {
const BUFFER_SIZE: usize = 1024;
Self { buffer: vec![0_u8; BUFFER_SIZE] }
Self { buffer: Box::new(Align8([0u8; Self::BUFFER_SIZE])) }
}
fn capacity(&self) -> usize {
self.buffer.len()
self.buffer.0.len()
}
fn as_mut_ptr(&mut self) -> *mut u8 {
self.buffer.as_mut_ptr().cast()
self.buffer.0.as_mut_ptr().cast()
}
/// Returns a `DirBuffIter`.
fn iter(&self) -> DirBuffIter<'_> {
@ -669,7 +678,7 @@ impl DirBuff {
}
impl AsRef<[u8]> for DirBuff {
fn as_ref(&self) -> &[u8] {
&self.buffer
&self.buffer.0
}
}
@ -697,9 +706,12 @@ impl<'a> Iterator for DirBuffIter<'a> {
// used to get the file name slice.
let (name, is_directory, next_entry) = unsafe {
let info = buffer.as_ptr().cast::<c::FILE_ID_BOTH_DIR_INFO>();
// Guaranteed to be aligned in documentation for
// https://docs.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_both_dir_info
assert!(info.is_aligned());
let next_entry = (*info).NextEntryOffset as usize;
let name = crate::slice::from_raw_parts(
(*info).FileName.as_ptr().cast::<u16>(),
ptr::addr_of!((*info).FileName).cast::<u16>(),
(*info).FileNameLength as usize / size_of::<u16>(),
);
let is_directory = ((*info).FileAttributes & c::FILE_ATTRIBUTE_DIRECTORY) != 0;
@ -1337,9 +1349,10 @@ fn symlink_junction_inner(original: &Path, junction: &Path) -> io::Result<()> {
let h = f.as_inner().as_raw_handle();
unsafe {
let mut data = [0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
let db = data.as_mut_ptr() as *mut c::REPARSE_MOUNTPOINT_DATA_BUFFER;
let buf = &mut (*db).ReparseTarget as *mut c::WCHAR;
let mut data = Align8([0u8; c::MAXIMUM_REPARSE_DATA_BUFFER_SIZE]);
let data_ptr = data.0.as_mut_ptr();
let db = data_ptr.cast::<c::REPARSE_MOUNTPOINT_DATA_BUFFER>();
let buf = ptr::addr_of_mut!((*db).ReparseTarget).cast::<c::WCHAR>();
let mut i = 0;
// FIXME: this conversion is very hacky
let v = br"\??\";
@ -1359,7 +1372,7 @@ fn symlink_junction_inner(original: &Path, junction: &Path) -> io::Result<()> {
cvt(c::DeviceIoControl(
h as *mut _,
c::FSCTL_SET_REPARSE_POINT,
data.as_ptr() as *mut _,
data_ptr.cast(),
(*db).ReparseDataLength + 8,
ptr::null_mut(),
0,

8
library/std/src/sys/windows/mod.rs
View File

@ -329,3 +329,11 @@ pub fn abort_internal() -> ! {
}
crate::intrinsics::abort();
}
/// Align the inner value to 8 bytes.
///
/// This is enough for almost all of the buffers we're likely to work with in
/// the Windows APIs we use.
#[repr(C, align(8))]
#[derive(Copy, Clone)]
pub(crate) struct Align8<T: ?Sized>(pub T);