Re: [PATCH v3 03/21] rust: validate: add `Validate` trait

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The small wrapper functions `validate`, `validate_ref`, and `validate_mut` in the `Untrusted` type are missing the `#[inline]` attribute, violating subsystem guidelines.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but transmuting references between `Vec<T, A>` and `Vec<Untrusted<T>, A>` via pointer cast is Undefined Behavior because `Vec` lacks a guaranteed memory layout.
--

commit 6a4bc1cc90cb4c5dd96a67a8f893828d2d0c3473
Author: Benno Lossin <lossin@kernel.org>

rust: validate: add `Validate` trait

Introduce the `Validate<Input>` trait and functions to validate
`Untrusted<T>` using said trait.

> diff --git a/rust/kernel/validate.rs b/rust/kernel/validate.rs
> index 2b28625c25efe..663681b633c86 100644
> --- a/rust/kernel/validate.rs
> +++ b/rust/kernel/validate.rs
> @@ -107,6 +110,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)
> +    }

[Severity: Low]
Should these small wrapper functions be annotated with #[inline]?

[ ... ]

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

[Severity: High]
This isn't a bug introduced by this patch, but does casting pointers between
Vec<T, A> and Vec<Untrusted<T>, A> cause Undefined Behavior?

Since Vec<T, A> uses the default #[repr(rust)] representation, it does not
guarantee stable field ordering. The Rust Reference explicitly states that
pointer-casting between two generic types with the default representation is
Undefined Behavior, even if their type arguments share identical memory
layouts.

If the compiler orders fields differently between Vec<T, A> and
Vec<Untrusted<T>, A>, could dereferencing the casted pointer or swapping
values corrupt memory?

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