mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #95469 - ChrisDenton:unsound-read-write, r=joshtriplett

Browse Source 
Fix unsound `File` methods

This is a draft attempt to fix #81357. *EDIT*: this PR now tackles `read()`, `write()`, `read_at()`, `write_at()` and `read_buf`. Still needs more testing though.

cc `@jstarks,` can you confirm the the Windows team is ok with the Rust stdlib using `NtReadFile` and `NtWriteFile`?

~Also, I'm provisionally using `CancelIo` in a last ditch attempt to recover but I'm not sure that this is actually a good idea. Especially as getting into this state would be a programmer error so aborting the process is justified in any case.~ *EDIT*: removed, see comments.
This commit is contained in:
bors 2022-04-06 01:23:08 +00:00
commit 26b5e0cbb9
3 changed files with 155 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
library/std/src/fs.rs
View File

@ -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")]

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

@ -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 {
@ -316,15 +319,21 @@ 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,
@ -332,6 +341,12 @@ pub type LPOVERLAPPED_COMPLETION_ROUTINE = unsafe extern "system" fn(
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 {
@ -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,
@ -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 {

169
library/std/src/sys/windows/handle.rs
View File

@ -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
@ -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);
@ -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> {
@ -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> {
@ -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 {