-
Notifications
You must be signed in to change notification settings - Fork 85
Description
Related #600
Environment
- OS: Windows 11
- Runtime:
IOCP - Affected Version: v0.17.0, and master branch
Description
In compio-driver, when calling read_at (and other I/O operations), the buffer length (usize) is cast to u32 without any bounds checking. If the buffer size is exactly 2^32 bytes (4GB) or larger, this results in a silent truncation.
Reproducible Code
use compio::{BufResult, fs::File, io::AsyncReadAtExt};
#[compio::main]
async fn main() {
// 4GB buffer (2^32)
let size = 1usize << 32;
let buf: Vec<u8> = Vec::with_capacity(size);
let file = File::open("src/main.rs").await.unwrap();
let BufResult(res, buf) = file.read_to_end_at(buf, 0).await;
// Expected: Ok(4)
// Actual: Ok(0) (because the requested length was truncated to 0)
println!("Result: {:?}, Buffer len: {}", res, buf.len());
}Root Cause
Unlike the io_uring implementation where we could handle the capping within specific operation logic, the IOCP issue is deeper in the sys_slice conversion
In ReadAt::operate, it calls let slice = self.project().buffer.sys_slice_mut().
compio/compio-driver/src/sys/iocp/op.rs
Lines 169 to 193 in 4c2bb42
| impl<T: IoBufMut, S: AsFd> OpCode for ReadAt<T, S> { | |
| unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> { | |
| if let Some(overlapped) = unsafe { optr.as_mut() } { | |
| overlapped.Anonymous.Anonymous.Offset = (self.offset & 0xFFFFFFFF) as _; | |
| overlapped.Anonymous.Anonymous.OffsetHigh = (self.offset >> 32) as _; | |
| } | |
| let fd = self.fd.as_fd().as_raw_fd(); | |
| let slice = self.project().buffer.sys_slice_mut(); | |
| let mut transferred = 0; | |
| let res = unsafe { | |
| ReadFile( | |
| fd, | |
| slice.ptr() as _, | |
| slice.len() as _, | |
| &mut transferred, | |
| optr, | |
| ) | |
| }; | |
| win32_result(res, transferred) | |
| } | |
| fn cancel(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> io::Result<()> { | |
| cancel(self.fd.as_fd().as_raw_fd(), optr) | |
| } | |
| } |
The conversion logic in sys_slice.rs performs a direct cast (as u32), leading to truncation for values >= 4GB
compio/compio-driver/src/sys_slice.rs
Lines 22 to 33 in 4c2bb42
| mod sys { | |
| use std::mem::MaybeUninit; | |
| pub use windows_sys::Win32::Networking::WinSock::WSABUF as Inner; | |
| pub fn new(ptr: *mut MaybeUninit<u8>, len: usize) -> Inner { | |
| Inner { | |
| len: len as u32, | |
| buf: ptr as _, | |
| } | |
| } | |
| } |
Suggested Solution
We need to modify the SysSlice implementation in sys_slice.rs to cap the length at u32::MAX.
Since this affects the core slice conversion for Windows, would moving the capping logic directly into sys_slice.rs be safe, or are there more factors to consider?
#[cfg(windows)]
mod sys {
use std::mem::MaybeUninit;
pub use windows_sys::Win32::Networking::WinSock::WSABUF as Inner;
pub fn new(ptr: *mut MaybeUninit<u8>, len: usize) -> Inner {
Inner {
len: len.try_into().unwrap_or(u32::MAX),
buf: ptr as _,
}
}
}