linux-kernel - Re: [PATCH 3/4] rust: miscdevice: add uring

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aH3TVzxTlTUaDQXp@sidongui-MacBookPro.local>
Date: Mon, 21 Jul 2025 14:42:47 +0900
From: Sidong Yang <sidong.yang@...iosa.ai>
To: Caleb Sander Mateos <csander@...estorage.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, Arnd Bergmann <arnd@...db.de>,
	Jens Axboe <axboe@...nel.dk>, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org, io-uring@...r.kernel.org
Subject: Re: [PATCH 3/4] rust: miscdevice: add uring_cmd() for MiscDevice
 trait

On Sun, Jul 20, 2025 at 04:08:19PM -0400, Caleb Sander Mateos wrote:
> On Sat, Jul 19, 2025 at 10:35 AM Sidong Yang <sidong.yang@...iosa.ai> wrote:
> >
> > This patch adds uring_cmd() function for MiscDevice trait and its
> > callback implementation. It uses IoUringCmd that io_uring_cmd rust
> > abstraction.
> >
> > Signed-off-by: Sidong Yang <sidong.yang@...iosa.ai>
> > ---
> >  rust/kernel/miscdevice.rs | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >
> > diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> > index 288f40e79906..5255faf27934 100644
> > --- a/rust/kernel/miscdevice.rs
> > +++ b/rust/kernel/miscdevice.rs
> > @@ -14,6 +14,7 @@
> >      error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
> >      ffi::{c_int, c_long, c_uint, c_ulong},
> >      fs::File,
> > +    io_uring::IoUringCmd,
> >      mm::virt::VmaNew,
> >      prelude::*,
> >      seq_file::SeqFile,
> > @@ -175,6 +176,15 @@ fn show_fdinfo(
> >      ) {
> >          build_error!(VTABLE_DEFAULT_ERROR)
> >      }
> > +
> > +    fn uring_cmd(
> > +        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
> > +        _file: &File,
> > +        _io_uring_cmd: &IoUringCmd,
> > +        issue_flags: u32,
> > +    ) -> Result<isize> {
> 
> Would i32 make more sense as the return value, since that's what
> io_uring_cqe actually stores?

Agreed. thanks.
> 
> > +        build_error!(VTABLE_DEFAULT_ERROR)
> > +    }
> >  }
> >
> >  /// A vtable for the file operations of a Rust miscdevice.
> > @@ -332,6 +342,25 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
> >          T::show_fdinfo(device, m, file);
> >      }
> >
> > +    unsafe extern "C" fn uring_cmd(
> > +        ioucmd: *mut bindings::io_uring_cmd,
> > +        issue_flags: ffi::c_uint,
> > +    ) -> ffi::c_int {
> > +        // SAFETY: The file is valid for the duration of this call.
> > +        let ioucmd = unsafe { IoUringCmd::from_raw(ioucmd) };
> > +        let file = ioucmd.file();
> > +
> > +        // SAFETY: The file is valid for the duration of this call.
> > +        let private = unsafe { (*file.as_ptr()).private_data }.cast();
> > +        // SAFETY: Ioctl calls can borrow the private data of the file.
> 
> "Ioctl" -> "uring cmd"?

Thanks it's a bad mistake.

Thanks,
Sidong
> 
> Best,
> Caleb
> 
> > +        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
> > +
> > +        match T::uring_cmd(device, file, ioucmd, issue_flags) {
> > +            Ok(ret) => ret as ffi::c_int,
> > +            Err(err) => err.to_errno() as ffi::c_int,
> > +        }
> > +    }
> > +
> >      const VTABLE: bindings::file_operations = bindings::file_operations {
> >          open: Some(Self::open),
> >          release: Some(Self::release),
> > @@ -354,6 +383,11 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
> >          } else {
> >              None
> >          },
> > +        uring_cmd: if T::HAS_URING_CMD {
> > +            Some(Self::uring_cmd)
> > +        } else {
> > +            None
> > +        },
> >          // SAFETY: All zeros is a valid value for `bindings::file_operations`.
> >          ..unsafe { MaybeUninit::zeroed().assume_init() }
> >      };
> > --
> > 2.43.0
> >
> >