Repository: bytecodealliance/rustix (language: rust)

Issue:
`u32` struct fields in place of file descriptors
Hi,

I noticed that three structs use `u32` instead of file descriptors. This fits their definition in the kernel but seems like a break with the crates goal of using `OwnedFd`/`BorrowedFd`/`AsFd`/`RawFd`.

Affected structs I found:
- https://github.com/bytecodealliance/rustix/blob/main/src/net/types.rs#L1524 (I added this blindly following the kernel definition)
- https://github.com/bytecodealliance/rustix/blob/main/src/io_uring.rs#L1205
- https://github.com/bytecodealliance/rustix/blob/main/src/process/prctl.rs#L756

Is this intentional?


Discussion / hints:
In the `SocketAddrXdp` case, it's not intentional. I didn't realize that `SocketAddrXdp`'s file descriptor field means that any function that "dereferences" it would need to be unsafe, like `sendto_xdp` and `sendto_any`. That's also the case for `io_uring_params` and `io_uring_setup` too. So those are bugs. I'm not yet sure what to do about them.

For `PrctlMmMap`, the only user is `configure_virtual_memory_map`, and it's in the category of things where we can't practically describe the lifetime, so we just make it `unsafe`. We do a similar thing for [`kevent`](https://docs.rs/rustix/latest/x86_64-unknown-freebsd/rustix/event/kqueue/fn.kevent.html).
For `SocketAddrXdp`, the [`sendmsg`](https://github.com/torvalds/linux/blob/master/net/xdp/xsk.c#L934) and [`recvmsg`](https://github.com/torvalds/linux/blob/master/net/xdp/xsk.c#L971) implementations in the kernel currently don't make use of the file descriptor or `SocketAddrXdp` in general. Only the `msghdr`, in particular the flag(s) (`MSG_DONTWAIT`) is used.
The AF_XDP docs also show an [example](https://www.kernel.org/doc/html/next/networking/af_xdp.html#xdp-use-need-wakeup-bind-flag) for `sendto(...)` with the `sockaddr` parameter set to `NULL`.

As far as I can tell, `SocketAddrXdp` is only used in the [`bind`](https://github.com/torvalds/linux/blob/master/net/xdp/xsk.c#L1153) method to signal the usage of a [shared UMEM](https://www.kernel.org/doc/html/next/networking/af_xdp.html#xdp-shared-umem-bind-flag).

Could the `sendto_xdp`, `sendto_any` (and probably `recvmsg` too) methods just omit `SocketAddrXdp`?

Fix patch:
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 526cf1e87..8cf104afa 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -216,7 +216,8 @@ jobs:
     - run: cargo check -Z build-std --target aarch64-unknown-linux-gnu_ilp32 --all-targets --features=all-apis
     - run: cargo check -Z build-std --target x86_64-unknown-haiku --all-targets --features=all-apis
     - run: cargo check -Z build-std --target x86_64-uwp-windows-msvc --all-targets --features=all-apis
-    - run: cargo check -Z build-std --target=riscv32imc-esp-espidf --features=all-apis
+    # Temporarily disable riscv32imc-esp-espidf, as it gets build errors.
+    # - run: cargo check -Z build-std --target=riscv32imc-esp-espidf --features=all-apis
     - run: cargo check -Z build-std --target=aarch64-unknown-nto-qnx710 --features=all-apis
     - run: cargo check -Z build-std --target=x86_64-pc-nto-qnx710 --features=all-apis
     # Temporarily disable --features=all-apis, which doesn't build yet.
diff --git a/CHANGES.md b/CHANGES.md
index 2b9371988..dc046b236 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -254,5 +254,10 @@ renamed to [`SocketAddrXdpFlags`].
 [`SocketAddrXdpWithSharedUmem`]: https://docs.rs/rustix/1.0.0/rustix/net/xdp/struct.SocketAddrXdpWithSharedUmem.html
 [`SocketAddrXdpFlags`]: https://docs.rs/rustix/1.0.0/rustix/net/xdp/struct.SocketAddrXdpFlags.html
 
+[`rustix::io_uring::io_uring_setup`] is now unsafe, due its `io_uring_params`
+argument optionallly containing a raw file descriptor.
+
+[`rustix::io_uring::io_uring_setup`]: https://docs.rs/rustix/1.0.0/rustix/io_uring/fn.io_uring_setup.html
+
 All explicitly deprecated functions and types have been removed. Their
 deprecation messages will have identified alternatives.
diff --git a/src/io_uring/mod.rs b/src/io_uring/mod.rs
index 6391ff69d..5066620fa 100644
--- a/src/io_uring/mod.rs
+++ b/src/io_uring/mod.rs
@@ -80,12 +80,17 @@ pub struct MsgHdr {
 /// `io_uring_setup(entries, params)`—Setup a context for performing
 /// asynchronous I/O.
 ///
+/// # Safety
+///
+/// If [`IoringSetupFlags::ATTACH_WQ`] is set, the `wq_fd` field of
+/// `io_uring_params` must be an open file descriptor.
+///
 /// # References
 ///  - [Linux]
 ///
 /// [Linux]: https://www.man7.org/linux/man-pages/man2/io_uring_setup.2.html
 #[inline]
-pub fn io_uring_setup(entries: u32, params: &mut io_uring_params) -> io::Result<OwnedFd> {
+pub unsafe fn io_uring_setup(entries: u32, params: &mut io_uring_params) -> io::Result<OwnedFd> {
     backend::io_uring::syscalls::io_uring_setup(entries, params)
 }
 
@@ -1469,7 +1474,7 @@ pub struct io_uring_params {
     pub sq_thread_cpu: u32,
     pub sq_thread_idle: u32,
     pub features: IoringFeatureFlags,
-    pub wq_fd: u32,
+    pub wq_fd: RawFd,
     pub resv: [u32; 3],
     pub sq_off: io_sqring_offsets,
     pub cq_off: io_cqring_offsets,
diff --git a/src/process/prctl.rs b/src/process/prctl.rs
index 4089e895f..c06f8e3ce 100644
--- a/src/process/prctl.rs
+++ b/src/process/prctl.rs
@@ -12,7 +12,7 @@ use core::ptr::{null, null_mut, NonNull};
 use bitflags::bitflags;
 
 use crate::backend::prctl::syscalls;
-use crate::fd::{AsRawFd as _, BorrowedFd};
+use crate::fd::{AsRawFd as _, BorrowedFd, RawFd};
 use crate::ffi::{c_int, c_uint, c_void, CStr};
 use crate::io;
 use crate::prctl::*;
@@ -765,7 +765,7 @@ pub struct PrctlMmMap {
     pub auxv_size: u32,
     /// File descriptor of executable file that was used to create this
     /// process.
-    pub exe_fd: u32,
+    pub exe_fd: RawFd,
 }
 
 /// Provides one-shot access to all the addresses by passing in a


Test patch:
diff --git a/tests/io_uring/register.rs b/tests/io_uring/register.rs
index d4e762763..d342d445f 100644
--- a/tests/io_uring/register.rs
+++ b/tests/io_uring/register.rs
@@ -108,7 +108,7 @@ where
 #[test]
 fn test_io_uring_register_with() {
     let mut params = io_uring_params::default();
-    let ring_fd = io_uring_setup(4, &mut params).unwrap();
+    let ring_fd = unsafe { io_uring_setup(4, &mut params).unwrap() };
     assert_eq!(params.sq_entries, 4);
     assert_eq!(params.cq_entries, 8);
 
@@ -130,7 +130,7 @@ fn io_uring_buf_ring_can_be_registered() {
     const BGID: u16 = 42;
 
     let mut params = io_uring_params::default();
-    let ring_fd = io_uring_setup(4, &mut params).unwrap();
+    let ring_fd = unsafe { io_uring_setup(4, &mut params).unwrap() };
 
     // Test that the kernel version supports IORING_REGISTER_PBUF_RING. If it
     // doesn't, the kernel will return EINVAL. Not setting a `ring_addr` on