linux-kernel - Re: [PATCH 2/5] rust: iov: add iov_iter abstractions for ITER

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: <52db38f9-427f-4cea-826f-eae5f39480c5@gmail.com>
Date: Wed, 19 Mar 2025 21:01:35 +0100
From: Christian Schrefl <chrisi.schrefl@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>,
 Andreas Hindborg <a.hindborg@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Alexander Viro <viro@...iv.linux.org.uk>, Arnd Bergmann <arnd@...db.de>,
 Miguel Ojeda <ojeda@...nel.org>, Boqun Feng <boqun.feng@...il.com>,
 Gary Guo <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>,
 Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>,
 Matthew Maurer <mmaurer@...gle.com>, Lee Jones <lee@...nel.org>,
 linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH 2/5] rust: iov: add iov_iter abstractions for ITER_DEST

On 19.03.25 8:14 PM, Andreas Hindborg wrote:
> "Christian Schrefl" <chrisi.schrefl@...il.com> writes:
> 
>> Hi Alice
>>
>> On 11.03.25 3:25 PM, Alice Ryhl wrote:
>>> This adds abstractions for the iov_iter type in the case where
>>> data_source is ITER_DEST. This will make Rust implementations of
>>> fops->read_iter possible.
>>>
>>> This series only has support for using existing IO vectors created by C
>>> code. Additional abstractions will be needed to support the creation of
>>> IO vectors in Rust code.
>>>
>>> These abstractions make the assumption that `struct iov_iter` does not
>>> have internal self-references, which implies that it is valid to move it
>>> between different local variables, and that you can make a copy of it to
>>> get two IO vectors into the same buffers.
>>>
>>> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
>>> ---
>>>  rust/kernel/iov.rs | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 139 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/rust/kernel/iov.rs b/rust/kernel/iov.rs
>>> index 4498f65e1f65bd964909810c020db3a9f8fae389..dc32c27c5c76d059562fd7c6b9d4b178a8ea7c81 100644
>>> --- a/rust/kernel/iov.rs
>>> +++ b/rust/kernel/iov.rs
>>> @@ -7,7 +7,12 @@
>>>  //! C headers: [`include/linux/iov_iter.h`](srctree/include/linux/iov_iter.h),
>>>  //! [`include/linux/uio.h`](srctree/include/linux/uio.h)
>>>
>>> -use crate::{bindings, prelude::*, types::Opaque};
>>> +use crate::{
>>> +    alloc::{Allocator, Flags},
>>> +    bindings,
>>> +    prelude::*,
>>> +    types::Opaque,
>>> +};
>>>  use core::{marker::PhantomData, mem::MaybeUninit, slice};
>>>
>>>  const ITER_SOURCE: bool = bindings::ITER_SOURCE != 0;
>>> @@ -168,3 +173,136 @@ fn clone(&self) -> IovIterSource<'data> {
>>>          unsafe { core::ptr::read(self) }
>>>      }
>>>  }
>>> +
>>> +/// An IO vector that acts as a destination for data.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// Must hold a valid `struct iov_iter` with `data_source` set to `ITER_DEST`. The buffers
>>> +/// referenced by the IO vector must be valid for writing for the duration of `'data`.
>>> +///
>>> +/// Note that if the IO vector is backed by a userspace pointer, it is always considered valid for
>>> +/// writing.
>>> +#[repr(transparent)]
>>> +pub struct IovIterDest<'data> {
>>> +    iov: Opaque<bindings::iov_iter>,
>>> +    /// Represent to the type system that this value contains a pointer to writable data it does
>>> +    /// not own.
>>> +    _source: PhantomData<&'data mut [u8]>,
>>> +}
>>
>> It might be a bit nicer to add a (private) struct 'IovIter' that implements the common operations.
>> Then 'IovIterDest' and 'IovIterSource' could store that struct and forward the implementations to
>> it.
>> But I'm not sure if that's really much better.
> 
> Yea, I was thinking the same. Maybe we could have an `IovInner` and a
> local `AsIovInner` trait that would give all the shared methods?
> 
> I am sure Alice knows the idiomatic way to achieve code sharing here.
> 

It would also be possible to use generics here.

Something like (Maybe using types instead of the const-generic):

pub struct IovIter<const DEST: bool> {
   ...
}

impl <const DEST: bool> IovIter<DEST> {
   // Common functions
}

impl IovIter<false> {
   // Source functions
}

impl IovIter<false> {
   // Dest functions
}

Possibly also providing type aliases for 'IovIterDest' and 'IovIterSource'.

Cheers
Christian