Re: [PATCH v2 02/21] rust: create basic untrusted data API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Invalid pointer cast between `Vec<T, A>` and `Vec<Untrusted<T>, A>` in `Deref` and `DerefMut` implementations leads to undefined behavior, memory corruption, and Use-After-Free.
- [Low] Missing `#[inline]` annotations on small forwarding functions in an abstraction.
- [Low] Incorrect import formatting: `use core::ops::{Deref, DerefMut};` does not follow the vertical import style.
--

commit 5d58092058655501597b482a4a94d2d50da29fc6
Author: Benno Lossin <lossin@kernel.org>

rust: create basic untrusted data API

This commit introduces the `Untrusted<T>` wrapper type to explicitly mark and
validate untrusted external data. It prevents direct usage of untrusted data
in logic decisions while allowing it to be copied back to userspace.

Message-ID: <20250814124424.516191-3-lossin@kernel.org>

diff --git a/rust/kernel/validate.rs b/rust/kernel/validate.rs
new file mode 100644
index 0000000000000..2b28625c25efe
--- /dev/null
+++ b/rust/kernel/validate.rs
[ ... ]
> +use core::ops::{Deref, DerefMut};

[Severity: Low]
This isn't a bug, but the kernel Rust coding guidelines require vertical import
formatting (one import per line). Should this be spread across multiple lines?

> +
> +use crate::{
[ ... ]
> +impl<T: ?Sized> Untrusted<T> {
[ ... ]
> +    pub fn new(value: T) -> Self

[Severity: Low]
This isn't a bug, but the kernel Rust coding guidelines require small or
forwarding functions in abstractions to be annotated with `#[inline]`. Should
we add the annotation here?

> +    where
> +        T: Sized,
> +    {
> +        Self(value)
> +    }
> +}
> +
> +impl<T> Deref for Untrusted<[T]> {
> +    type Target = [Untrusted<T>];
> +
> +    fn deref(&self) -> &Self::Target {

[Severity: Low]
Similar to `new()`, should this small forwarding function be annotated with
`#[inline]` to match the guidelines?

> +        // 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 {

[Severity: Low]
Should this small forwarding function also be annotated with `#[inline]`?

> +        // 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 }

[Severity: High]
Does this pointer cast invoke undefined behavior?

Since `Vec` has `#[repr(Rust)]`, the compiler provides no guarantee that the
field orderings of `Vec<T>` and `Vec<Untrusted<T>>` match. If the layout
differs between these two monomorphizations, could casting the reference result
in memory corruption when accessing the `ptr`, `len`, and `layout` fields?

> +    }
> +}
> +
> +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 }

[Severity: High]
Similar to `deref()`, does casting a mutable reference between different
instantiations of a `#[repr(Rust)]` type risk memory corruption?

Safe code could call `deref_mut` to obtain `&mut Vec<Untrusted<T>, A>`, extract
ownership via `core::mem::take` (since `Vec` implements `Default`), and then
mutate or drop it. This could process the fields from mismatched offsets and
pass garbage pointers to the allocator.

> +    }
> +}

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