Re: [PATCH v5 02/23] rust: create basic untrusted data API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rust: validate: from_mut() and from_mut_slice() bypass Untrusted opacity
--

commit 46cfb0c607f570933dd68037f09805a7bea851ee
Author: Benno Lossin <lossin@kernel.org>

rust: create basic untrusted data API

This commit introduces the Untrusted<T> wrapper type in kernel::validate to
explicitly mark external data as untrusted and prevent unvalidated data from
being directly read or used for kernel logic decisions.

> diff --git a/rust/kernel/validate.rs b/rust/kernel/validate.rs
> new file mode 100644
> index 0000000000000..3c1fe37624573
> --- /dev/null
> +++ b/rust/kernel/validate.rs
> @@ -0,0 +1,192 @@

[ ... ]

> +impl<T> Untrusted<T> {
> +    /// Marks the data behind the given reference as untrusted.
> +    ///
> +    /// Unlike [`Untrusted::new`], this does not require moving the value.
> +    #[inline]
> +    pub const fn from_ref(value: &T) -> &Self {
> +        // SAFETY: `Untrusted<T>` transparently wraps `T`. The cast
> +        // does not introduce interior mutability, as `Untrusted<T>`
> +        // has interior mutability if and only if `T` has.
> +        unsafe { &*from_ref(value).cast::<Self>() }
> +    }
> +
> +    /// Marks the data behind the given mutable reference as untrusted.
> +    ///
> +    /// Unlike [`Untrusted::new`], this does not require moving the value.
> +    #[inline]
> +    pub const fn from_mut(value: &mut T) -> &mut Self {
> +        // SAFETY: Same reasoning as `from_ref()`.
> +        unsafe { &mut *from_mut(value).cast::<Self>() }
> +    }
> +
> +    /// Marks the given slice of data as untrusted.
> +    #[inline]
> +    pub const fn from_slice(value: &[T]) -> &[Self] {
> +        // SAFETY: `Untrusted<T>` transparently wraps `T`. The cast
> +        // does not introduce interior mutability either, as `Untrusted<T>`
> +        // has interior mutability if and only if `T` has.
> +        unsafe { slice::from_raw_parts(value.as_ptr().cast::<Self>(), value.len()) }
> +    }
> +
> +    /// Marks the given mutable slice of data as untrusted.
> +    #[inline]
> +    pub const fn from_mut_slice(value: &mut [T]) -> &mut [Self] {
> +        // SAFETY: Same reasoning as `from_slice()`.
> +        unsafe { slice::from_raw_parts_mut(value.as_mut_ptr().cast::<Self>(), value.len()) }
> +    }
> +}

[Severity: Medium]
Do public Untrusted::from_mut() and Untrusted::from_mut_slice() allow safe
code to bypass the opacity guarantee of Untrusted<T>?

Because mutable references are invariant in T, converting &mut T into
&mut Untrusted<T> (or &mut [T] into &mut [Untrusted<T>]) allows callers to
write unvalidated Untrusted<T> values directly into a plain, trusted T or [T]
variable without validation:

    let mut trusted: u8 = 0;
    *Untrusted::from_mut(&mut trusted) = untrusted_byte;

Similarly, a caller with a plain mutable slice buf: &mut [u8] can pass
Untrusted::from_mut_slice(buf) to a function expecting &mut [Untrusted<u8>]
(such as the read_from_userspace(buf: &mut [Untrusted<u8>]) pattern shown in
the documentation) and then read the unvalidated bytes directly from buf
after the call returns.

Since DerefMut on Untrusted<[T]> and Untrusted<Vec<T, A>> only converts an
already-untrusted container into &mut [Untrusted<T>], should from_mut_slice()
be private and from_mut() be removed or kept private?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027052536.1722081-1-alistair.francis@wdc.com?part=2