linux-kernel - Re: [PATCH 2/4] rust: io_uring: introduce rust abstraction for io-uring cmd

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: <aH-hMYr4u95x03H0@sidongui-MacBookPro.local>
Date: Tue, 22 Jul 2025 23:33:21 +0900
From: Sidong Yang <sidong.yang@...iosa.ai>
To: Benno Lossin <lossin@...nel.org>
Cc: Caleb Sander Mateos <csander@...estorage.com>,
	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 2/4] rust: io_uring: introduce rust abstraction for
 io-uring cmd

On Mon, Jul 21, 2025 at 05:52:41PM +0200, Benno Lossin wrote:
> On Mon Jul 21, 2025 at 5:04 PM CEST, Caleb Sander Mateos wrote:
> > On Mon, Jul 21, 2025 at 1:23 AM Sidong Yang <sidong.yang@...iosa.ai> wrote:
> >> On Sun, Jul 20, 2025 at 03:10:28PM -0400, Caleb Sander Mateos wrote:
> >> > On Sat, Jul 19, 2025 at 10:34 AM Sidong Yang <sidong.yang@...iosa.ai> wrote:
> >> > > +    }
> >> > > +
> >> > > +    // Called by consumers of io_uring_cmd, if they originally returned -EIOCBQUEUED upon receiving the command
> >> > > +    #[inline]
> >> > > +    pub fn done(self, ret: isize, res2: u64, issue_flags: u32) {
> >> >
> >> > I don't think it's safe to move io_uring_cmd. io_uring_cmd_done(), for
> >> > example, calls cmd_to_io_kiocb() to turn struct io_uring_cmd *ioucmd
> >> > into struct io_kiocb *req via a pointer cast. And struct io_kiocb's
> >> > definitely need to be pinned in memory. For example,
> >> > io_req_normal_work_add() inserts the struct io_kiocb into a linked
> >> > list. Probably some sort of pinning is necessary for IoUringCmd.
> >>
> >> Understood, Normally the users wouldn't create IoUringCmd than use borrowed cmd
> >> in uring_cmd() callback. How about change to &mut self and also uring_cmd provides
> >> &mut IoUringCmd for arg.
> >
> > I'm still a little worried about exposing &mut IoUringCmd without
> > pinning. It would allow swapping the fields of two IoUringCmd's (and
> > therefore struct io_uring_cmd's), for example. If a struct
> > io_uring_cmd belongs to a struct io_kiocb linked into task_list,
> > swapping it with another struct io_uring_cmd would result in
> > io_uring_cmd_work() being invoked on the wrong struct io_uring_cmd.
> > Maybe it would be okay if IoUringCmd had an invariant that the struct
> > io_uring_cmd is not on the task work list. But I would feel safer with
> > using Pin<&mut IoUringCmd>. I don't have much experience with Rust in
> > the kernel, though, so I would welcome other opinions.
> 
> Pinning in the kernel isn't much different from userspace. From your
> description of what normally happens with `struct io_uring_cmd`, it
> definitely must be pinned.
> 
> From a quick glance at the patch series, I don't see a way to create a
> `IoUringCmd` by-value, which also means that the `done` function won't
> be callable (also the `fn pdu(&mut self)` function won't be callable,
> since you only ever create a `&IoUringCmd`). I'm not sure if I'm missing
> something, do you plan on further patches in the future?

Sure, this version is full of nonsence. v2 will be better than this.

> 
> How (aside from `from_raw`) are `IoUringCmd` values going to be created
> or exposed to the user?

Nomrally user would gets Pin<&mut IoUringCmd> from MiscDevice::uring_cmd().

Thanks,
Sidong

> 
> ---
> Cheers,
> Benno