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 for Android: free password hash cracker in your pocket

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

Message-Id: <DBHVI5WDLCY3.33K0F1UAJSHPK@kernel.org>
Date: Mon, 21 Jul 2025 18:28:09 +0200
From: "Benno Lossin" <lossin@...nel.org>
To: "Sidong Yang" <sidong.yang@...iosa.ai>, "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 2/4] rust: io_uring: introduce rust abstraction for
 io-uring cmd

On Mon Jul 21, 2025 at 5:47 PM CEST, Sidong Yang wrote:
> On Mon, Jul 21, 2025 at 11:04:31AM -0400, 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.
>
> I've thought about this deeply. You're right. exposing &mut without
> pinning make it unsafe.

> User also can make *mut and memmove to anywhere without unsafe block.

How so? Using `*mut T` always needs unsafe.

> It's safest to get NonNull from from_raw and it returns
> Pin<&mut IoUringCmd>.

I don't think you need `NonNull<T>`.

> from_raw() name is weird. it should be from_nonnnull()? Also, done()
> would get Pin<&mut Self>.

That sounds reasonable.

Are you certain that it's an exclusive reference?

---
Cheers,
Benno