[PATCH v3 0/4] Untrusted Data API

[PATCH v3 0/4] Untrusted Data API

[Benno Lossin]

Sorry for taking so long with this, I didn't find enough time to work on
this and when I did, I spent a lot of time on trying to get the
interface into a satisfying state. But I always found something that I
didn't like or something that didn't fit correctly. I blame my lack of
knowledge about "all the ways of reading bytes in the kernel" together
with "how should reading bytes in the kernel look like in Rust" for
that. I think it's best I leave the ivory tower behind and just
implement the part that I'm confident about (it's not a lot :) and then
just roll with the use-cases from there. I should've decided to do that
much earlier.

I did manage to create a very basic validation API that I *think* is on
the correct path, but I still marked it as an RFC just to be sure.

We can & should merge the first two patches, as they allow new APIs to
already use `Untrusted` where possible. (some APIs might of course need
to wait for the validation API)

The first use case is Alice's `struct iov_iter` series [1]. It simply
reads untrusted data and then writes it back into userspace without
looking at it. The RFC patch introducing that can either become a normal
patch or be folded into Alice's series.

@Sima: Since you also were pretty enthusiastic about this patch series,
would you mind giving me some pointers to branches/patch series that
have your use-cases for this API? That way I hope to have a better time
designing the validation part.

[1]: https://lore.kernel.org/all/20250311-iov-iter-v1-0-f6c9134ea824@google.com

---
Cheers,
Benno

Benno Lossin (4):
  rust: transmute: add `cast_slice[_mut]` functions
  rust: create basic untrusted data API
  rust: validate: add `Validate` trait
  rust: iov: use untrusted data API

 rust/kernel/iov.rs               |  25 ++--
 rust/kernel/lib.rs               |   1 +
 rust/kernel/transmute.rs         |  41 +++++++
 rust/kernel/validate.rs          | 201 +++++++++++++++++++++++++++++++
 samples/rust/rust_misc_device.rs |   5 +-
 5 files changed, 263 insertions(+), 10 deletions(-)
 create mode 100644 rust/kernel/validate.rs


base-commit: 80e54e84911a923c40d7bee33a34c1b4be148d7a
-- 
2.48.1

[PATCH v3 1/4] rust: transmute: add `cast_slice[_mut]` functions

[Benno Lossin]

Add functions to make casting slices only one `unsafe` block.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
 rust/kernel/transmute.rs | 41 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
index 1c7d43771a37..242623bbb565 100644
--- a/rust/kernel/transmute.rs
+++ b/rust/kernel/transmute.rs
@@ -2,6 +2,8 @@
 
 //! Traits for transmuting types.
 
+use core::slice;
+
 /// Types for which any bit pattern is valid.
 ///
 /// Not all types are valid for all values. For example, a `bool` must be either zero or one, so
@@ -69,3 +71,42 @@ macro_rules! impl_asbytes {
     {<T: AsBytes>} [T],
     {<T: AsBytes, const N: usize>} [T; N],
 }
+
+/// Casts the type of a slice to another.
+///
+/// # Examples
+///
+/// ```rust
+/// # use kernel::transmute::cast_slice;
+/// #[repr(transparent)]
+/// #[derive(Debug)]
+/// struct Container<T>(T);
+///
+/// let array = [0u32; 42];
+/// let slice = &array;
+/// // SAFETY: `Container<T>` transparently wraps a `T`.
+/// let container_slice = unsafe { cast_slice(slice) };
+/// pr_info!("{container_slice}");
+/// ```
+///
+/// # Safety
+/// - `T` and `U` must have the same layout.
+pub unsafe fn cast_slice<T, U>(slice: &[T]) -> &[U] {
+    // CAST: by the safety requirements, `T` and `U` have the same layout.
+    let ptr = slice.as_ptr().cast::<U>();
+    // SAFETY: `ptr` and `len` come from the same slice reference.
+    unsafe { slice::from_raw_parts(ptr, slice.len()) }
+}
+
+/// Casts the type of a slice to another.
+///
+/// Also see [`cast_slice`].
+///
+/// # Safety
+/// - `T` and `U` must have the same layout.
+pub unsafe fn cast_slice_mut<T, U>(slice: &mut [T]) -> &mut [U] {
+    // CAST: by the safety requirements, `T` and `U` have the same layout.
+    let ptr = slice.as_mut_ptr().cast::<U>();
+    // SAFETY: `ptr` and `len` come from the same slice reference.
+    unsafe { slice::from_raw_parts_mut(ptr, slice.len()) }
+}
-- 
2.48.1

[PATCH v3 2/4] rust: create basic untrusted data API

[Benno Lossin]

When the kernel receives external data (e.g. from userspace), it usually
is a very bad idea to directly use the data for logic decision in the
kernel. For this reason, such data should be explicitly marked and
validated before making decision based on its value.

The `Untrusted<T>` wrapper type marks a value of type `T` as untrusted.
The particular meaning of "untrusted" highly depends on the type `T`.
For example `T = u8` ensures that the value of the byte cannot be
retrieved. However, `T = [u8]` still allows to access the length of the
slice. Similarly, `T = KVec<U>` allows modifications.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---

Thanks a lot to Gary who suggested to add the `Deref for
Untrusted<[T]>`, it allows implicit conversions at the right places and
hopefully makes the whole API have a lot less friction.

---
 rust/kernel/lib.rs      |   1 +
 rust/kernel/validate.rs | 142 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 143 insertions(+)
 create mode 100644 rust/kernel/validate.rs

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index de9d6e797953..b2da57bd2c02 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -85,6 +85,7 @@
 pub mod transmute;
 pub mod types;
 pub mod uaccess;
+pub mod validate;
 pub mod workqueue;
 
 #[doc(hidden)]
diff --git a/rust/kernel/validate.rs b/rust/kernel/validate.rs
new file mode 100644
index 000000000000..8b33716be0c7
--- /dev/null
+++ b/rust/kernel/validate.rs
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Untrusted data API.
+//!
+//! # Overview
+//!
+//! Untrusted data is marked using the [`Untrusted<T>`] type. See [Rationale](#rationale) for the
+//! reasons to mark untrusted data throughout the kernel. It is a totally opaque wrapper, it is not
+//! possible to read the data inside.
+//!
+//! APIs that write back into userspace usually allow writing untrusted bytes directly, allowing
+//! direct copying of untrusted user data back into userspace without validation.
+//!
+//! # Rationale
+//!
+//! When reading data from an untrusted source, it must be validated before it can be used for
+//! **logic**. For example, this is a very bad idea:
+//!
+//! ```
+//! # fn read_bytes_from_network() -> Box<[u8]> {
+//! #     Box::new([1, 0], kernel::alloc::flags::GFP_KERNEL).unwrap()
+//! # }
+//! let bytes: Box<[u8]> = read_bytes_from_network();
+//! let data_index = bytes[0];
+//! let data = bytes[usize::from(data_index)];
+//! ```
+//!
+//! While this will not lead to a memory violation (because the array index checks the bounds), it
+//! might result in a kernel panic. For this reason, all untrusted data must be wrapped in
+//! [`Untrusted<T>`]. This type only allows validating the data or passing it along, since copying
+//! data from userspace back into userspace is allowed for untrusted data.
+
+use core::ops::{Deref, DerefMut};
+
+use crate::{
+    alloc::{Allocator, Vec},
+    transmute::{cast_slice, cast_slice_mut},
+};
+
+/// Untrusted data of type `T`.
+///
+/// Data coming from userspace is considered untrusted and should be marked by this type.
+///
+/// The particular meaning of [`Untrusted<T>`] depends heavily on the type `T`. For example,
+/// `&Untrusted<[u8]>` is a reference to an untrusted slice. But the length is not considered
+/// untrusted, as it would otherwise violate normal Rust rules. For this reason, one can easily
+/// convert that reference to `&[Untrusted<u8>]`. Another such example is `Untrusted<KVec<T>>`, it
+/// derefs to `KVec<Untrusted<T>>`. Raw bytes however do not behave in this way, `Untrusted<u8>` is
+/// totally opaque.
+///
+/// # Usage in API Design
+///
+/// The exact location where to put [`Untrusted`] depends on the kind of API. When asking for an
+/// untrusted input value, or buffer to write to, always move the [`Untrusted`] wrapper as far
+/// inwards as possible:
+///
+/// ```
+/// // use this
+/// pub fn read_from_userspace(buf: &mut [Untrusted<u8>]) { todo!() }
+///
+/// // and not this
+/// pub fn read_from_userspace(buf: &mut Untrusted<[u8]>) { todo!() }
+/// ```
+///
+/// The reason for this is that `&mut Untrusted<[u8]>` can beconverted into `&mut [Untrusted<u8>]`
+/// very easily, but the converse is not possible.
+///
+/// For the same reason, when returning untrusted data by-value, one should move the [`Untrusted`]
+/// wrapper as far outward as possible:
+///
+/// ```
+/// // use this
+/// pub fn read_all_from_userspace() -> Untrusted<KVec<u8>> { todo!() }
+///
+/// // and not this
+/// pub fn read_all_from_userspace() -> KVec<Untrusted<u8>> { todo!() }
+/// ```
+///
+/// Here too the reason is that `KVec<Untrusted<u8>>` is more restrictive compared to
+/// `Untrusted<KVec<u8>>`.
+#[repr(transparent)]
+pub struct Untrusted<T: ?Sized>(T);
+
+impl<T: ?Sized> Untrusted<T> {
+    /// Marks the given value as untrusted.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::validate::Untrusted;
+    ///
+    /// # mod bindings { pub(crate) unsafe fn read_foo_info() -> [u8; 4] { todo!() } };
+    /// fn read_foo_info() -> Untrusted<[u8; 4]> {
+    ///     // SAFETY: just an FFI call without preconditions.
+    ///     Untrusted::new(unsafe { bindings::read_foo_info() })
+    /// }
+    /// ```
+    pub fn new(value: T) -> Self
+    where
+        T: Sized,
+    {
+        Self(value)
+    }
+}
+
+impl<T> Deref for Untrusted<[T]> {
+    type Target = [Untrusted<T>];
+
+    fn deref(&self) -> &Self::Target {
+        // SAFETY: `Untrusted<T>` transparently wraps `T`.
+        unsafe { cast_slice(&self.0) }
+    }
+}
+
+impl<T> DerefMut for Untrusted<[T]> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        // SAFETY: `Untrusted<T>` transparently wraps `T`.
+        unsafe { cast_slice_mut(&mut self.0) }
+    }
+}
+
+impl<T, A: Allocator> Deref for Untrusted<Vec<T, A>> {
+    type Target = Vec<Untrusted<T>, A>;
+
+    fn deref(&self) -> &Self::Target {
+        let ptr: *const Untrusted<Vec<T, A>> = self;
+        // CAST: `Untrusted<T>` transparently wraps `T`.
+        let ptr: *const Vec<Untrusted<T>, A> = ptr.cast();
+        // SAFETY: `ptr` is derived from the reference `self`.
+        unsafe { &*ptr }
+    }
+}
+
+impl<T, A: Allocator> DerefMut for Untrusted<Vec<T, A>> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        let ptr: *mut Untrusted<Vec<T, A>> = self;
+        // CAST: `Untrusted<T>` transparently wraps `T`.
+        let ptr: *mut Vec<Untrusted<T>, A> = ptr.cast();
+        // SAFETY: `ptr` is derived from the reference `self`.
+        unsafe { &mut *ptr }
+    }
+}
-- 
2.48.1

[PATCH v3 3/4] rust: validate: add `Validate` trait

[Benno Lossin]

Introduce the `Validate<Input>` trait and functions to validate
`Untrusted<T>` using said trait. This allows one to access the inner
value of `Untrusted<T>` via `validate{,_ref,_mut}` functions which
subsequently delegate the validation to user-implemented `Validate`
trait.

The `Validate` trait is the only entry point for validation code, making
it easy to spot where data is being validated.

The reason for restricting the types that can be inputs to
`Validate::validate` is to be able to have the `validate...` functions
on `Untrusted`. This is also the reason for the suggestions in the
`Usage in API Design` section in the commit that introduced
`Untrusted<T>`.

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---
 rust/kernel/validate.rs | 61 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/validate.rs b/rust/kernel/validate.rs
index 8b33716be0c7..eecac39365db 100644
--- a/rust/kernel/validate.rs
+++ b/rust/kernel/validate.rs
@@ -11,6 +11,9 @@
 //! APIs that write back into userspace usually allow writing untrusted bytes directly, allowing
 //! direct copying of untrusted user data back into userspace without validation.
 //!
+//! The only way to access untrusted data is to [`Validate::validate`] it. This is facilitated by
+//! the [`Validate`] trait.
+//!
 //! # Rationale
 //!
 //! When reading data from an untrusted source, it must be validated before it can be used for
@@ -46,7 +49,7 @@
 /// untrusted, as it would otherwise violate normal Rust rules. For this reason, one can easily
 /// convert that reference to `&[Untrusted<u8>]`. Another such example is `Untrusted<KVec<T>>`, it
 /// derefs to `KVec<Untrusted<T>>`. Raw bytes however do not behave in this way, `Untrusted<u8>` is
-/// totally opaque.
+/// totally opaque and one can only access its value by calling [`Untrusted::validate()`].
 ///
 /// # Usage in API Design
 ///
@@ -101,6 +104,30 @@ pub fn new(value: T) -> Self
     {
         Self(value)
     }
+
+    /// Validate the underlying untrusted data.
+    ///
+    /// See the [`Validate`] trait for more information.
+    pub fn validate<V: Validate<Self>>(self) -> Result<V, V::Err>
+    where
+        T: Sized,
+    {
+        V::validate(self.0)
+    }
+
+    /// Validate the underlying untrusted data.
+    ///
+    /// See the [`Validate`] trait for more information.
+    pub fn validate_ref<'a, V: Validate<&'a Self>>(&'a self) -> Result<V, V::Err> {
+        V::validate(&self.0)
+    }
+
+    /// Validate the underlying untrusted data.
+    ///
+    /// See the [`Validate`] trait for more information.
+    pub fn validate_mut<'a, V: Validate<&'a mut Self>>(&'a mut self) -> Result<V, V::Err> {
+        V::validate(&mut self.0)
+    }
 }
 
 impl<T> Deref for Untrusted<[T]> {
@@ -140,3 +167,35 @@ fn deref_mut(&mut self) -> &mut Self::Target {
         unsafe { &mut *ptr }
     }
 }
+
+/// Marks valid input for the [`Validate`] trait.
+pub trait ValidateInput: private::Sealed {}
+
+impl<T: ?Sized> ValidateInput for Untrusted<T> {}
+
+impl<'a, T: ?Sized> ValidateInput for &'a Untrusted<T> {}
+
+impl<'a, T: ?Sized> ValidateInput for &'a mut Untrusted<T> {}
+
+mod private {
+    use super::Untrusted;
+
+    pub trait Sealed {}
+
+    impl<T: ?Sized> Sealed for Untrusted<T> {}
+    impl<'a, T: ?Sized> Sealed for &'a Untrusted<T> {}
+    impl<'a, T: ?Sized> Sealed for &'a mut Untrusted<T> {}
+}
+
+/// Validate [`Untrusted`] data.
+///
+/// Care must be taken when implementing this trait, as unprotected access to unvalidated data is
+/// given to the [`Validate::validate`] function. The implementer must ensure that the data is only
+/// used for logic after successful validation.
+pub trait Validate<Input: ValidateInput>: Sized {
+    /// Validation error.
+    type Err;
+
+    /// Validate the raw input.
+    fn validate(raw: Input) -> Result<Self, Self::Err>;
+}
-- 
2.48.1

[PATCH v3 4/4] rust: iov: use untrusted data API

[Benno Lossin]

Signed-off-by: Benno Lossin <benno.lossin@proton.me>
---

This patch depends on Alice's `struct iov_iter` patch series:

    https://lore.kernel.org/all/20250311-iov-iter-v1-0-f6c9134ea824@google.com

---
 rust/kernel/iov.rs               | 25 +++++++++++++++++--------
 samples/rust/rust_misc_device.rs |  5 +++--
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/rust/kernel/iov.rs b/rust/kernel/iov.rs
index dc32c27c5c76..840c2aa82e41 100644
--- a/rust/kernel/iov.rs
+++ b/rust/kernel/iov.rs
@@ -11,7 +11,9 @@
     alloc::{Allocator, Flags},
     bindings,
     prelude::*,
+    transmute::cast_slice_mut,
     types::Opaque,
+    validate::Untrusted,
 };
 use core::{marker::PhantomData, mem::MaybeUninit, slice};
 
@@ -124,10 +126,10 @@ pub unsafe fn revert(&mut self, bytes: usize) {
     ///
     /// 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>]) };
-
+    pub fn copy_from_iter(&mut self, out: &mut [Untrusted<u8>]) -> usize {
+        // CAST: The call to `copy_from_iter_raw` below only writes initialized values.
+        // SAFETY: `Untrusted<T>` and `MaybeUninit<T>` transparently wrap a `T`.
+        let out: &mut [MaybeUninit<Untrusted<u8>>] = unsafe { cast_slice_mut(out) };
         self.copy_from_iter_raw(out).len()
     }
 
@@ -137,7 +139,7 @@ pub fn copy_from_iter(&mut self, out: &mut [u8]) -> usize {
     #[inline]
     pub fn copy_from_iter_vec<A: Allocator>(
         &mut self,
-        out: &mut Vec<u8, A>,
+        out: &mut Vec<Untrusted<u8>, A>,
         flags: Flags,
     ) -> Result<usize> {
         out.reserve(self.len(), flags)?;
@@ -152,7 +154,10 @@ pub fn copy_from_iter_vec<A: Allocator>(
     /// 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] {
+    pub fn copy_from_iter_raw(
+        &mut self,
+        out: &mut [MaybeUninit<Untrusted<u8>>],
+    ) -> &mut [Untrusted<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()) };
@@ -274,7 +279,7 @@ pub unsafe fn revert(&mut self, bytes: usize) {
     /// 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 {
+    pub fn copy_to_iter(&mut self, input: &[Untrusted<u8>]) -> usize {
         // SAFETY: `input` is valid for `input.len()` bytes.
         unsafe { bindings::_copy_to_iter(input.as_ptr().cast(), input.len(), self.as_raw()) }
     }
@@ -286,7 +291,11 @@ pub fn copy_to_iter(&mut self, input: &[u8]) -> usize {
     /// 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> {
+    pub fn simple_read_from_buffer(
+        &mut self,
+        ppos: &mut i64,
+        contents: &[Untrusted<u8>],
+    ) -> Result<usize> {
         if *ppos < 0 {
             return Err(EINVAL);
         }
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 6405713fc8ff..bd2ac2e8f13d 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -109,6 +109,7 @@
     sync::Mutex,
     types::ARef,
     uaccess::{UserSlice, UserSliceReader, UserSliceWriter},
+    validate::Untrusted,
 };
 
 const RUST_MISC_DEV_HELLO: u32 = _IO('|' as u32, 0x80);
@@ -145,7 +146,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
 
 struct Inner {
     value: i32,
-    buffer: KVec<u8>,
+    buffer: Untrusted<KVec<u8>>,
 }
 
 #[pin_data(PinnedDrop)]
@@ -169,7 +170,7 @@ fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Se
                 RustMiscDevice {
                     inner <- new_mutex!(Inner {
                         value: 0_i32,
-                        buffer: kvec![],
+                        buffer: Untrusted::new(kvec![]),
                     }),
                     dev: dev,
                 }
-- 
2.48.1

Re: [PATCH v3 3/4] rust: validate: add `Validate` trait

[Guangbo Cui]

Really cool patch! I got a few thoughts below.

> Introduce the `Validate<Input>` trait and functions to validate
> `Untrusted<T>` using said trait. This allows one to access the inner
> value of `Untrusted<T>` via `validate{,_ref,_mut}` functions which
> subsequently delegate the validation to user-implemented `Validate`
> trait.
> 
> The `Validate` trait is the only entry point for validation code, making
> it easy to spot where data is being validated.
> 
> The reason for restricting the types that can be inputs to
> `Validate::validate` is to be able to have the `validate...` functions
> on `Untrusted`. This is also the reason for the suggestions in the
> `Usage in API Design` section in the commit that introduced
> `Untrusted<T>`.
> 
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
>  rust/kernel/validate.rs | 61 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/validate.rs b/rust/kernel/validate.rs
> index 8b33716be0c7..eecac39365db 100644
> --- a/rust/kernel/validate.rs
> +++ b/rust/kernel/validate.rs
> @@ -11,6 +11,9 @@
>  //! APIs that write back into userspace usually allow writing untrusted bytes directly, allowing
>  //! direct copying of untrusted user data back into userspace without validation.
>  //!
> +//! The only way to access untrusted data is to [`Validate::validate`] it. This is facilitated by
> +//! the [`Validate`] trait.
> +//!
>  //! # Rationale
>  //!
>  //! When reading data from an untrusted source, it must be validated before it can be used for
> @@ -46,7 +49,7 @@
>  /// untrusted, as it would otherwise violate normal Rust rules. For this reason, one can easily
>  /// convert that reference to `&[Untrusted<u8>]`. Another such example is `Untrusted<KVec<T>>`, it
>  /// derefs to `KVec<Untrusted<T>>`. Raw bytes however do not behave in this way, `Untrusted<u8>` is
> -/// totally opaque.
> +/// totally opaque and one can only access its value by calling [`Untrusted::validate()`].
>  ///
>  /// # Usage in API Design
>  ///
> @@ -101,6 +104,30 @@ pub fn new(value: T) -> Self
>      {
>          Self(value)
>      }
> +
> +    /// Validate the underlying untrusted data.
> +    ///
> +    /// See the [`Validate`] trait for more information.
> +    pub fn validate<V: Validate<Self>>(self) -> Result<V, V::Err>
> +    where
> +        T: Sized,
> +    {
> +        V::validate(self.0)
> +    }
> +
> +    /// Validate the underlying untrusted data.
> +    ///
> +    /// See the [`Validate`] trait for more information.
> +    pub fn validate_ref<'a, V: Validate<&'a Self>>(&'a self) -> Result<V, V::Err> {
> +        V::validate(&self.0)
> +    }
> +
> +    /// Validate the underlying untrusted data.
> +    ///
> +    /// See the [`Validate`] trait for more information.
> +    pub fn validate_mut<'a, V: Validate<&'a mut Self>>(&'a mut self) -> Result<V, V::Err> {
> +        V::validate(&mut self.0)
> +    }
>  }

The `validate_ref` and `validate_mut` functions should just call `V::validate(self)`
directly, since self is an Untrusted<T>, and you already implemented ValidateInput for it.
Calling `V::validate(&self.0)` would cause a type mismatch error.

> +/// Marks valid input for the [`Validate`] trait.
> +pub trait ValidateInput: private::Sealed {}
> +
> +impl<T: ?Sized> ValidateInput for Untrusted<T> {}
> +
> +impl<'a, T: ?Sized> ValidateInput for &'a Untrusted<T> {}
> +
> +impl<'a, T: ?Sized> ValidateInput for &'a mut Untrusted<T> {}
> +
> +mod private {
> +    use super::Untrusted;
> +
> +    pub trait Sealed {}
> +
> +    impl<T: ?Sized> Sealed for Untrusted<T> {}
> +    impl<'a, T: ?Sized> Sealed for &'a Untrusted<T> {}
> +    impl<'a, T: ?Sized> Sealed for &'a mut Untrusted<T> {}
> +}
> +
> +/// Validate [`Untrusted`] data.
> +///
> +/// Care must be taken when implementing this trait, as unprotected access to unvalidated data is
> +/// given to the [`Validate::validate`] function. The implementer must ensure that the data is only
> +/// used for logic after successful validation.
> +pub trait Validate<Input: ValidateInput>: Sized {
> +    /// Validation error.
> +    type Err;
> +
> +    /// Validate the raw input.
> +    fn validate(raw: Input) -> Result<Self, Self::Err>;
> +}

Best regards,
Guangbo Cui

Re: [PATCH v3 1/4] rust: transmute: add `cast_slice[_mut]` functions

[Tamir Duberstein]

On Mon, Apr 21, 2025 at 9:50 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> Add functions to make casting slices only one `unsafe` block.
>
> Signed-off-by: Benno Lossin <benno.lossin@proton.me>
> ---
>  rust/kernel/transmute.rs | 41 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/rust/kernel/transmute.rs b/rust/kernel/transmute.rs
> index 1c7d43771a37..242623bbb565 100644
> --- a/rust/kernel/transmute.rs
> +++ b/rust/kernel/transmute.rs
> @@ -2,6 +2,8 @@
>
>  //! Traits for transmuting types.
>
> +use core::slice;
> +
>  /// Types for which any bit pattern is valid.
>  ///
>  /// Not all types are valid for all values. For example, a `bool` must be either zero or one, so
> @@ -69,3 +71,42 @@ macro_rules! impl_asbytes {
>      {<T: AsBytes>} [T],
>      {<T: AsBytes, const N: usize>} [T; N],
>  }
> +
> +/// Casts the type of a slice to another.
> +///
> +/// # Examples
> +///
> +/// ```rust
> +/// # use kernel::transmute::cast_slice;
> +/// #[repr(transparent)]
> +/// #[derive(Debug)]
> +/// struct Container<T>(T);
> +///
> +/// let array = [0u32; 42];
> +/// let slice = &array;
> +/// // SAFETY: `Container<T>` transparently wraps a `T`.
> +/// let container_slice = unsafe { cast_slice(slice) };
> +/// pr_info!("{container_slice}");
> +/// ```

How can this example compile? The type of `container_slice` can't
possibly be known.

> +///
> +/// # Safety
> +/// - `T` and `U` must have the same layout.
> +pub unsafe fn cast_slice<T, U>(slice: &[T]) -> &[U] {
> +    // CAST: by the safety requirements, `T` and `U` have the same layout.
> +    let ptr = slice.as_ptr().cast::<U>();
> +    // SAFETY: `ptr` and `len` come from the same slice reference.
> +    unsafe { slice::from_raw_parts(ptr, slice.len()) }
> +}
> +
> +/// Casts the type of a slice to another.
> +///
> +/// Also see [`cast_slice`].
> +///
> +/// # Safety
> +/// - `T` and `U` must have the same layout.
> +pub unsafe fn cast_slice_mut<T, U>(slice: &mut [T]) -> &mut [U] {
> +    // CAST: by the safety requirements, `T` and `U` have the same layout.
> +    let ptr = slice.as_mut_ptr().cast::<U>();
> +    // SAFETY: `ptr` and `len` come from the same slice reference.
> +    unsafe { slice::from_raw_parts_mut(ptr, slice.len()) }
> +}

With the example fixed (or if I'm mistaken and it does compile):

Reviewed-by: Tamir Duberstein <tamird@gmail.com>

Re: [PATCH v3 0/4] Untrusted Data API

[Benno Lossin]

On Mon Apr 21, 2025 at 3:49 PM CEST, Benno Lossin wrote:
> Sorry for taking so long with this, I didn't find enough time to work on
> this and when I did, I spent a lot of time on trying to get the
> interface into a satisfying state. But I always found something that I
> didn't like or something that didn't fit correctly. I blame my lack of
> knowledge about "all the ways of reading bytes in the kernel" together
> with "how should reading bytes in the kernel look like in Rust" for
> that. I think it's best I leave the ivory tower behind and just
> implement the part that I'm confident about (it's not a lot :) and then
> just roll with the use-cases from there. I should've decided to do that
> much earlier.
>
> I did manage to create a very basic validation API that I *think* is on
> the correct path, but I still marked it as an RFC just to be sure.
>
> We can & should merge the first two patches, as they allow new APIs to
> already use `Untrusted` where possible. (some APIs might of course need
> to wait for the validation API)
>
> The first use case is Alice's `struct iov_iter` series [1]. It simply
> reads untrusted data and then writes it back into userspace without
> looking at it. The RFC patch introducing that can either become a normal
> patch or be folded into Alice's series.

Seems like I forgot to change the last two commits to be RFC commits,
sorry. To be clear, these two commits are meant to be RFCs:

   rust: validate: add `Validate` trait
   rust: iov: use untrusted data API

---
Cheers,
Benno

Re: [PATCH v3 3/4] rust: validate: add `Validate` trait

[Benno Lossin]

On Mon Apr 21, 2025 at 6:47 PM CEST, Guangbo Cui wrote:
> Really cool patch! I got a few thoughts below.

Thanks!

>> +    /// Validate the underlying untrusted data.
>> +    ///
>> +    /// See the [`Validate`] trait for more information.
>> +    pub fn validate_mut<'a, V: Validate<&'a mut Self>>(&'a mut self) -> Result<V, V::Err> {
>> +        V::validate(&mut self.0)
>> +    }
>>  }
>
> The `validate_ref` and `validate_mut` functions should just call `V::validate(self)`
> directly, since self is an Untrusted<T>, and you already implemented ValidateInput for it.
> Calling `V::validate(&self.0)` would cause a type mismatch error.

Ah good catch, but the fix should be a different one. I intend to remove
the `Untrusted` wrapper in the input to `Validate::validate`, as
otherwise one would not be able to access the underlying value.

Maybe I don't need the `ValidateInput` trait after all. So the signature
of `validate{,_ref,_mut}` should ask for `V: Validate<{T, &T, &mut T}>`
instead.

---
Cheers,
Benno

Re: [PATCH v3 1/4] rust: transmute: add `cast_slice[_mut]` functions

[Benno Lossin]

On Mon Apr 21, 2025 at 8:42 PM CEST, Tamir Duberstein wrote:
> On Mon, Apr 21, 2025 at 9:50 AM Benno Lossin <benno.lossin@proton.me> wrote:
>> +/// Casts the type of a slice to another.
>> +///
>> +/// # Examples
>> +///
>> +/// ```rust
>> +/// # use kernel::transmute::cast_slice;
>> +/// #[repr(transparent)]
>> +/// #[derive(Debug)]
>> +/// struct Container<T>(T);
>> +///
>> +/// let array = [0u32; 42];
>> +/// let slice = &array;
>> +/// // SAFETY: `Container<T>` transparently wraps a `T`.
>> +/// let container_slice = unsafe { cast_slice(slice) };
>> +/// pr_info!("{container_slice}");
>> +/// ```
>
> How can this example compile? The type of `container_slice` can't
> possibly be known.

I should really start the habit of rerunning my testing scripts before
final submission... I added this example after the initial run... I
think you're correct in that it doesn't compile. Will fix for the next
version.

---
Cheers,
Benno