Skip to content

refactor(driver)!: make opcode unsafe #650

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.

Sign up for GitHub

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

Merged
George-Miao merged 1 commit into from
Jan 28, 2026
Merged

refactor(driver)!: make opcode unsafe #650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions compio-driver/src/sys/fusion/op.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ macro_rules! op {
}
}

impl<$($ty: $trait),*> poll::OpCode for $name<$($ty),*> {
unsafe impl<$($ty: $trait),*> poll::OpCode for $name<$($ty),*> {
fn pre_submit(self: std::pin::Pin<&mut Self>) -> std::io::Result<crate::Decision> {
unsafe { self.map_unchecked_mut(|x| x.inner.poll() ) }.pre_submit()
}
Expand All @@ -93,7 +93,7 @@ macro_rules! op {
}
}

impl<$($ty: $trait),*> iour::OpCode for $name<$($ty),*> {
unsafe impl<$($ty: $trait),*> iour::OpCode for $name<$($ty),*> {
fn create_entry(self: std::pin::Pin<&mut Self>) -> OpEntry {
unsafe { self.map_unchecked_mut(|x| x.inner.iour() ) }.create_entry()
}
Expand Down Expand Up @@ -179,7 +179,7 @@ macro_rules! mop {
}
}

impl<$($ty: $trait),*> poll::OpCode for $name<$($ty),*> {
unsafe impl<$($ty: $trait),*> poll::OpCode for $name<$($ty),*> {
fn pre_submit(self: std::pin::Pin<&mut Self>) -> std::io::Result<crate::Decision> {
unsafe { self.map_unchecked_mut(|x| x.inner.poll() ) }.pre_submit()
}
Expand All @@ -195,7 +195,7 @@ macro_rules! mop {
}
}

impl<$($ty: $trait),*> iour::OpCode for $name<$($ty),*> {
unsafe impl<$($ty: $trait),*> iour::OpCode for $name<$($ty),*> {
fn create_entry(self: std::pin::Pin<&mut Self>) -> OpEntry {
unsafe { self.map_unchecked_mut(|x| x.inner.iour() ) }.create_entry()
}
Expand Down
7 changes: 6 additions & 1 deletion compio-driver/src/sys/iocp/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,12 @@ pub enum OpType {
}

/// Abstraction of IOCP operations.
pub trait OpCode {
///
/// # Safety
///
/// Implementors must ensure that the operation is safe to be polled
/// according to the returned [`OpType`].
pub unsafe trait OpCode {
Comment on lines 276 to +282
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since OpCode is now an unsafe trait, the # Safety section should enumerate the concrete invariants implementors must uphold (e.g., OpType::Event handles must remain valid until completion, OpType::Blocking requires operate to be thread-safe, and Overlapped requires correct use of optr). Right now it’s very high-level and easy for downstream implementors to miss required guarantees that can lead to UB.

Copilot uses AI. Check for mistakes.
/// Determines that the operation is really overlapped defined by Windows
/// API. If not, the driver will try to operate it in another thread.
fn op_type(&self) -> OpType {
Expand Down
54 changes: 27 additions & 27 deletions compio-driver/src/sys/iocp/op.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn get_wsa_fn<F>(handle: RawFd, fguid: GUID) -> io::Result<Option<F>> {
Ok(fptr)
}

impl<
unsafe impl<
D: std::marker::Send + 'static,
F: (FnOnce() -> BufResult<usize, D>) + std::marker::Send + 'static,
> OpCode for Asyncify<F, D>
Expand All @@ -131,7 +131,7 @@ impl<
}
}

impl<
unsafe impl<
S,
D: std::marker::Send + 'static,
F: (FnOnce(&S) -> BufResult<usize, D>) + std::marker::Send + 'static,
Expand All @@ -154,7 +154,7 @@ impl<
}
}

impl OpCode for CloseFile {
unsafe impl OpCode for CloseFile {
fn op_type(&self) -> OpType {
OpType::Blocking
}
Expand All @@ -166,7 +166,7 @@ impl OpCode for CloseFile {
}
}

impl<T: IoBufMut, S: AsFd> OpCode for ReadAt<T, S> {
unsafe 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 _;
Expand All @@ -192,7 +192,7 @@ impl<T: IoBufMut, S: AsFd> OpCode for ReadAt<T, S> {
}
}

impl<T: IoBuf, S: AsFd> OpCode for WriteAt<T, S> {
unsafe impl<T: IoBuf, S: AsFd> OpCode for WriteAt<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 _;
Expand All @@ -217,7 +217,7 @@ impl<T: IoBuf, S: AsFd> OpCode for WriteAt<T, S> {
}
}

impl<S: AsFd> OpCode for ReadManagedAt<S> {
unsafe impl<S: AsFd> OpCode for ReadManagedAt<S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
unsafe { self.project().op.operate(optr) }
}
Expand All @@ -227,7 +227,7 @@ impl<S: AsFd> OpCode for ReadManagedAt<S> {
}
}

impl<T: IoBufMut, S: AsFd> OpCode for Read<T, S> {
unsafe impl<T: IoBufMut, S: AsFd> OpCode for Read<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let fd = self.fd.as_fd().as_raw_fd();
let slice = self.project().buffer.sys_slice_mut();
Expand All @@ -249,7 +249,7 @@ impl<T: IoBufMut, S: AsFd> OpCode for Read<T, S> {
}
}

impl<T: IoBuf, S: AsFd> OpCode for Write<T, S> {
unsafe impl<T: IoBuf, S: AsFd> OpCode for Write<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let slice = self.buffer.as_init();
let mut transferred = 0;
Expand All @@ -270,7 +270,7 @@ impl<T: IoBuf, S: AsFd> OpCode for Write<T, S> {
}
}

impl<S: AsFd> OpCode for ReadManaged<S> {
unsafe impl<S: AsFd> OpCode for ReadManaged<S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
unsafe { self.project().op.operate(optr) }
}
Expand All @@ -280,7 +280,7 @@ impl<S: AsFd> OpCode for ReadManaged<S> {
}
}

impl<S: AsFd> OpCode for Sync<S> {
unsafe impl<S: AsFd> OpCode for Sync<S> {
fn op_type(&self) -> OpType {
OpType::Blocking
}
Expand All @@ -292,7 +292,7 @@ impl<S: AsFd> OpCode for Sync<S> {
}
}

impl<S: AsFd> OpCode for ShutdownSocket<S> {
unsafe impl<S: AsFd> OpCode for ShutdownSocket<S> {
fn op_type(&self) -> OpType {
OpType::Blocking
}
Expand All @@ -309,7 +309,7 @@ impl<S: AsFd> OpCode for ShutdownSocket<S> {
}
}

impl OpCode for CloseSocket {
unsafe impl OpCode for CloseSocket {
fn op_type(&self) -> OpType {
OpType::Blocking
}
Expand Down Expand Up @@ -407,7 +407,7 @@ impl<S: AsFd> Accept<S> {
}
}

impl<S: AsFd> OpCode for Accept<S> {
unsafe impl<S: AsFd> OpCode for Accept<S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let accept_fn = ACCEPT_EX
.get_or_try_init(|| get_wsa_fn(self.fd.as_fd().as_raw_fd(), WSAID_ACCEPTEX))?
Expand Down Expand Up @@ -455,7 +455,7 @@ impl<S: AsFd> Connect<S> {
}
}

impl<S: AsFd> OpCode for Connect<S> {
unsafe impl<S: AsFd> OpCode for Connect<S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let connect_fn = CONNECT_EX
.get_or_try_init(|| get_wsa_fn(self.fd.as_fd().as_raw_fd(), WSAID_CONNECTEX))?
Expand Down Expand Up @@ -518,7 +518,7 @@ impl<T: IoBufMut, S> IntoInner for Recv<T, S> {
}
}

impl<T: IoBufMut, S: AsFd> OpCode for Recv<T, S> {
unsafe impl<T: IoBufMut, S: AsFd> OpCode for Recv<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();
let fd = this.fd.as_fd().as_raw_fd();
Expand All @@ -544,7 +544,7 @@ impl<T: IoBufMut, S: AsFd> OpCode for Recv<T, S> {
}
}

impl<S: AsFd> OpCode for RecvManaged<S> {
unsafe impl<S: AsFd> OpCode for RecvManaged<S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
unsafe { self.project().op.operate(optr) }
}
Expand Down Expand Up @@ -587,7 +587,7 @@ impl<T: IoVectoredBufMut, S> IntoInner for RecvVectored<T, S> {
}
}

impl<T: IoVectoredBufMut, S: AsFd> OpCode for RecvVectored<T, S> {
unsafe impl<T: IoVectoredBufMut, S: AsFd> OpCode for RecvVectored<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();
let fd = this.fd.as_fd().as_raw_fd();
Expand Down Expand Up @@ -649,7 +649,7 @@ impl<T: IoBuf, S> IntoInner for Send<T, S> {
}
}

impl<T: IoBuf, S: AsFd> OpCode for Send<T, S> {
unsafe impl<T: IoBuf, S: AsFd> OpCode for Send<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();
*this.slice = this.buffer.as_ref().sys_slice();
Expand Down Expand Up @@ -706,7 +706,7 @@ impl<T: IoVectoredBuf, S> IntoInner for SendVectored<T, S> {
}
}

impl<T: IoVectoredBuf, S: AsFd> OpCode for SendVectored<T, S> {
unsafe impl<T: IoVectoredBuf, S: AsFd> OpCode for SendVectored<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();
*this.slices = this.buffer.as_ref().sys_slices();
Expand Down Expand Up @@ -769,7 +769,7 @@ impl<T: IoBufMut, S> IntoInner for RecvFrom<T, S> {
}
}

impl<T: IoBufMut, S: AsFd> OpCode for RecvFrom<T, S> {
unsafe impl<T: IoBufMut, S: AsFd> OpCode for RecvFrom<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();
let fd = this.fd.as_fd().as_raw_fd();
Expand Down Expand Up @@ -836,7 +836,7 @@ impl<T: IoVectoredBufMut, S> IntoInner for RecvFromVectored<T, S> {
}
}

impl<T: IoVectoredBufMut, S: AsFd> OpCode for RecvFromVectored<T, S> {
unsafe impl<T: IoVectoredBufMut, S: AsFd> OpCode for RecvFromVectored<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();
let fd = this.fd.as_fd().as_raw_fd();
Expand Down Expand Up @@ -899,7 +899,7 @@ impl<T: IoBuf, S> IntoInner for SendTo<T, S> {
}
}

impl<T: IoBuf, S: AsFd> OpCode for SendTo<T, S> {
unsafe impl<T: IoBuf, S: AsFd> OpCode for SendTo<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();
*this.slice = this.buffer.as_ref().sys_slice();
Expand Down Expand Up @@ -960,7 +960,7 @@ impl<T: IoVectoredBuf, S> IntoInner for SendToVectored<T, S> {
}
}

impl<T: IoVectoredBuf, S: AsFd> OpCode for SendToVectored<T, S> {
unsafe impl<T: IoVectoredBuf, S: AsFd> OpCode for SendToVectored<T, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();
*this.slices = this.buffer.as_ref().sys_slices();
Expand Down Expand Up @@ -1042,7 +1042,7 @@ impl<T: IoVectoredBufMut, C: IoBufMut, S> IntoInner for RecvMsg<T, C, S> {
}
}

impl<T: IoVectoredBufMut, C: IoBufMut, S: AsFd> OpCode for RecvMsg<T, C, S> {
unsafe impl<T: IoVectoredBufMut, C: IoBufMut, S: AsFd> OpCode for RecvMsg<T, C, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let recvmsg_fn = WSA_RECVMSG
.get_or_try_init(|| get_wsa_fn(self.fd.as_fd().as_raw_fd(), WSAID_WSARECVMSG))?
Expand Down Expand Up @@ -1126,7 +1126,7 @@ impl<T: IoVectoredBuf, C: IoBuf, S> IntoInner for SendMsg<T, C, S> {
}
}

impl<T: IoVectoredBuf, C: IoBuf, S: AsFd> OpCode for SendMsg<T, C, S> {
unsafe impl<T: IoVectoredBuf, C: IoBuf, S: AsFd> OpCode for SendMsg<T, C, S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let this = self.project();

Expand Down Expand Up @@ -1174,7 +1174,7 @@ impl<S> ConnectNamedPipe<S> {
}
}

impl<S: AsFd> OpCode for ConnectNamedPipe<S> {
unsafe impl<S: AsFd> OpCode for ConnectNamedPipe<S> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let res = unsafe { ConnectNamedPipe(self.fd.as_fd().as_raw_fd() as _, optr) };
win32_result(res, 0)
Expand Down Expand Up @@ -1219,7 +1219,7 @@ impl<S, I: IoBuf, O: IoBufMut> IntoInner for DeviceIoControl<S, I, O> {
}
}

impl<S: AsFd, I: IoBuf, O: IoBufMut> OpCode for DeviceIoControl<S, I, O> {
unsafe impl<S: AsFd, I: IoBuf, O: IoBufMut> OpCode for DeviceIoControl<S, I, O> {
unsafe fn operate(self: Pin<&mut Self>, optr: *mut OVERLAPPED) -> Poll<io::Result<usize>> {
let mut this = self.project();

Expand Down
7 changes: 6 additions & 1 deletion compio-driver/src/sys/iour/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,12 @@ impl From<io_uring::squeue::Entry128> for OpEntry {
}

/// Abstraction of io-uring operations.
pub trait OpCode {
///
/// # Safety
///
/// The returned Entry from `create_entry` must be valid until the operation is
/// completed.
Comment on lines +113 to +114
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new # Safety contract for this unsafe trait is too ambiguous: the io_uring::squeue::Entry value itself does not need to stay alive, but any resources it refers to (e.g., buffer pointers/lengths, iovec arrays, strings, etc.) must remain valid (and properly aligned) until the operation completes or is cancelled. Please tighten the wording to explicitly state what must be kept alive and what constitutes “completed” (incl. cancellation).

Suggested change
/// The returned Entry from `create_entry` must be valid until the operation is
/// completed.
/// Implementors must uphold the safety contract between the io-uring submission
/// entry created by [`create_entry`] and any user-space resources it refers to.
///
/// In particular:
/// - The `io_uring::squeue::Entry` / `Entry128` value itself does *not* need to
/// stay alive after `create_entry` returns; the runtime may move, copy, or drop
/// that value at will.
/// - However, every resource that the submission entry *refers to* (for example,
/// buffer pointers and lengths, `iovec`/slice arrays, strings, and any other
/// memory or file descriptors passed to the kernel through the SQE) must remain
/// allocated, properly aligned, and valid for the kernel to read/write for the
/// entire lifetime of the in-flight operation.
///
/// For the purposes of this contract, an operation is considered *completed* once
/// the runtime has finished handling the associated io-uring completion queue
/// entry (CQE), including CQEs that report an explicit cancellation (for example,
/// via an `AsyncCancel` request). Until that point, implementations must not free,
/// move, or otherwise invalidate any memory or descriptors that are referenced by
/// the submission entry in a way that would make the kernel observe dangling or
/// misaligned pointers.

Copilot uses AI. Check for mistakes.
pub unsafe trait OpCode {
/// Create submission entry.
fn create_entry(self: Pin<&mut Self>) -> OpEntry;

Expand Down
Loading