Re: [PATCH v2 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:
- [Medium] The `Validate` trait's design makes it impossible to validate stack-allocated array buffers as a single contiguous chunk.
- [Low] Small forwarding functions in a core abstraction are missing the required `#[inline]` annotation.
--

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

rust: validate: add Validate trait

This commit introduces the Validate trait and functions to validate
Untrusted data. It allows users to access the inner value of untrusted
data via validation functions which delegate to a user-implemented
trait. It also uses a sealed trait to restrict valid inputs.

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

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

[Severity: Low]
This isn't a bug, but should this small forwarding function in a core
abstraction be annotated with #[inline] per the Rust subsystem coding
guidelines?

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

[Severity: Low]
This isn't a bug, but should this small forwarding function be annotated
with #[inline] as well?

> +
> +    /// 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]
This isn't a bug, but should this small forwarding function also be annotated
with #[inline]?

[ ... ]

> +/// Marks valid input for the [`Validate`] trait.
> +pub trait ValidateInput: private::Sealed {
> +    /// Type of the inner data.
> +    type Inner: ?Sized;
> +}
> +
> +impl<T: ?Sized> ValidateInput for Untrusted<T> {
> +    type Inner = T;
> +}
> +
> +impl<'a, T: ?Sized> ValidateInput for &'a Untrusted<T> {
> +    type Inner = &'a T;
> +}
> +
> +impl<'a, T: ?Sized> ValidateInput for &'a mut Untrusted<T> {
> +    type Inner = &'a mut T;
> +}

[Severity: Medium]
Does the current ValidateInput implementation prevent validating
stack-allocated array buffers as a single contiguous chunk?

If a user populates a stack-allocated buffer like [Untrusted<u8>; N], they
can use it as a slice &mut [Untrusted<u8>]. However, since ValidateInput
is sealed and only implements Untrusted<T> wrappers (and not slices or
arrays of wrappers), they cannot pass this slice to Validate::validate().

Since converting &mut [Untrusted<u8>] back to &mut Untrusted<[u8]> isn't
possible outside the module, this seems to force users to either validate
byte-by-byte or dynamically allocate. 

Could ValidateInput be implemented for slices of Untrusted<T> to support
validating stack-allocated buffers?

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