linux-kernel - Re: [RFC PATCH v2 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: <06EA9E60-9BED-4275-9ED3-DA54CF3A8451@collabora.com>
Date: Tue, 12 Aug 2025 11:38:51 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Benno Lossin <lossin@...nel.org>
Cc: Sidong Yang <sidong.yang@...iosa.ai>,
 Caleb Sander Mateos <csander@...estorage.com>,
 Miguel Ojeda <ojeda@...nel.org>,
 Arnd Bergmann <arnd@...db.de>,
 Jens Axboe <axboe@...nel.dk>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 io-uring@...r.kernel.org
Subject: Re: [RFC PATCH v2 2/4] rust: io_uring: introduce rust abstraction for
 io-uring cmd



> On 12 Aug 2025, at 05:34, Benno Lossin <lossin@...nel.org> wrote:
> 
> On Mon Aug 11, 2025 at 4:50 PM CEST, Sidong Yang wrote:
>> On Mon, Aug 11, 2025 at 09:44:22AM -0300, Daniel Almeida wrote:
>>>> There is `uring_cmd` callback in `file_operation` at c side. `Pin<&mut IoUringCmd>`
>>>> would be create in the callback function. But the callback function could be
>>>> called repeatedly with same `io_uring_cmd` instance as far as I know.
>>>> 
>>>> But in c side, there is initialization step `io_uring_cmd_prep()`.
>>>> How about fill zero pdu in `io_uring_cmd_prep()`? And we could assign a byte
>>>> as flag in pdu for checking initialized also we should provide 31 bytes except
>>>> a byte for the flag.
>>>> 
>>> 
>>> That was a follow-up question of mine. Can´t we enforce zero-initialization
>>> in C to get rid of this MaybeUninit? Uninitialized data is just bad in general.
>>> 
>>> Hopefully this can be done as you've described above, but I don't want to over
>>> extend my opinion on something I know nothing about.
>> 
>> I need to add a commit that initialize pdu in prep step in next version. 
>> I'd like to get a comment from io_uring maintainer Jens. Thanks.
>> 
>> If we could initialize (filling zero) in prep step, How about casting issue?
>> Driver still needs to cast array to its private struct in unsafe?
> 
> We still would have the casting issue.
> 
> Can't we do the following:
> 
> * Add a new associated type to `MiscDevice` called `IoUringPdu` that
>  has to implement `Default` and have a size of at most 32 bytes.
> * make `IoUringCmd` generic
> * make `MiscDevice::uring_cmd` take `Pin<&mut IoUringCmd<Self::IoUringPdu>>`
> * initialize the private data to be `IoUringPdu::default()` when we
>  create the `IoUringCmd` object.
> * provide a `fn pdu(&mut self) -> &mut Pdu` on `IoUringPdu<Pdu>`.
> 
> Any thoughts? If we don't want to add a new associated type to
> `MiscDevice` (because not everyone has to declare the `IoUringCmd`
> data), I have a small trait dance that we can do to avoid that:


Benno,

IIUC, and note that I'm not proficient with io_uring in general:

I think we have to accept that we will need to parse types from and to byte
arrays, and that is inherently unsafe. It is no different from what is going on
in UserSliceReader/UserSliceWriter, and IMHO, we should copy that in as much as
it makes sense.

I think that the only difference is that all uAPI types de-facto satisfy all
the requirements for FromBytes/AsBytes, as we've discussed previously, whereas
here, drivers have to prove that their types can implement the trait.


By the way, Sidong, is this byte array shared with userspace? i.e.: is there
any copy_to/from_user() taking place here?

-- Daniel