[PATCH 0/5] Rust support for `struct iov_iter`

[PATCH 0/5] Rust support for `struct iov_iter`

[Alice Ryhl]

This series adds support for the `struct iov_iter` type. This type
represents an IO buffer for reading or writing, and can be configured
for either direction of communication.

In Rust, we define separate types for reading and writing. This will
ensure that you cannot mix them up and e.g. call copy_from_iter in a
read_iter syscall.

To use the new abstractions, miscdevices are given new methods read_iter
and write_iter that can be used to implement the read/write syscalls on
a miscdevice. The miscdevice sample is updated to provide read/write
operations.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Alice Ryhl (4):
      rust: iov: add iov_iter abstractions for ITER_SOURCE
      rust: iov: add iov_iter abstractions for ITER_DEST
      rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures
      rust: alloc: add Vec::clear

Lee Jones (1):
      samples: rust_misc_device: Expand the sample to support read()ing from userspace

 rust/kernel/alloc/kvec.rs        |  27 ++++
 rust/kernel/iov.rs               | 308 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs               |   1 +
 rust/kernel/miscdevice.rs        |  97 +++++++++++-
 samples/rust/rust_misc_device.rs |  37 ++++-
 5 files changed, 467 insertions(+), 3 deletions(-)
---
base-commit: 046cc01be6b9d139b49dfc396b7201c633ff1a26
change-id: 20250311-iov-iter-c984aea07d18

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

[PATCH 1/5] rust: iov: add iov_iter abstractions for ITER_SOURCE

[Alice Ryhl]

This adds abstractions for the iov_iter type in the case where
data_source is ITER_SOURCE. This will make Rust implementations of
fops->write_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@google.com>
---
 rust/kernel/iov.rs | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs |   1 +
 2 files changed, 171 insertions(+)

diff --git a/rust/kernel/iov.rs b/rust/kernel/iov.rs
new file mode 100644
index 0000000000000000000000000000000000000000..4498f65e1f65bd964909810c020db3a9f8fae389
--- /dev/null
+++ b/rust/kernel/iov.rs
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2025 Google LLC.
+
+//! IO vectors.
+//!
+//! 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 core::{marker::PhantomData, mem::MaybeUninit, slice};
+
+const ITER_SOURCE: bool = bindings::ITER_SOURCE != 0;
+const ITER_DEST: bool = bindings::ITER_DEST != 0;
+
+// Compile-time assertion for the above constants.
+const _: () = {
+    if ITER_SOURCE == ITER_DEST {
+        panic!("ITER_DEST and ITER_SOURCE should be different.");
+    }
+};
+
+/// An IO vector that acts as a source of data.
+///
+/// # Invariants
+///
+/// Must hold a valid `struct iov_iter` with `data_source` set to `ITER_SOURCE`. The buffers
+/// referenced by the IO vector must be valid for reading for the duration of `'data`.
+///
+/// Note that if the IO vector is backed by a userspace pointer, it is always considered valid for
+/// reading.
+#[repr(transparent)]
+pub struct IovIterSource<'data> {
+    iov: Opaque<bindings::iov_iter>,
+    /// Represent to the type system that this value contains a pointer to readable data it does
+    /// not own.
+    _source: PhantomData<&'data [u8]>,
+}
+
+// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&[u8]>`, and that type is safe
+// to send across thread boundaries.
+unsafe impl<'data> Send for IovIterSource<'data> {}
+// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&[u8]>`, and that type is safe
+// to share across thread boundaries.
+unsafe impl<'data> Sync for IovIterSource<'data> {}
+
+impl<'data> IovIterSource<'data> {
+    /// Obtain an `IovIterSource` from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// * For the duration of `'iov`, the `struct iov_iter` must remain valid and must not be
+    ///   accessed except through the returned reference.
+    /// * For the duration of `'data`, the buffers backing this IO vector must be valid for
+    ///   reading.
+    #[track_caller]
+    #[inline]
+    pub unsafe fn from_raw<'iov>(ptr: *mut bindings::iov_iter) -> &'iov mut IovIterSource<'data> {
+        // SAFETY: The caller ensures that `ptr` is valid.
+        let data_source = unsafe { (*ptr).data_source };
+        assert_eq!(data_source, ITER_SOURCE);
+
+        // SAFETY: The caller ensures the struct invariants for the right durations.
+        unsafe { &mut *ptr.cast::<IovIterSource<'data>>() }
+    }
+
+    /// Access this as a raw `struct iov_iter`.
+    #[inline]
+    pub fn as_raw(&mut self) -> *mut bindings::iov_iter {
+        self.iov.get()
+    }
+
+    /// Returns the number of bytes available in this IO vector.
+    ///
+    /// Note that this may overestimate the number of bytes. For example, reading from userspace
+    /// memory could fail with EFAULT, which will be treated as the end of the IO vector.
+    #[inline]
+    pub fn len(&self) -> usize {
+        // SAFETY: It is safe to access the `count` field.
+        unsafe {
+            (*self.iov.get())
+                .__bindgen_anon_1
+                .__bindgen_anon_1
+                .as_ref()
+                .count
+        }
+    }
+
+    /// Returns whether there are any bytes left in this IO vector.
+    ///
+    /// This may return `true` even if there are no more bytes available. For example, reading from
+    /// userspace memory could fail with EFAULT, which will be treated as the end of the IO vector.
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
+    /// Advance this IO vector by `bytes` bytes.
+    ///
+    /// If `bytes` is larger than the size of this IO vector, it is advanced to the end.
+    #[inline]
+    pub fn advance(&mut self, bytes: usize) {
+        // SAFETY: `self.iov` is a valid IO vector.
+        unsafe { bindings::iov_iter_advance(self.as_raw(), bytes) };
+    }
+
+    /// Advance this IO vector backwards by `bytes` bytes.
+    ///
+    /// # Safety
+    ///
+    /// The IO vector must not be reverted to before its beginning.
+    #[inline]
+    pub unsafe fn revert(&mut self, bytes: usize) {
+        // SAFETY: `self.iov` is a valid IO vector, and `bytes` is in bounds.
+        unsafe { bindings::iov_iter_revert(self.as_raw(), bytes) };
+    }
+
+    /// Read data from this IO vector.
+    ///
+    /// Returns the number of bytes that have been copied.
+    #[inline]
+    pub fn copy_from_iter(&mut self, out: &mut [u8]) -> usize {
+        // SAFETY: We will not write uninitialized bytes to `out`.
+        let out = unsafe { &mut *(out as *mut [u8] as *mut [MaybeUninit<u8>]) };
+
+        self.copy_from_iter_raw(out).len()
+    }
+
+    /// Read data from this IO vector and append it to a vector.
+    ///
+    /// Returns the number of bytes that have been copied.
+    #[inline]
+    pub fn copy_from_iter_vec<A: Allocator>(
+        &mut self,
+        out: &mut Vec<u8, A>,
+        flags: Flags,
+    ) -> Result<usize> {
+        out.reserve(self.len(), flags)?;
+        let len = self.copy_from_iter_raw(out.spare_capacity_mut()).len();
+        // SAFETY: The next `len` bytes of the vector have been initialized.
+        unsafe { out.set_len(out.len() + len) };
+        Ok(len)
+    }
+
+    /// Read data from this IO vector into potentially uninitialized memory.
+    ///
+    /// Returns the sub-slice of the output that has been initialized. If the returned slice is
+    /// shorter than the input buffer, then the entire IO vector has been read.
+    #[inline]
+    pub fn copy_from_iter_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> &mut [u8] {
+        // SAFETY: `out` is valid for `out.len()` bytes.
+        let len =
+            unsafe { bindings::_copy_from_iter(out.as_mut_ptr().cast(), out.len(), self.as_raw()) };
+
+        // SAFETY: We just initialized the first `len` bytes of `out`.
+        unsafe { slice::from_raw_parts_mut(out.as_mut_ptr().cast(), len) }
+    }
+}
+
+impl<'data> Clone for IovIterSource<'data> {
+    #[inline]
+    fn clone(&self) -> IovIterSource<'data> {
+        // SAFETY: This duplicates the bytes inside the `Opaque` value exactly. Since `struct
+        // iov_iter` does not have any internal self references, that is okay.
+        //
+        // Since this IO vector only reads from the backing buffers, having multiple IO vectors to
+        // the same source can't lead to data races on the backing buffers.
+        unsafe { core::ptr::read(self) }
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 398242f92a961c3a445d681c65449047a847968a..de9d6e797953cb69637eef127997b1f357f3f90b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -53,6 +53,7 @@
 pub mod init;
 pub mod io;
 pub mod ioctl;
+pub mod iov;
 pub mod jump_label;
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;

-- 
2.49.0.rc0.332.g42c0ae87b1-goog

Re: [PATCH 1/5] rust: iov: add iov_iter abstractions for ITER_SOURCE

[Christian Schrefl]

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_SOURCE. This will make Rust implementations of
> fops->write_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.

Would it make sense to add comments in the C side `struct iov_iter` for
these assumptions?

> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/iov.rs | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs |   1 +
>  2 files changed, 171 insertions(+)
> 
> diff --git a/rust/kernel/iov.rs b/rust/kernel/iov.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..4498f65e1f65bd964909810c020db3a9f8fae389
> --- /dev/null
> +++ b/rust/kernel/iov.rs
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2025 Google LLC.
> +
> +//! IO vectors.
> +//!
> +//! 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 core::{marker::PhantomData, mem::MaybeUninit, slice};
> +
> +const ITER_SOURCE: bool = bindings::ITER_SOURCE != 0;
> +const ITER_DEST: bool = bindings::ITER_DEST != 0;
> +
> +// Compile-time assertion for the above constants.
> +const _: () = {
> +    if ITER_SOURCE == ITER_DEST {
> +        panic!("ITER_DEST and ITER_SOURCE should be different.");
> +    }
> +};
> +
> +/// An IO vector that acts as a source of data.
> +///
> +/// # Invariants
> +///
> +/// Must hold a valid `struct iov_iter` with `data_source` set to `ITER_SOURCE`. The buffers
> +/// referenced by the IO vector must be valid for reading for the duration of `'data`.
> +///
> +/// Note that if the IO vector is backed by a userspace pointer, it is always considered valid for
> +/// reading.
> +#[repr(transparent)]
> +pub struct IovIterSource<'data> {
> +    iov: Opaque<bindings::iov_iter>,
> +    /// Represent to the type system that this value contains a pointer to readable data it does
> +    /// not own.
> +    _source: PhantomData<&'data [u8]>,
> +}
> +
> +// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&[u8]>`, and that type is safe
> +// to send across thread boundaries.
> +unsafe impl<'data> Send for IovIterSource<'data> {}
> +// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&[u8]>`, and that type is safe
> +// to share across thread boundaries.
> +unsafe impl<'data> Sync for IovIterSource<'data> {}
> +
> +impl<'data> IovIterSource<'data> {
> +    /// Obtain an `IovIterSource` from a raw pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// * For the duration of `'iov`, the `struct iov_iter` must remain valid and must not be
> +    ///   accessed except through the returned reference.
> +    /// * For the duration of `'data`, the buffers backing this IO vector must be valid for
> +    ///   reading.
> +    #[track_caller]
> +    #[inline]
> +    pub unsafe fn from_raw<'iov>(ptr: *mut bindings::iov_iter) -> &'iov mut IovIterSource<'data> {
> +        // SAFETY: The caller ensures that `ptr` is valid.
> +        let data_source = unsafe { (*ptr).data_source };
> +        assert_eq!(data_source, ITER_SOURCE);
> +
> +        // SAFETY: The caller ensures the struct invariants for the right durations.
> +        unsafe { &mut *ptr.cast::<IovIterSource<'data>>() }
> +    }
> +
> +    /// Access this as a raw `struct iov_iter`.
> +    #[inline]
> +    pub fn as_raw(&mut self) -> *mut bindings::iov_iter {
> +        self.iov.get()
> +    }
> +
> +    /// Returns the number of bytes available in this IO vector.
> +    ///
> +    /// Note that this may overestimate the number of bytes. For example, reading from userspace
> +    /// memory could fail with EFAULT, which will be treated as the end of the IO vector.
> +    #[inline]
> +    pub fn len(&self) -> usize {
> +        // SAFETY: It is safe to access the `count` field.
> +        unsafe {
> +            (*self.iov.get())
> +                .__bindgen_anon_1
> +                .__bindgen_anon_1
> +                .as_ref()
> +                .count
> +        }
> +    }

Maybe add a C helper to avoid having to use '__bindgen_anon_1'?

> +
> +    /// Returns whether there are any bytes left in this IO vector.
> +    ///
> +    /// This may return `true` even if there are no more bytes available. For example, reading from
> +    /// userspace memory could fail with EFAULT, which will be treated as the end of the IO vector.
> +    #[inline]
> +    pub fn is_empty(&self) -> bool {
> +        self.len() == 0
> +    }
> +
> +    /// Advance this IO vector by `bytes` bytes.
> +    ///
> +    /// If `bytes` is larger than the size of this IO vector, it is advanced to the end.
> +    #[inline]
> +    pub fn advance(&mut self, bytes: usize) {
> +        // SAFETY: `self.iov` is a valid IO vector.
> +        unsafe { bindings::iov_iter_advance(self.as_raw(), bytes) };
> +    }
> +
> +    /// Advance this IO vector backwards by `bytes` bytes.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The IO vector must not be reverted to before its beginning.
> +    #[inline]
> +    pub unsafe fn revert(&mut self, bytes: usize) {
> +        // SAFETY: `self.iov` is a valid IO vector, and `bytes` is in bounds.
> +        unsafe { bindings::iov_iter_revert(self.as_raw(), bytes) };
> +    }
> +
> +    /// Read data from this IO vector.
> +    ///
> +    /// Returns the number of bytes that have been copied.
> +    #[inline]
> +    pub fn copy_from_iter(&mut self, out: &mut [u8]) -> usize {
> +        // SAFETY: We will not write uninitialized bytes to `out`.
> +        let out = unsafe { &mut *(out as *mut [u8] as *mut [MaybeUninit<u8>]) };
> +
> +        self.copy_from_iter_raw(out).len()
> +    }
> +
> +    /// Read data from this IO vector and append it to a vector.
> +    ///
> +    /// Returns the number of bytes that have been copied.
> +    #[inline]
> +    pub fn copy_from_iter_vec<A: Allocator>(
> +        &mut self,
> +        out: &mut Vec<u8, A>,
> +        flags: Flags,
> +    ) -> Result<usize> {
> +        out.reserve(self.len(), flags)?;
> +        let len = self.copy_from_iter_raw(out.spare_capacity_mut()).len();
> +        // SAFETY: The next `len` bytes of the vector have been initialized.
> +        unsafe { out.set_len(out.len() + len) };
> +        Ok(len)
> +    }
> +
> +    /// Read data from this IO vector into potentially uninitialized memory.
> +    ///
> +    /// Returns the sub-slice of the output that has been initialized. If the returned slice is
> +    /// shorter than the input buffer, then the entire IO vector has been read.
> +    #[inline]
> +    pub fn copy_from_iter_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> &mut [u8] {
> +        // SAFETY: `out` is valid for `out.len()` bytes.
> +        let len =
> +            unsafe { bindings::_copy_from_iter(out.as_mut_ptr().cast(), out.len(), self.as_raw()) };
> +
> +        // SAFETY: We just initialized the first `len` bytes of `out`.
> +        unsafe { slice::from_raw_parts_mut(out.as_mut_ptr().cast(), len) }
> +    }
> +}
> +
> +impl<'data> Clone for IovIterSource<'data> {
> +    #[inline]
> +    fn clone(&self) -> IovIterSource<'data> {
> +        // SAFETY: This duplicates the bytes inside the `Opaque` value exactly. Since `struct
> +        // iov_iter` does not have any internal self references, that is okay.
> +        //
> +        // Since this IO vector only reads from the backing buffers, having multiple IO vectors to
> +        // the same source can't lead to data races on the backing buffers.
> +        unsafe { core::ptr::read(self) }
> +    }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 398242f92a961c3a445d681c65449047a847968a..de9d6e797953cb69637eef127997b1f357f3f90b 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -53,6 +53,7 @@
>  pub mod init;
>  pub mod io;
>  pub mod ioctl;
> +pub mod iov;
>  pub mod jump_label;
>  #[cfg(CONFIG_KUNIT)]
>  pub mod kunit;
> 

Cheers
Christian

Re: [PATCH 1/5] rust: iov: add iov_iter abstractions for ITER_SOURCE

[Alice Ryhl]

On Tue, Mar 18, 2025 at 09:10:47PM +0100, Christian Schrefl wrote:
> 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_SOURCE. This will make Rust implementations of
> > fops->write_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.
> 
> Would it make sense to add comments in the C side `struct iov_iter` for
> these assumptions?

Good idea.

> > +    /// Returns the number of bytes available in this IO vector.
> > +    ///
> > +    /// Note that this may overestimate the number of bytes. For example, reading from userspace
> > +    /// memory could fail with EFAULT, which will be treated as the end of the IO vector.
> > +    #[inline]
> > +    pub fn len(&self) -> usize {
> > +        // SAFETY: It is safe to access the `count` field.
> > +        unsafe {
> > +            (*self.iov.get())
> > +                .__bindgen_anon_1
> > +                .__bindgen_anon_1
> > +                .as_ref()
> > +                .count
> > +        }
> > +    }
> 
> Maybe add a C helper to avoid having to use '__bindgen_anon_1'?

I guess I could do that. Though it introduces a mandatory function call
that can't be inlined to get the length.

Alice

Re: [PATCH 1/5] rust: iov: add iov_iter abstractions for ITER_SOURCE

[Andreas Hindborg]

Hi Alice,

"Alice Ryhl" <aliceryhl@google.com> writes:

> This adds abstractions for the iov_iter type in the case where
> data_source is ITER_SOURCE. This will make Rust implementations of
> fops->write_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@google.com>

This does not build for me:


    error[E0405]: cannot find trait `Allocator` in this scope
      --> /home/aeh/src/linux-rust/linux/rust/kernel/iov.rs:133:34
        |
    133 |     pub fn copy_from_iter_vec<A: Allocator>(
        |                                  ^^^^^^^^^ not found in this scope
        |
    help: consider importing one of these items
        |
    10  + use core::alloc::Allocator;
        |
    10  + use crate::alloc::Allocator;
        |

    error[E0412]: cannot find type `Flags` in this scope
      --> /home/aeh/src/linux-rust/linux/rust/kernel/iov.rs:136:16
        |
    136 |         flags: Flags,
        |                ^^^^^ not found in this scope
        |
    help: consider importing this struct
        |
    10  + use crate::alloc::Flags;
        |

      RUSTC L rust/kernel.o
    error: aborting due to 2 previous errors


Best regards,
Andreas Hindborg

Re: [PATCH 1/5] rust: iov: add iov_iter abstractions for ITER_SOURCE

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> This adds abstractions for the iov_iter type in the case where
> data_source is ITER_SOURCE. This will make Rust implementations of
> fops->write_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@google.com>
> ---
>  rust/kernel/iov.rs | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs |   1 +
>  2 files changed, 171 insertions(+)
>
> diff --git a/rust/kernel/iov.rs b/rust/kernel/iov.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..4498f65e1f65bd964909810c020db3a9f8fae389
> --- /dev/null
> +++ b/rust/kernel/iov.rs
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2025 Google LLC.
> +
> +//! IO vectors.
> +//!
> +//! 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 core::{marker::PhantomData, mem::MaybeUninit, slice};
> +
> +const ITER_SOURCE: bool = bindings::ITER_SOURCE != 0;
> +const ITER_DEST: bool = bindings::ITER_DEST != 0;
> +
> +// Compile-time assertion for the above constants.
> +const _: () = {
> +    if ITER_SOURCE == ITER_DEST {
> +        panic!("ITER_DEST and ITER_SOURCE should be different.");
> +    }
> +};

Don't we have some kind of `build_assert` macro to do this? If not,
maybe we should.

> +
> +/// An IO vector that acts as a source of data.
> +///
> +/// # Invariants
> +///
> +/// Must hold a valid `struct iov_iter` with `data_source` set to `ITER_SOURCE`. The buffers
> +/// referenced by the IO vector must be valid for reading for the duration of `'data`.
> +///
> +/// Note that if the IO vector is backed by a userspace pointer, it is always considered valid for
> +/// reading.

How did you arrive at this conclusion? In the discussions with Ralph on
the coherent allocator thread, we arrived at conflicting answers.

> +#[repr(transparent)]
> +pub struct IovIterSource<'data> {
> +    iov: Opaque<bindings::iov_iter>,
> +    /// Represent to the type system that this value contains a pointer to readable data it does
> +    /// not own.
> +    _source: PhantomData<&'data [u8]>,
> +}
> +
> +// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&[u8]>`, and that type is safe
> +// to send across thread boundaries.

I don't think this safety requirement is enough. Could you give the
argument without deferring to this type in `std`?

> +unsafe impl<'data> Send for IovIterSource<'data> {}
> +// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&[u8]>`, and that type is safe
> +// to share across thread boundaries.

Same as above.

> +unsafe impl<'data> Sync for IovIterSource<'data> {}
> +
> +impl<'data> IovIterSource<'data> {
> +    /// Obtain an `IovIterSource` from a raw pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// * For the duration of `'iov`, the `struct iov_iter` must remain valid and must not be
> +    ///   accessed except through the returned reference.
> +    /// * For the duration of `'data`, the buffers backing this IO vector must be valid for
> +    ///   reading.
> +    #[track_caller]
> +    #[inline]
> +    pub unsafe fn from_raw<'iov>(ptr: *mut bindings::iov_iter) -> &'iov mut IovIterSource<'data> {
> +        // SAFETY: The caller ensures that `ptr` is valid.
> +        let data_source = unsafe { (*ptr).data_source };
> +        assert_eq!(data_source, ITER_SOURCE);
> +
> +        // SAFETY: The caller ensures the struct invariants for the right durations.
> +        unsafe { &mut *ptr.cast::<IovIterSource<'data>>() }
> +    }
> +
> +    /// Access this as a raw `struct iov_iter`.
> +    #[inline]
> +    pub fn as_raw(&mut self) -> *mut bindings::iov_iter {
> +        self.iov.get()
> +    }
> +
> +    /// Returns the number of bytes available in this IO vector.
> +    ///
> +    /// Note that this may overestimate the number of bytes. For example, reading from userspace
> +    /// memory could fail with EFAULT, which will be treated as the end of the IO vector.
> +    #[inline]
> +    pub fn len(&self) -> usize {
> +        // SAFETY: It is safe to access the `count` field.

Why?

> +        unsafe {
> +            (*self.iov.get())
> +                .__bindgen_anon_1
> +                .__bindgen_anon_1
> +                .as_ref()
> +                .count
> +        }
> +    }
> +
> +    /// Returns whether there are any bytes left in this IO vector.
> +    ///
> +    /// This may return `true` even if there are no more bytes available. For example, reading from
> +    /// userspace memory could fail with EFAULT, which will be treated as the end of the IO vector.
> +    #[inline]
> +    pub fn is_empty(&self) -> bool {
> +        self.len() == 0
> +    }
> +
> +    /// Advance this IO vector by `bytes` bytes.
> +    ///
> +    /// If `bytes` is larger than the size of this IO vector, it is advanced to the end.
> +    #[inline]
> +    pub fn advance(&mut self, bytes: usize) {
> +        // SAFETY: `self.iov` is a valid IO vector.
> +        unsafe { bindings::iov_iter_advance(self.as_raw(), bytes) };
> +    }
> +
> +    /// Advance this IO vector backwards by `bytes` bytes.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The IO vector must not be reverted to before its beginning.
> +    #[inline]
> +    pub unsafe fn revert(&mut self, bytes: usize) {
> +        // SAFETY: `self.iov` is a valid IO vector, and `bytes` is in bounds.
> +        unsafe { bindings::iov_iter_revert(self.as_raw(), bytes) };
> +    }
> +
> +    /// Read data from this IO vector.
> +    ///
> +    /// Returns the number of bytes that have been copied.
> +    #[inline]
> +    pub fn copy_from_iter(&mut self, out: &mut [u8]) -> usize {
> +        // SAFETY: We will not write uninitialized bytes to `out`.

You don't know what you are reading from user space. That could be
uninit bytes?

> +        let out = unsafe { &mut *(out as *mut [u8] as *mut [MaybeUninit<u8>]) };
> +
> +        self.copy_from_iter_raw(out).len()
> +    }
> +
> +    /// Read data from this IO vector and append it to a vector.
> +    ///
> +    /// Returns the number of bytes that have been copied.
> +    #[inline]
> +    pub fn copy_from_iter_vec<A: Allocator>(
> +        &mut self,
> +        out: &mut Vec<u8, A>,
> +        flags: Flags,
> +    ) -> Result<usize> {
> +        out.reserve(self.len(), flags)?;
> +        let len = self.copy_from_iter_raw(out.spare_capacity_mut()).len();
> +        // SAFETY: The next `len` bytes of the vector have been initialized.
> +        unsafe { out.set_len(out.len() + len) };
> +        Ok(len)
> +    }
> +
> +    /// Read data from this IO vector into potentially uninitialized memory.
> +    ///
> +    /// Returns the sub-slice of the output that has been initialized. If the returned slice is
> +    /// shorter than the input buffer, then the entire IO vector has been read.
> +    #[inline]
> +    pub fn copy_from_iter_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> &mut [u8] {
> +        // SAFETY: `out` is valid for `out.len()` bytes.
> +        let len =
> +            unsafe { bindings::_copy_from_iter(out.as_mut_ptr().cast(), out.len(), self.as_raw()) };
> +
> +        // SAFETY: We just initialized the first `len` bytes of `out`.
> +        unsafe { slice::from_raw_parts_mut(out.as_mut_ptr().cast(), len) }
> +    }
> +}
> +
> +impl<'data> Clone for IovIterSource<'data> {
> +    #[inline]
> +    fn clone(&self) -> IovIterSource<'data> {
> +        // SAFETY: This duplicates the bytes inside the `Opaque` value exactly. Since `struct
> +        // iov_iter` does not have any internal self references, that is okay.
> +        //
> +        // Since this IO vector only reads from the backing buffers, having multiple IO vectors to
> +        // the same source can't lead to data races on the backing buffers.
> +        unsafe { core::ptr::read(self) }

I'm not entirely sure about the following, so please enlighten me:

What is the `'data` here? Will it be feely assignable by the caller?
Because it's not elided to the lifetime of `&self`, right? If not, I
think the source `IovIterSouce` and associated buffers might go away
before the result of the clone dies.


Best regards,
Andreas Hindborg

Re: [PATCH 1/5] rust: iov: add iov_iter abstractions for ITER_SOURCE

[Miguel Ojeda]

On Wed, Mar 19, 2025 at 7:54 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Don't we have some kind of `build_assert` macro to do this? If not,
> maybe we should.

`static_assert!` is the one (`build_assert!` is the other one, which
is a heavier hammer that should only be used if really needed).

Though our `static_assert!` does not have support for an (optional)
message -- we could add that, perhaps as a good first issue. In this
case it is not a big deal either way.

> How did you arrive at this conclusion? In the discussions with Ralph on

Ralf?

Cheers,
Miguel

Re: [PATCH 1/5] rust: iov: add iov_iter abstractions for ITER_SOURCE

[Andreas Hindborg]

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Wed, Mar 19, 2025 at 7:54 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> Don't we have some kind of `build_assert` macro to do this? If not,
>> maybe we should.
>
> `static_assert!` is the one (`build_assert!` is the other one, which
> is a heavier hammer that should only be used if really needed).
>
> Though our `static_assert!` does not have support for an (optional)
> message -- we could add that, perhaps as a good first issue. In this
> case it is not a big deal either way.
>
>> How did you arrive at this conclusion? In the discussions with Ralph on
>
> Ralf?

Sorry, misspell. Ralf Jung, added to CC.


Best regards,
Andreas Hindborg

[PATCH 2/5] rust: iov: add iov_iter abstractions for ITER_DEST

[Alice Ryhl]

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@google.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]>,
+}
+
+// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&mut [u8]>`, and that type is
+// safe to send across thread boundaries.
+unsafe impl<'data> Send for IovIterDest<'data> {}
+// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&mut [u8]>`, and that type is
+// safe to share across thread boundaries.
+unsafe impl<'data> Sync for IovIterDest<'data> {}
+
+impl<'data> IovIterDest<'data> {
+    /// Obtain an `IovIterDest` from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// * For the duration of `'iov`, the `struct iov_iter` must remain valid and must not be
+    ///   accessed except through the returned reference.
+    /// * For the duration of `'data`, the buffers backing this IO vector must be valid for
+    ///   writing.
+    #[track_caller]
+    #[inline]
+    pub unsafe fn from_raw<'iov>(ptr: *mut bindings::iov_iter) -> &'iov mut IovIterDest<'data> {
+        // SAFETY: The caller ensures that `ptr` is valid.
+        let data_source = unsafe { (*ptr).data_source };
+        assert_eq!(data_source, ITER_DEST);
+
+        // SAFETY: The caller ensures the struct invariants for the right durations.
+        unsafe { &mut *ptr.cast::<IovIterDest<'data>>() }
+    }
+
+    /// Access this as a raw `struct iov_iter`.
+    #[inline]
+    pub fn as_raw(&mut self) -> *mut bindings::iov_iter {
+        self.iov.get()
+    }
+
+    /// Returns the number of bytes available in this IO vector.
+    ///
+    /// Note that this may overestimate the number of bytes. For example, reading from userspace
+    /// memory could fail with EFAULT, which will be treated as the end of the IO vector.
+    #[inline]
+    pub fn len(&self) -> usize {
+        // SAFETY: It is safe to access the `count` field.
+        unsafe {
+            (*self.iov.get())
+                .__bindgen_anon_1
+                .__bindgen_anon_1
+                .as_ref()
+                .count
+        }
+    }
+
+    /// Returns whether there are any bytes left in this IO vector.
+    ///
+    /// This may return `true` even if there are no more bytes available. For example, reading from
+    /// userspace memory could fail with EFAULT, which will be treated as the end of the IO vector.
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        self.len() == 0
+    }
+
+    /// Advance this IO vector by `bytes` bytes.
+    ///
+    /// If `bytes` is larger than the size of this IO vector, it is advanced to the end.
+    #[inline]
+    pub fn advance(&mut self, bytes: usize) {
+        // SAFETY: `self.iov` is a valid IO vector.
+        unsafe { bindings::iov_iter_advance(self.as_raw(), bytes) };
+    }
+
+    /// Advance this IO vector backwards by `bytes` bytes.
+    ///
+    /// # Safety
+    ///
+    /// The IO vector must not be reverted to before its beginning.
+    #[inline]
+    pub unsafe fn revert(&mut self, bytes: usize) {
+        // SAFETY: `self.iov` is a valid IO vector, and `bytes` is in bounds.
+        unsafe { bindings::iov_iter_revert(self.as_raw(), bytes) };
+    }
+
+    /// Write data to this IO vector.
+    ///
+    /// Returns the number of bytes that were written. If this is shorter than the provided slice,
+    /// then no more bytes can be written.
+    #[inline]
+    pub fn copy_to_iter(&mut self, input: &[u8]) -> usize {
+        // SAFETY: `input` is valid for `input.len()` bytes.
+        unsafe { bindings::_copy_to_iter(input.as_ptr().cast(), input.len(), self.as_raw()) }
+    }
+
+    /// Utility for implementing `read_iter` given the full contents of the file.
+    ///
+    /// The full contents of the file being read from is represented by `contents`. This call will
+    /// write the appropriate sub-slice of `contents` and update the file position in `ppos` so
+    /// that the file will appear to contain `contents` even if takes multiple reads to read the
+    /// entire file.
+    #[inline]
+    pub fn simple_read_from_buffer(&mut self, ppos: &mut i64, contents: &[u8]) -> Result<usize> {
+        if *ppos < 0 {
+            return Err(EINVAL);
+        }
+        let Ok(pos) = usize::try_from(*ppos) else {
+            return Ok(0);
+        };
+        if pos >= contents.len() {
+            return Ok(0);
+        }
+
+        // BOUNDS: We just checked that `pos < contents.len()` above.
+        let num_written = self.copy_to_iter(&contents[pos..]);
+
+        // OVERFLOW: pos+num_written <= contents.len() <= isize::MAX <= i64::MAX
+        *ppos = (pos + num_written) as i64;
+
+        Ok(num_written)
+    }
+}

-- 
2.49.0.rc0.332.g42c0ae87b1-goog

Re: [PATCH 2/5] rust: iov: add iov_iter abstractions for ITER_DEST

[Christian Schrefl]

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@google.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.

> +
> +// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&mut [u8]>`, and that type is
> +// safe to send across thread boundaries.
> +unsafe impl<'data> Send for IovIterDest<'data> {}
> +// SAFETY: This struct is essentially just a fancy `std::io::Cursor<&mut [u8]>`, and that type is
> +// safe to share across thread boundaries.
> +unsafe impl<'data> Sync for IovIterDest<'data> {}
> +
> +impl<'data> IovIterDest<'data> {
> +    /// Obtain an `IovIterDest` from a raw pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// * For the duration of `'iov`, the `struct iov_iter` must remain valid and must not be
> +    ///   accessed except through the returned reference.
> +    /// * For the duration of `'data`, the buffers backing this IO vector must be valid for
> +    ///   writing.
> +    #[track_caller]
> +    #[inline]
> +    pub unsafe fn from_raw<'iov>(ptr: *mut bindings::iov_iter) -> &'iov mut IovIterDest<'data> {
> +        // SAFETY: The caller ensures that `ptr` is valid.
> +        let data_source = unsafe { (*ptr).data_source };
> +        assert_eq!(data_source, ITER_DEST);
> +
> +        // SAFETY: The caller ensures the struct invariants for the right durations.
> +        unsafe { &mut *ptr.cast::<IovIterDest<'data>>() }
> +    }
> +
> +    /// Access this as a raw `struct iov_iter`.
> +    #[inline]
> +    pub fn as_raw(&mut self) -> *mut bindings::iov_iter {
> +        self.iov.get()
> +    }
> +
> +    /// Returns the number of bytes available in this IO vector.
> +    ///
> +    /// Note that this may overestimate the number of bytes. For example, reading from userspace
> +    /// memory could fail with EFAULT, which will be treated as the end of the IO vector.
> +    #[inline]
> +    pub fn len(&self) -> usize {
> +        // SAFETY: It is safe to access the `count` field.
> +        unsafe {
> +            (*self.iov.get())
> +                .__bindgen_anon_1
> +                .__bindgen_anon_1
> +                .as_ref()
> +                .count
> +        }
> +    }
> +
> +    /// Returns whether there are any bytes left in this IO vector.
> +    ///
> +    /// This may return `true` even if there are no more bytes available. For example, reading from
> +    /// userspace memory could fail with EFAULT, which will be treated as the end of the IO vector.
> +    #[inline]
> +    pub fn is_empty(&self) -> bool {
> +        self.len() == 0
> +    }
> +
> +    /// Advance this IO vector by `bytes` bytes.
> +    ///
> +    /// If `bytes` is larger than the size of this IO vector, it is advanced to the end.
> +    #[inline]
> +    pub fn advance(&mut self, bytes: usize) {
> +        // SAFETY: `self.iov` is a valid IO vector.
> +        unsafe { bindings::iov_iter_advance(self.as_raw(), bytes) };
> +    }
> +
> +    /// Advance this IO vector backwards by `bytes` bytes.
> +    ///
> +    /// # Safety
> +    ///
> +    /// The IO vector must not be reverted to before its beginning.
> +    #[inline]
> +    pub unsafe fn revert(&mut self, bytes: usize) {
> +        // SAFETY: `self.iov` is a valid IO vector, and `bytes` is in bounds.
> +        unsafe { bindings::iov_iter_revert(self.as_raw(), bytes) };
> +    }
> +
> +    /// Write data to this IO vector.
> +    ///
> +    /// Returns the number of bytes that were written. If this is shorter than the provided slice,
> +    /// then no more bytes can be written.
> +    #[inline]
> +    pub fn copy_to_iter(&mut self, input: &[u8]) -> usize {
> +        // SAFETY: `input` is valid for `input.len()` bytes.
> +        unsafe { bindings::_copy_to_iter(input.as_ptr().cast(), input.len(), self.as_raw()) }
> +    }
> +
> +    /// Utility for implementing `read_iter` given the full contents of the file.
> +    ///
> +    /// The full contents of the file being read from is represented by `contents`. This call will
> +    /// write the appropriate sub-slice of `contents` and update the file position in `ppos` so
> +    /// that the file will appear to contain `contents` even if takes multiple reads to read the
> +    /// entire file.
> +    #[inline]
> +    pub fn simple_read_from_buffer(&mut self, ppos: &mut i64, contents: &[u8]) -> Result<usize> {
> +        if *ppos < 0 {
> +            return Err(EINVAL);
> +        }
> +        let Ok(pos) = usize::try_from(*ppos) else {
> +            return Ok(0);
> +        };
> +        if pos >= contents.len() {
> +            return Ok(0);
> +        }
> +
> +        // BOUNDS: We just checked that `pos < contents.len()` above.
> +        let num_written = self.copy_to_iter(&contents[pos..]);
> +
> +        // OVERFLOW: pos+num_written <= contents.len() <= isize::MAX <= i64::MAX
> +        *ppos = (pos + num_written) as i64;
> +
> +        Ok(num_written)
> +    }
> +}


Cheers
Christian

Re: [PATCH 2/5] rust: iov: add iov_iter abstractions for ITER_DEST

[Andreas Hindborg]

"Christian Schrefl" <chrisi.schrefl@gmail.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@google.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.


Best regards,
Andreas Hindborg

Re: [PATCH 2/5] rust: iov: add iov_iter abstractions for ITER_DEST

[Christian Schrefl]

On 19.03.25 8:14 PM, Andreas Hindborg wrote:
> "Christian Schrefl" <chrisi.schrefl@gmail.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@google.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

Re: [PATCH 2/5] rust: iov: add iov_iter abstractions for ITER_DEST

[Benno Lossin]

On Wed Mar 19, 2025 at 9:01 PM CET, Christian Schrefl wrote:
> On 19.03.25 8:14 PM, Andreas Hindborg wrote:
>> "Christian Schrefl" <chrisi.schrefl@gmail.com> writes:
>>> On 11.03.25 3:25 PM, Alice Ryhl wrote:
>>>> +/// 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
> }

I would prefer generic types, so `IovIter<Dest>` similar to what Danilo
implemented for `Device<Core>` and `Device<Normal>`. `false` and `true`
aren't descriptive and you also made a typo above :)

---
Cheers,
Benno

[PATCH 3/5] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures

[Alice Ryhl]

These will be used for the read_iter() and write_iter() callbacks, which
are now the preferred back-ends for when a user operates on a char device
with read() and write() respectively.

Co-developed-by: Lee Jones <lee@kernel.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/miscdevice.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index fa9ecc42602a477328a25b5d357db90b59dc72ae..8daafdc7f3e47aef3c90507082d35ad6819598eb 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -14,12 +14,13 @@
     error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
     ffi::{c_int, c_long, c_uint, c_ulong},
     fs::File,
+    iov::{IovIterDest, IovIterSource},
     prelude::*,
     seq_file::SeqFile,
     str::CStr,
     types::{ForeignOwnable, Opaque},
 };
-use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
+use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin, ptr::NonNull};
 
 /// Options for creating a misc device.
 #[derive(Copy, Clone)]
@@ -119,6 +120,16 @@ fn release(device: Self::Ptr, _file: &File) {
         drop(device);
     }
 
+    /// Read from this miscdevice.
+    fn read_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterDest<'_>) -> Result<usize> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Write to this miscdevice.
+    fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Result<usize> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
     /// Handler for ioctls.
     ///
     /// The `cmd` argument is usually manipulated using the utilties in [`kernel::ioctl`].
@@ -160,6 +171,36 @@ fn show_fdinfo(
     }
 }
 
+/// Wrapper for the kernel's `struct kiocb`.
+///
+/// The type `T` represents the private data of the file.
+pub struct Kiocb<'a, T> {
+    inner: NonNull<bindings::kiocb>,
+    _phantom: PhantomData<&'a T>,
+}
+
+impl<'a, T: ForeignOwnable> Kiocb<'a, T> {
+    /// Get the private data in this kiocb.
+    pub fn private_data(&self) -> <T as ForeignOwnable>::Borrowed<'a> {
+        // SAFETY: The `kiocb` lets us access the private data.
+        let private = unsafe { (*(*self.inner.as_ptr()).ki_filp).private_data };
+        // SAFETY: The kiocb has shared access to the private data.
+        unsafe { <T as ForeignOwnable>::borrow(private) }
+    }
+
+    /// Gets the current value of `ki_pos`.
+    pub fn ki_pos(&self) -> i64 {
+        // SAFETY: The `kiocb` can access `ki_pos`.
+        unsafe { (*self.inner.as_ptr()).ki_pos }
+    }
+
+    /// Gets a mutable reference to the `ki_pos` field.
+    pub fn ki_pos_mut(&mut self) -> &mut i64 {
+        // SAFETY: The `kiocb` can access `ki_pos`.
+        unsafe { &mut (*self.inner.as_ptr()).ki_pos }
+    }
+}
+
 /// A vtable for the file operations of a Rust miscdevice.
 struct MiscdeviceVTable<T: MiscDevice>(PhantomData<T>);
 
@@ -223,6 +264,50 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
         0
     }
 
+    /// # Safety
+    ///
+    /// `kiocb` must be correspond to a valid file that is associated with a
+    /// `MiscDeviceRegistration<T>`. `iter` must be a valid `struct iov_iter` for writing.
+    unsafe extern "C" fn read_iter(
+        kiocb: *mut bindings::kiocb,
+        iter: *mut bindings::iov_iter,
+    ) -> isize {
+        let kiocb = Kiocb {
+            // SAFETY: The read_iter call of a file is given a kiocb for that file.
+            inner: unsafe { NonNull::new_unchecked(kiocb) },
+            _phantom: PhantomData,
+        };
+        // SAFETY: This is a valid `struct iov_iter` for writing.
+        let iov = unsafe { IovIterDest::from_raw(iter) };
+
+        match T::read_iter(kiocb, iov) {
+            Ok(res) => res as isize,
+            Err(err) => err.to_errno() as isize,
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `kiocb` must be correspond to a valid file that is associated with a
+    /// `MiscDeviceRegistration<T>`. `iter` must be a valid `struct iov_iter` for writing.
+    unsafe extern "C" fn write_iter(
+        kiocb: *mut bindings::kiocb,
+        iter: *mut bindings::iov_iter,
+    ) -> isize {
+        let kiocb = Kiocb {
+            // SAFETY: The read_iter call of a file is given a kiocb for that file.
+            inner: unsafe { NonNull::new_unchecked(kiocb) },
+            _phantom: PhantomData,
+        };
+        // SAFETY: This is a valid `struct iov_iter` for reading.
+        let iov = unsafe { IovIterSource::from_raw(iter) };
+
+        match T::write_iter(kiocb, iov) {
+            Ok(res) => res as isize,
+            Err(err) => err.to_errno() as isize,
+        }
+    }
+
     /// # Safety
     ///
     /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
@@ -291,6 +376,16 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
     const VTABLE: bindings::file_operations = bindings::file_operations {
         open: Some(Self::open),
         release: Some(Self::release),
+        read_iter: if T::HAS_READ_ITER {
+            Some(Self::read_iter)
+        } else {
+            None
+        },
+        write_iter: if T::HAS_WRITE_ITER {
+            Some(Self::write_iter)
+        } else {
+            None
+        },
         unlocked_ioctl: if T::HAS_IOCTL {
             Some(Self::ioctl)
         } else {

-- 
2.49.0.rc0.332.g42c0ae87b1-goog

Re: [PATCH 3/5] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> These will be used for the read_iter() and write_iter() callbacks, which
> are now the preferred back-ends for when a user operates on a char device
> with read() and write() respectively.
>
> Co-developed-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/miscdevice.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> index fa9ecc42602a477328a25b5d357db90b59dc72ae..8daafdc7f3e47aef3c90507082d35ad6819598eb 100644
> --- a/rust/kernel/miscdevice.rs
> +++ b/rust/kernel/miscdevice.rs
> @@ -14,12 +14,13 @@
>      error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
>      ffi::{c_int, c_long, c_uint, c_ulong},
>      fs::File,
> +    iov::{IovIterDest, IovIterSource},
>      prelude::*,
>      seq_file::SeqFile,
>      str::CStr,
>      types::{ForeignOwnable, Opaque},
>  };
> -use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
> +use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin, ptr::NonNull};
>
>  /// Options for creating a misc device.
>  #[derive(Copy, Clone)]
> @@ -119,6 +120,16 @@ fn release(device: Self::Ptr, _file: &File) {
>          drop(device);
>      }
>
> +    /// Read from this miscdevice.
> +    fn read_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterDest<'_>) -> Result<usize> {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
> +    /// Write to this miscdevice.
> +    fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Result<usize> {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
>      /// Handler for ioctls.
>      ///
>      /// The `cmd` argument is usually manipulated using the utilties in [`kernel::ioctl`].
> @@ -160,6 +171,36 @@ fn show_fdinfo(
>      }
>  }
>
> +/// Wrapper for the kernel's `struct kiocb`.

Could you give more context? Please describe the purpose for the type
and intended use. Perhaps give an example that can be compile tested.


> +///
> +/// The type `T` represents the private data of the file.
> +pub struct Kiocb<'a, T> {
> +    inner: NonNull<bindings::kiocb>,
> +    _phantom: PhantomData<&'a T>,
> +}

An abstraction for `kiocb` does not belong here. It should go into kernel/fs.


Best regards,
Andreas Hindborg

Re: [PATCH 3/5] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> These will be used for the read_iter() and write_iter() callbacks, which
> are now the preferred back-ends for when a user operates on a char device
> with read() and write() respectively.
>
> Co-developed-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

[...]

> +impl<'a, T: ForeignOwnable> Kiocb<'a, T> {
> +    /// Get the private data in this kiocb.
> +    pub fn private_data(&self) -> <T as ForeignOwnable>::Borrowed<'a> {
> +        // SAFETY: The `kiocb` lets us access the private data.
> +        let private = unsafe { (*(*self.inner.as_ptr()).ki_filp).private_data };
> +        // SAFETY: The kiocb has shared access to the private data.
> +        unsafe { <T as ForeignOwnable>::borrow(private) }
> +    }

Another quick observation: `kiocb` has a `void *private` field that is
used in the same way that `struct file` `private_data` field is used.
The naming of this function is misleading, since it does not return the
`kiocb` private data.

Either rename this function or exchange it for an access method for the
associated file.


Best regards,
Andreas Hindborg

[PATCH 4/5] rust: alloc: add Vec::clear

[Alice Ryhl]

Our custom Vec type is missing the stdlib method `clear`, thus add it.
It will be used in the miscdevice sample.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/alloc/kvec.rs | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index ae9d072741cedbb34bed0be0c20cc75472aa53be..2d213ede2873cef87116a5527e8e24008c970a58 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -395,6 +395,33 @@ pub fn into_raw_parts(self) -> (*mut T, usize, usize) {
         (ptr, len, capacity)
     }
 
+    /// Clears the vector, removing all values.
+    ///
+    /// Note that this method has no effect on the allocated capacity
+    /// of the vector.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let mut v = kernel::kvec![1, 2, 3]?;
+    ///
+    /// v.clear();
+    ///
+    /// assert!(v.is_empty());
+    /// # Ok::<(), Error>(())
+    /// ```
+    #[inline]
+    pub fn clear(&mut self) {
+        let elems: *mut [T] = self.as_mut_slice();
+
+        // INVARIANT: This call changes the number of elements to zero.
+        self.len = 0;
+
+        // SAFETY: The values being dropped are valid values of type `T` by the type invariants.
+        // It's okay to invalidate them as we just changed the length to zero.
+        unsafe { ptr::drop_in_place(elems) };
+    }
+
     /// Ensures that the capacity exceeds the length by at least `additional` elements.
     ///
     /// # Examples

-- 
2.49.0.rc0.332.g42c0ae87b1-goog

Re: [PATCH 4/5] rust: alloc: add Vec::clear

[Danilo Krummrich]

On Tue, Mar 11, 2025 at 02:25:15PM +0000, Alice Ryhl wrote:
> Our custom Vec type is missing the stdlib method `clear`, thus add it.
> It will be used in the miscdevice sample.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/alloc/kvec.rs | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> index ae9d072741cedbb34bed0be0c20cc75472aa53be..2d213ede2873cef87116a5527e8e24008c970a58 100644
> --- a/rust/kernel/alloc/kvec.rs
> +++ b/rust/kernel/alloc/kvec.rs
> @@ -395,6 +395,33 @@ pub fn into_raw_parts(self) -> (*mut T, usize, usize) {
>          (ptr, len, capacity)
>      }
>  
> +    /// Clears the vector, removing all values.
> +    ///
> +    /// Note that this method has no effect on the allocated capacity
> +    /// of the vector.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// let mut v = kernel::kvec![1, 2, 3]?;
> +    ///
> +    /// v.clear();
> +    ///
> +    /// assert!(v.is_empty());
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    pub fn clear(&mut self) {
> +        let elems: *mut [T] = self.as_mut_slice();
> +
> +        // INVARIANT: This call changes the number of elements to zero.
> +        self.len = 0;

Please use self.set_len() instead, such that we have to cover all safety
requirements enforced by self.set_len().

With that,

	Acked-by: Danilo Krummrich <dakr@kernel.org> # for char-misc

in case you want to take it through the char-misc tree.

Ironically, self.set_len() lacks your invariant comment and corresponding safety
requirement, I will send a patch to fix this up, unless you want to. :-)

> +
> +        // SAFETY: The values being dropped are valid values of type `T` by the type invariants.
> +        // It's okay to invalidate them as we just changed the length to zero.
> +        unsafe { ptr::drop_in_place(elems) };
> +    }
> +
>      /// Ensures that the capacity exceeds the length by at least `additional` elements.
>      ///
>      /// # Examples
> 
> -- 
> 2.49.0.rc0.332.g42c0ae87b1-goog
> 

Re: [PATCH 4/5] rust: alloc: add Vec::clear

[Alice Ryhl]

On Tue, Mar 11, 2025 at 03:40:51PM +0100, Danilo Krummrich wrote:
> On Tue, Mar 11, 2025 at 02:25:15PM +0000, Alice Ryhl wrote:
> > Our custom Vec type is missing the stdlib method `clear`, thus add it.
> > It will be used in the miscdevice sample.
> > 
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  rust/kernel/alloc/kvec.rs | 27 +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> > 
> > diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> > index ae9d072741cedbb34bed0be0c20cc75472aa53be..2d213ede2873cef87116a5527e8e24008c970a58 100644
> > --- a/rust/kernel/alloc/kvec.rs
> > +++ b/rust/kernel/alloc/kvec.rs
> > @@ -395,6 +395,33 @@ pub fn into_raw_parts(self) -> (*mut T, usize, usize) {
> >          (ptr, len, capacity)
> >      }
> >  
> > +    /// Clears the vector, removing all values.
> > +    ///
> > +    /// Note that this method has no effect on the allocated capacity
> > +    /// of the vector.
> > +    ///
> > +    /// # Examples
> > +    ///
> > +    /// ```
> > +    /// let mut v = kernel::kvec![1, 2, 3]?;
> > +    ///
> > +    /// v.clear();
> > +    ///
> > +    /// assert!(v.is_empty());
> > +    /// # Ok::<(), Error>(())
> > +    /// ```
> > +    #[inline]
> > +    pub fn clear(&mut self) {
> > +        let elems: *mut [T] = self.as_mut_slice();
> > +
> > +        // INVARIANT: This call changes the number of elements to zero.
> > +        self.len = 0;
> 
> Please use self.set_len() instead, such that we have to cover all safety
> requirements enforced by self.set_len().

Will do.

> With that,
> 
> 	Acked-by: Danilo Krummrich <dakr@kernel.org> # for char-misc
> 
> in case you want to take it through the char-misc tree.
> 
> Ironically, self.set_len() lacks your invariant comment and corresponding safety
> requirement, I will send a patch to fix this up, unless you want to. :-)

Go for it.

Alice

Re: [PATCH 4/5] rust: alloc: add Vec::clear

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> Our custom Vec type is missing the stdlib method `clear`, thus add it.
> It will be used in the miscdevice sample.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---


Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg

Re: [PATCH 4/5] rust: alloc: add Vec::clear

[Tamir Duberstein]

On Tue, Mar 11, 2025 at 10:27 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Our custom Vec type is missing the stdlib method `clear`, thus add it.
> It will be used in the miscdevice sample.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/alloc/kvec.rs | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> index ae9d072741cedbb34bed0be0c20cc75472aa53be..2d213ede2873cef87116a5527e8e24008c970a58 100644
> --- a/rust/kernel/alloc/kvec.rs
> +++ b/rust/kernel/alloc/kvec.rs
> @@ -395,6 +395,33 @@ pub fn into_raw_parts(self) -> (*mut T, usize, usize) {
>          (ptr, len, capacity)
>      }
>
> +    /// Clears the vector, removing all values.
> +    ///
> +    /// Note that this method has no effect on the allocated capacity
> +    /// of the vector.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// let mut v = kernel::kvec![1, 2, 3]?;
> +    ///
> +    /// v.clear();
> +    ///
> +    /// assert!(v.is_empty());
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    #[inline]
> +    pub fn clear(&mut self) {
> +        let elems: *mut [T] = self.as_mut_slice();
> +
> +        // INVARIANT: This call changes the number of elements to zero.
> +        self.len = 0;
> +
> +        // SAFETY: The values being dropped are valid values of type `T` by the type invariants.
> +        // It's okay to invalidate them as we just changed the length to zero.
> +        unsafe { ptr::drop_in_place(elems) };
> +    }
> +
>      /// Ensures that the capacity exceeds the length by at least `additional` elements.
>      ///
>      /// # Examples
>
> --
> 2.49.0.rc0.332.g42c0ae87b1-goog
>
>

It'd be great to implement this in terms of `truncate` from Andrew's series[0].

[0] https://lore.kernel.org/all/20250316111644.154602-2-andrewjballance@gmail.com/

[PATCH 5/5] samples: rust_misc_device: Expand the sample to support read()ing from userspace

[Alice Ryhl]

From: Lee Jones <lee@kernel.org>

A userland application can now operate on the char device with read() in
order to consume a locally held buffer.  Memory for the buffer is to be
provisioned and the buffer populated in its subsequently provided
write() counterpart.

Signed-off-by: Lee Jones <lee@kernel.org>
Co-developed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 samples/rust/rust_misc_device.rs | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 40ad7266c2252e5c0b4e91e501ef9ada2eda3b16..6405713fc8ff69659799f30710074487ea3bb14e 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -101,7 +101,9 @@
     device::Device,
     fs::File,
     ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
-    miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
+    iov::{IovIterDest, IovIterSource},
+    kvec,
+    miscdevice::{Kiocb, MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
     new_mutex,
     prelude::*,
     sync::Mutex,
@@ -143,6 +145,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
 
 struct Inner {
     value: i32,
+    buffer: KVec<u8>,
 }
 
 #[pin_data(PinnedDrop)]
@@ -164,7 +167,10 @@ fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Se
         KBox::try_pin_init(
             try_pin_init! {
                 RustMiscDevice {
-                    inner <- new_mutex!( Inner{ value: 0_i32 } ),
+                    inner <- new_mutex!(Inner {
+                        value: 0_i32,
+                        buffer: kvec![],
+                    }),
                     dev: dev,
                 }
             },
@@ -172,6 +178,33 @@ fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Se
         )
     }
 
+    fn read_iter(mut kiocb: Kiocb<'_, Self::Ptr>, iov: &mut IovIterDest<'_>) -> Result<usize> {
+        let me = kiocb.private_data();
+        dev_info!(me.dev, "Reading from Rust Misc Device Sample\n");
+
+        let inner = me.inner.lock();
+        // Read the buffer contents, taking the file position into account.
+        let read = iov.simple_read_from_buffer(kiocb.ki_pos_mut(), &inner.buffer)?;
+
+        Ok(read)
+    }
+
+    fn write_iter(mut kiocb: Kiocb<'_, Self::Ptr>, iov: &mut IovIterSource<'_>) -> Result<usize> {
+        let me = kiocb.private_data();
+        dev_info!(me.dev, "Writing to Rust Misc Device Sample\n");
+
+        let mut inner = me.inner.lock();
+
+        // Replace buffer contents.
+        inner.buffer.clear();
+        let len = iov.copy_from_iter_vec(&mut inner.buffer, GFP_KERNEL)?;
+
+        // Set position to zero so that future `read` calls will see the new contents.
+        *kiocb.ki_pos_mut() = 0;
+
+        Ok(len)
+    }
+
     fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result<isize> {
         dev_info!(me.dev, "IOCTLing Rust Misc Device Sample\n");
 

-- 
2.49.0.rc0.332.g42c0ae87b1-goog

Re: [PATCH 5/5] samples: rust_misc_device: Expand the sample to support read()ing from userspace

[Andreas Hindborg]

"Alice Ryhl" <aliceryhl@google.com> writes:

> From: Lee Jones <lee@kernel.org>
>
> A userland application can now operate on the char device with read() in
> order to consume a locally held buffer.  Memory for the buffer is to be
> provisioned and the buffer populated in its subsequently provided
> write() counterpart.
>
> Signed-off-by: Lee Jones <lee@kernel.org>
> Co-developed-by: Alice Ryhl <aliceryhl@google.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>


Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>


Best regards,
Andreas Hindborg

Re: [PATCH 0/5] Rust support for `struct iov_iter`

[Greg Kroah-Hartman]

On Tue, Mar 11, 2025 at 02:25:11PM +0000, Alice Ryhl wrote:
> This series adds support for the `struct iov_iter` type. This type
> represents an IO buffer for reading or writing, and can be configured
> for either direction of communication.
> 
> In Rust, we define separate types for reading and writing. This will
> ensure that you cannot mix them up and e.g. call copy_from_iter in a
> read_iter syscall.
> 
> To use the new abstractions, miscdevices are given new methods read_iter
> and write_iter that can be used to implement the read/write syscalls on
> a miscdevice. The miscdevice sample is updated to provide read/write
> operations.

Nice, this is good to have, but what's the odds of tieing in the
"untrusted buffer" logic here so that all misc drivers HAVE to properly
validate the data sent to them before they can touch it:
	https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@proton.me

I'd like to force drivers to do this, otherwise it's just going to force
us to audit all paths from userspace->kernel that happen.

thanks,

greg k-h

Re: [PATCH 0/5] Rust support for `struct iov_iter`

[Benno Lossin]

On Tue Mar 11, 2025 at 3:37 PM CET, Greg Kroah-Hartman wrote:
> On Tue, Mar 11, 2025 at 02:25:11PM +0000, Alice Ryhl wrote:
>> This series adds support for the `struct iov_iter` type. This type
>> represents an IO buffer for reading or writing, and can be configured
>> for either direction of communication.
>> 
>> In Rust, we define separate types for reading and writing. This will
>> ensure that you cannot mix them up and e.g. call copy_from_iter in a
>> read_iter syscall.
>> 
>> To use the new abstractions, miscdevices are given new methods read_iter
>> and write_iter that can be used to implement the read/write syscalls on
>> a miscdevice. The miscdevice sample is updated to provide read/write
>> operations.
>
> Nice, this is good to have, but what's the odds of tieing in the
> "untrusted buffer" logic here so that all misc drivers HAVE to properly
> validate the data sent to them before they can touch it:
> 	https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@proton.me

I have started to work on that again, just needed to get through several
things in my backlog...

Are there any drivers or abstractions in mainline that I can use for
creating the interface? Or are those still out of tree? I don't think
that I can use tarfs for that as I did back when I started with this
patch set, as it will probably be hopelessly out of date.

---
Cheers,
Benno

> I'd like to force drivers to do this, otherwise it's just going to force
> us to audit all paths from userspace->kernel that happen.
>
> thanks,
>
> greg k-h

Re: [PATCH 0/5] Rust support for `struct iov_iter`

[Greg Kroah-Hartman]

On Wed, Mar 12, 2025 at 02:16:43AM +0000, Benno Lossin wrote:
> On Tue Mar 11, 2025 at 3:37 PM CET, Greg Kroah-Hartman wrote:
> > On Tue, Mar 11, 2025 at 02:25:11PM +0000, Alice Ryhl wrote:
> >> This series adds support for the `struct iov_iter` type. This type
> >> represents an IO buffer for reading or writing, and can be configured
> >> for either direction of communication.
> >> 
> >> In Rust, we define separate types for reading and writing. This will
> >> ensure that you cannot mix them up and e.g. call copy_from_iter in a
> >> read_iter syscall.
> >> 
> >> To use the new abstractions, miscdevices are given new methods read_iter
> >> and write_iter that can be used to implement the read/write syscalls on
> >> a miscdevice. The miscdevice sample is updated to provide read/write
> >> operations.
> >
> > Nice, this is good to have, but what's the odds of tieing in the
> > "untrusted buffer" logic here so that all misc drivers HAVE to properly
> > validate the data sent to them before they can touch it:
> > 	https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@proton.me
> 
> I have started to work on that again, just needed to get through several
> things in my backlog...
> 
> Are there any drivers or abstractions in mainline that I can use for
> creating the interface? Or are those still out of tree? I don't think
> that I can use tarfs for that as I did back when I started with this
> patch set, as it will probably be hopelessly out of date.

You can use the misc device api as a start, bindings for it are in the
tree, and this series here has an example that would need it directly.

thanks,

greg k-h

Re: [PATCH 0/5] Rust support for `struct iov_iter`

[Andreas Hindborg]

"Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:

> On Tue, Mar 11, 2025 at 02:25:11PM +0000, Alice Ryhl wrote:
>> This series adds support for the `struct iov_iter` type. This type
>> represents an IO buffer for reading or writing, and can be configured
>> for either direction of communication.
>>
>> In Rust, we define separate types for reading and writing. This will
>> ensure that you cannot mix them up and e.g. call copy_from_iter in a
>> read_iter syscall.
>>
>> To use the new abstractions, miscdevices are given new methods read_iter
>> and write_iter that can be used to implement the read/write syscalls on
>> a miscdevice. The miscdevice sample is updated to provide read/write
>> operations.
>
> Nice, this is good to have, but what's the odds of tieing in the
> "untrusted buffer" logic here so that all misc drivers HAVE to properly
> validate the data sent to them before they can touch it:
> 	https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@proton.me
>
> I'd like to force drivers to do this, otherwise it's just going to force
> us to audit all paths from userspace->kernel that happen.
>

I think that for user backed iterators (`user_backed_iter(iter) != 0`)
we will have the same problems as discussed in [1]. To validate, we
would have to copy the data to another buffer and then validate it
there, in a race free place. But the copying is apparently a problem.


Best regards,
Andreas Hindborg


[1] https://lore.kernel.org/all/ab8fd525-9a63-46e2-a443-b9d94eed6004@ralfj.de/

Re: [PATCH 0/5] Rust support for `struct iov_iter`

[Greg Kroah-Hartman]

On Tue, Mar 18, 2025 at 09:57:55PM +0100, Andreas Hindborg wrote:
> "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:
> 
> > On Tue, Mar 11, 2025 at 02:25:11PM +0000, Alice Ryhl wrote:
> >> This series adds support for the `struct iov_iter` type. This type
> >> represents an IO buffer for reading or writing, and can be configured
> >> for either direction of communication.
> >>
> >> In Rust, we define separate types for reading and writing. This will
> >> ensure that you cannot mix them up and e.g. call copy_from_iter in a
> >> read_iter syscall.
> >>
> >> To use the new abstractions, miscdevices are given new methods read_iter
> >> and write_iter that can be used to implement the read/write syscalls on
> >> a miscdevice. The miscdevice sample is updated to provide read/write
> >> operations.
> >
> > Nice, this is good to have, but what's the odds of tieing in the
> > "untrusted buffer" logic here so that all misc drivers HAVE to properly
> > validate the data sent to them before they can touch it:
> > 	https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@proton.me
> >
> > I'd like to force drivers to do this, otherwise it's just going to force
> > us to audit all paths from userspace->kernel that happen.
> >
> 
> I think that for user backed iterators (`user_backed_iter(iter) != 0`)
> we will have the same problems as discussed in [1]. To validate, we
> would have to copy the data to another buffer and then validate it
> there, in a race free place. But the copying is apparently a problem.

We already copy all data first, that's not an issue.  Validate it after
it has been copied before you do something with it, just like we do
today for normal ioctl C code.  Same goes for data coming from hardware,
it's already been copied into a buffer that you can use, no need to copy
it again, just "validate" it before using it.

thanks,

greg k-h

Re: [PATCH 0/5] Rust support for `struct iov_iter`

[Andreas Hindborg]

"Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:

> On Tue, Mar 18, 2025 at 09:57:55PM +0100, Andreas Hindborg wrote:
>> "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:
>>
>> > On Tue, Mar 11, 2025 at 02:25:11PM +0000, Alice Ryhl wrote:
>> >> This series adds support for the `struct iov_iter` type. This type
>> >> represents an IO buffer for reading or writing, and can be configured
>> >> for either direction of communication.
>> >>
>> >> In Rust, we define separate types for reading and writing. This will
>> >> ensure that you cannot mix them up and e.g. call copy_from_iter in a
>> >> read_iter syscall.
>> >>
>> >> To use the new abstractions, miscdevices are given new methods read_iter
>> >> and write_iter that can be used to implement the read/write syscalls on
>> >> a miscdevice. The miscdevice sample is updated to provide read/write
>> >> operations.
>> >
>> > Nice, this is good to have, but what's the odds of tieing in the
>> > "untrusted buffer" logic here so that all misc drivers HAVE to properly
>> > validate the data sent to them before they can touch it:
>> > 	https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@proton.me
>> >
>> > I'd like to force drivers to do this, otherwise it's just going to force
>> > us to audit all paths from userspace->kernel that happen.
>> >
>>
>> I think that for user backed iterators (`user_backed_iter(iter) != 0`)
>> we will have the same problems as discussed in [1]. To validate, we
>> would have to copy the data to another buffer and then validate it
>> there, in a race free place. But the copying is apparently a problem.
>
> We already copy all data first, that's not an issue.  Validate it after
> it has been copied before you do something with it, just like we do
> today for normal ioctl C code.  Same goes for data coming from hardware,
> it's already been copied into a buffer that you can use, no need to copy
> it again, just "validate" it before using it.

I guess that depends on the user of the `iov_iter`? At least when it is
used for direct IO, the operation is zero copy with pages mapped into
kernel and IO performed directly from those pages.


Best regards,
Andreas Hindborg

Re: [PATCH 0/5] Rust support for `struct iov_iter`

[Greg Kroah-Hartman]

On Wed, Mar 19, 2025 at 12:10:03PM +0100, Andreas Hindborg wrote:
> "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:
> 
> > On Tue, Mar 18, 2025 at 09:57:55PM +0100, Andreas Hindborg wrote:
> >> "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:
> >>
> >> > On Tue, Mar 11, 2025 at 02:25:11PM +0000, Alice Ryhl wrote:
> >> >> This series adds support for the `struct iov_iter` type. This type
> >> >> represents an IO buffer for reading or writing, and can be configured
> >> >> for either direction of communication.
> >> >>
> >> >> In Rust, we define separate types for reading and writing. This will
> >> >> ensure that you cannot mix them up and e.g. call copy_from_iter in a
> >> >> read_iter syscall.
> >> >>
> >> >> To use the new abstractions, miscdevices are given new methods read_iter
> >> >> and write_iter that can be used to implement the read/write syscalls on
> >> >> a miscdevice. The miscdevice sample is updated to provide read/write
> >> >> operations.
> >> >
> >> > Nice, this is good to have, but what's the odds of tieing in the
> >> > "untrusted buffer" logic here so that all misc drivers HAVE to properly
> >> > validate the data sent to them before they can touch it:
> >> > 	https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@proton.me
> >> >
> >> > I'd like to force drivers to do this, otherwise it's just going to force
> >> > us to audit all paths from userspace->kernel that happen.
> >> >
> >>
> >> I think that for user backed iterators (`user_backed_iter(iter) != 0`)
> >> we will have the same problems as discussed in [1]. To validate, we
> >> would have to copy the data to another buffer and then validate it
> >> there, in a race free place. But the copying is apparently a problem.
> >
> > We already copy all data first, that's not an issue.  Validate it after
> > it has been copied before you do something with it, just like we do
> > today for normal ioctl C code.  Same goes for data coming from hardware,
> > it's already been copied into a buffer that you can use, no need to copy
> > it again, just "validate" it before using it.
> 
> I guess that depends on the user of the `iov_iter`? At least when it is
> used for direct IO, the operation is zero copy with pages mapped into
> kernel and IO performed directly from those pages.

Great, and then, before you actually do something with that data, you
have to validate it that it is correct, right?  If this is just a
"pass-through" then no need to do anything to it.  But if you have to
inspect/act-on-it, then just inspect it in the verifier portion.

I'm not trying to add any additional overhead here, as you normally
would have to verify the data is correct before doing something with it,
but rather just forcing that validation to be done in an obvious place
where everyone can see that it is being done, or not being done.

In other words, make it obvious to reviewers that this is happening,
instead of forcing them to dig through code paths to hopefully find out
where it happens.

thanks,

greg k-h

Re: [PATCH 0/5] Rust support for `struct iov_iter`

[Andreas Hindborg]

"Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:

> On Wed, Mar 19, 2025 at 12:10:03PM +0100, Andreas Hindborg wrote:
>> "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:
>>
>> > On Tue, Mar 18, 2025 at 09:57:55PM +0100, Andreas Hindborg wrote:
>> >> "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> writes:
>> >>
>> >> > On Tue, Mar 11, 2025 at 02:25:11PM +0000, Alice Ryhl wrote:
>> >> >> This series adds support for the `struct iov_iter` type. This type
>> >> >> represents an IO buffer for reading or writing, and can be configured
>> >> >> for either direction of communication.
>> >> >>
>> >> >> In Rust, we define separate types for reading and writing. This will
>> >> >> ensure that you cannot mix them up and e.g. call copy_from_iter in a
>> >> >> read_iter syscall.
>> >> >>
>> >> >> To use the new abstractions, miscdevices are given new methods read_iter
>> >> >> and write_iter that can be used to implement the read/write syscalls on
>> >> >> a miscdevice. The miscdevice sample is updated to provide read/write
>> >> >> operations.
>> >> >
>> >> > Nice, this is good to have, but what's the odds of tieing in the
>> >> > "untrusted buffer" logic here so that all misc drivers HAVE to properly
>> >> > validate the data sent to them before they can touch it:
>> >> > 	https://lore.kernel.org/r/20240925205244.873020-1-benno.lossin@proton.me
>> >> >
>> >> > I'd like to force drivers to do this, otherwise it's just going to force
>> >> > us to audit all paths from userspace->kernel that happen.
>> >> >
>> >>
>> >> I think that for user backed iterators (`user_backed_iter(iter) != 0`)
>> >> we will have the same problems as discussed in [1]. To validate, we
>> >> would have to copy the data to another buffer and then validate it
>> >> there, in a race free place. But the copying is apparently a problem.
>> >
>> > We already copy all data first, that's not an issue.  Validate it after
>> > it has been copied before you do something with it, just like we do
>> > today for normal ioctl C code.  Same goes for data coming from hardware,
>> > it's already been copied into a buffer that you can use, no need to copy
>> > it again, just "validate" it before using it.
>>
>> I guess that depends on the user of the `iov_iter`? At least when it is
>> used for direct IO, the operation is zero copy with pages mapped into
>> kernel and IO performed directly from those pages.
>
> Great, and then, before you actually do something with that data, you
> have to validate it that it is correct, right?  If this is just a
> "pass-through" then no need to do anything to it.  But if you have to
> inspect/act-on-it, then just inspect it in the verifier portion.
>
> I'm not trying to add any additional overhead here, as you normally
> would have to verify the data is correct before doing something with it,
> but rather just forcing that validation to be done in an obvious place
> where everyone can see that it is being done, or not being done.
>
> In other words, make it obvious to reviewers that this is happening,
> instead of forcing them to dig through code paths to hopefully find out
> where it happens.

I would agree.

Just to be clear, I am not objecting to validating data before
interpreting, that is the only sane thing to do. I'm just raising a
concern in relation to reading pages mapped from user space in Rust.
Because apparently it is undefined behavior, as discussed in the thread
I linked.


Best regards,
Andreas Hindborg