Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unsound File methods #95469

Merged
merged 5 commits into from
Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions library/std/src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ use crate::time::SystemTime;
/// by different processes. Avoid assuming that holding a `&File` means that the
/// file will not change.
///
/// # Platform-specific behavior
///
/// On Windows, the implementation of [`Read`] and [`Write`] traits for `File`
/// perform synchronous I/O operations. Therefore the underlying file must not
/// have been opened for asynchronous I/O (e.g. by using `FILE_FLAG_OVERLAPPED`).
///
/// [`BufReader<R>`]: io::BufReader
/// [`sync_all`]: File::sync_all
#[stable(feature = "rust1", since = "1.0.0")]
Expand Down
58 changes: 46 additions & 12 deletions library/std/src/sys/windows/c.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,9 @@ pub const STATUS_SUCCESS: NTSTATUS = 0x00000000;
pub const STATUS_DELETE_PENDING: NTSTATUS = 0xc0000056_u32 as _;
pub const STATUS_INVALID_PARAMETER: NTSTATUS = 0xc000000d_u32 as _;

pub const STATUS_PENDING: NTSTATUS = 0x103 as _;
pub const STATUS_END_OF_FILE: NTSTATUS = 0xC0000011_u32 as _;

// Equivalent to the `NT_SUCCESS` C preprocessor macro.
// See: https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/using-ntstatus-values
pub fn nt_success(status: NTSTATUS) -> bool {
Expand Down Expand Up @@ -316,22 +319,34 @@ impl Default for OBJECT_ATTRIBUTES {
}
}
#[repr(C)]
pub struct IO_STATUS_BLOCK {
pub Pointer: *mut c_void,
pub Information: usize,
union IO_STATUS_BLOCK_union {
Status: NTSTATUS,
Pointer: *mut c_void,
}
impl Default for IO_STATUS_BLOCK {
impl Default for IO_STATUS_BLOCK_union {
fn default() -> Self {
Self { Pointer: ptr::null_mut(), Information: 0 }
Self { Pointer: ptr::null_mut() }
}
}
#[repr(C)]
#[derive(Default)]
pub struct IO_STATUS_BLOCK {
u: IO_STATUS_BLOCK_union,
pub Information: usize,
}

pub type LPOVERLAPPED_COMPLETION_ROUTINE = unsafe extern "system" fn(
dwErrorCode: DWORD,
dwNumberOfBytesTransfered: DWORD,
lpOverlapped: *mut OVERLAPPED,
);

type IO_APC_ROUTINE = unsafe extern "system" fn(
ApcContext: *mut c_void,
IoStatusBlock: *mut IO_STATUS_BLOCK,
Reserved: ULONG,
);

#[repr(C)]
#[cfg(not(target_pointer_width = "64"))]
pub struct WSADATA {
Expand Down Expand Up @@ -971,13 +986,6 @@ extern "system" {
lpOverlapped: LPOVERLAPPED,
lpCompletionRoutine: LPOVERLAPPED_COMPLETION_ROUTINE,
) -> BOOL;
pub fn WriteFile(
hFile: BorrowedHandle<'_>,
lpBuffer: LPVOID,
nNumberOfBytesToWrite: DWORD,
lpNumberOfBytesWritten: LPDWORD,
lpOverlapped: LPOVERLAPPED,
) -> BOOL;
pub fn WriteFileEx(
hFile: BorrowedHandle<'_>,
lpBuffer: LPVOID,
Expand Down Expand Up @@ -1268,6 +1276,32 @@ compat_fn! {
) -> NTSTATUS {
panic!("`NtCreateFile` not available");
}
pub fn NtReadFile(
FileHandle: BorrowedHandle<'_>,
Event: HANDLE,
ApcRoutine: Option<IO_APC_ROUTINE>,
ApcContext: *mut c_void,
IoStatusBlock: &mut IO_STATUS_BLOCK,
Buffer: *mut crate::mem::MaybeUninit<u8>,
Length: ULONG,
ByteOffset: Option<&LARGE_INTEGER>,
Key: Option<&ULONG>
) -> NTSTATUS {
panic!("`NtReadFile` not available");
}
pub fn NtWriteFile(
FileHandle: BorrowedHandle<'_>,
Event: HANDLE,
ApcRoutine: Option<IO_APC_ROUTINE>,
ApcContext: *mut c_void,
IoStatusBlock: &mut IO_STATUS_BLOCK,
Buffer: *const u8,
Length: ULONG,
ByteOffset: Option<&LARGE_INTEGER>,
Key: Option<&ULONG>
) -> NTSTATUS {
panic!("`NtWriteFile` not available");
}
pub fn RtlNtStatusToDosError(
Status: NTSTATUS
) -> ULONG {
Expand Down
169 changes: 103 additions & 66 deletions library/std/src/sys/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,10 @@ impl FromRawHandle for Handle {

impl Handle {
pub fn read(&self, buf: &mut [u8]) -> io::Result<usize> {
let mut read = 0;
let len = cmp::min(buf.len(), <c::DWORD>::MAX as usize) as c::DWORD;
let res = cvt(unsafe {
c::ReadFile(
self.as_handle(),
buf.as_mut_ptr() as c::LPVOID,
len,
&mut read,
ptr::null_mut(),
)
});
let res = unsafe { self.synchronous_read(buf.as_mut_ptr().cast(), buf.len(), None) };

match res {
Ok(_) => Ok(read as usize),
Ok(read) => Ok(read as usize),

// The special treatment of BrokenPipe is to deal with Windows
// pipe semantics, which yields this error when *reading* from
Expand All @@ -109,42 +99,23 @@ impl Handle {
}

pub fn read_at(&self, buf: &mut [u8], offset: u64) -> io::Result<usize> {
let mut read = 0;
let len = cmp::min(buf.len(), <c::DWORD>::MAX as usize) as c::DWORD;
let res = unsafe {
let mut overlapped: c::OVERLAPPED = mem::zeroed();
overlapped.Offset = offset as u32;
overlapped.OffsetHigh = (offset >> 32) as u32;
cvt(c::ReadFile(
self.as_handle(),
buf.as_mut_ptr() as c::LPVOID,
len,
&mut read,
&mut overlapped,
))
};
let res =
unsafe { self.synchronous_read(buf.as_mut_ptr().cast(), buf.len(), Some(offset)) };

match res {
Ok(_) => Ok(read as usize),
Ok(read) => Ok(read as usize),
Err(ref e) if e.raw_os_error() == Some(c::ERROR_HANDLE_EOF as i32) => Ok(0),
Err(e) => Err(e),
}
}

pub fn read_buf(&self, buf: &mut ReadBuf<'_>) -> io::Result<()> {
let mut read = 0;
let len = cmp::min(buf.remaining(), <c::DWORD>::MAX as usize) as c::DWORD;
let res = cvt(unsafe {
c::ReadFile(
self.as_handle(),
buf.unfilled_mut().as_mut_ptr() as c::LPVOID,
len,
&mut read,
ptr::null_mut(),
)
});
let res = unsafe {
self.synchronous_read(buf.unfilled_mut().as_mut_ptr(), buf.remaining(), None)
};

match res {
Ok(_) => {
Ok(read) => {
// Safety: `read` bytes were written to the initialized portion of the buffer
unsafe {
buf.assume_init(read as usize);
Expand Down Expand Up @@ -221,18 +192,7 @@ impl Handle {
}

pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
let mut amt = 0;
let len = cmp::min(buf.len(), <c::DWORD>::MAX as usize) as c::DWORD;
cvt(unsafe {
c::WriteFile(
self.as_handle(),
buf.as_ptr() as c::LPVOID,
len,
&mut amt,
ptr::null_mut(),
)
})?;
Ok(amt as usize)
self.synchronous_write(&buf, None)
}

pub fn write_vectored(&self, bufs: &[IoSlice<'_>]) -> io::Result<usize> {
Expand All @@ -245,21 +205,7 @@ impl Handle {
}

pub fn write_at(&self, buf: &[u8], offset: u64) -> io::Result<usize> {
let mut written = 0;
let len = cmp::min(buf.len(), <c::DWORD>::MAX as usize) as c::DWORD;
unsafe {
let mut overlapped: c::OVERLAPPED = mem::zeroed();
overlapped.Offset = offset as u32;
overlapped.OffsetHigh = (offset >> 32) as u32;
cvt(c::WriteFile(
self.as_handle(),
buf.as_ptr() as c::LPVOID,
len,
&mut written,
&mut overlapped,
))?;
}
Ok(written as usize)
self.synchronous_write(&buf, Some(offset))
}

pub fn try_clone(&self) -> io::Result<Self> {
Expand All @@ -274,6 +220,97 @@ impl Handle {
) -> io::Result<Self> {
Ok(Self(self.0.duplicate(access, inherit, options)?))
}

/// Performs a synchronous read.
///
/// If the handle is opened for asynchronous I/O then this abort the process.
/// See #81357.
///
/// If `offset` is `None` then the current file position is used.
unsafe fn synchronous_read(
&self,
buf: *mut mem::MaybeUninit<u8>,
len: usize,
offset: Option<u64>,
) -> io::Result<usize> {
let mut io_status = c::IO_STATUS_BLOCK::default();

// The length is clamped at u32::MAX.
let len = cmp::min(len, c::DWORD::MAX as usize) as c::DWORD;
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,
);
match status {
// If the operation has not completed then abort the process.
// Doing otherwise means that the buffer and stack may be written to
// after this function returns.
c::STATUS_PENDING => {
eprintln!("I/O error: operation failed to complete synchronously");
crate::process::abort();
}

// Return `Ok(0)` when there's nothing more to read.
c::STATUS_END_OF_FILE => Ok(0),

// Success!
status if c::nt_success(status) => Ok(io_status.Information),

status => {
let error = c::RtlNtStatusToDosError(status);
Err(io::Error::from_raw_os_error(error as _))
}
}
}

/// Performs a synchronous write.
///
/// If the handle is opened for asynchronous I/O then this abort the process.
/// See #81357.
///
/// If `offset` is `None` then the current file position is used.
fn synchronous_write(&self, buf: &[u8], offset: Option<u64>) -> io::Result<usize> {
let mut io_status = c::IO_STATUS_BLOCK::default();

// The length is clamped at u32::MAX.
let len = cmp::min(buf.len(), c::DWORD::MAX as usize) as c::DWORD;
let status = unsafe {
c::NtWriteFile(
self.as_handle(),
ptr::null_mut(),
None,
ptr::null_mut(),
&mut io_status,
buf.as_ptr(),
len,
offset.map(|n| n as _).as_ref(),
None,
)
};
match status {
// If the operation has not completed then abort the process.
// Doing otherwise means that the buffer may be read and the stack
// written to after this function returns.
c::STATUS_PENDING => {
rtabort!("I/O error: operation failed to complete synchronously");
}

// Success!
status if c::nt_success(status) => Ok(io_status.Information),

status => {
let error = unsafe { c::RtlNtStatusToDosError(status) };
Err(io::Error::from_raw_os_error(error as _))
}
}
}
}

impl<'a> Read for &'a Handle {
Expand Down