lib: Rust implementation of SPDM

From: Benno Lossin <[email protected]>

When reading data from userspace, hardware or other external untrusted sources, the data must be validated before it is used for logic within the kernel. This abstraction provides a generic newtype wrapper Untrusted; it prevents direct access to the inner type. The only way to use the underlying data is to call .validate() on such a value.

Doing so utilizes the new Validate trait that is responsible for all of the validation logic. This trait gives access to the inner value of Untrusted by means of another newtype wrapper Unvalidated. In contrast to Untrusted, Unvalidated allows direct access and additionally provides several helper functions for slices.

Having these two different newtype wrappers is an idea from Simona Vetter. It has several benefits: it fully prevents safe access to the underlying value of Untrusted without going through the Validate API. Additionally, it allows one to grep for validation logic by simply looking for Unvalidated<.

Any API that reads data from an untrusted source should return Untrusted<T> where T is the type of the underlying untrusted data. This generic allows other abstractions to return their custom type wrapped by Untrusted, signaling to the caller that the data must be validated before use. This allows those abstractions to be used both in a trusted and untrusted manner, increasing their generality. Additionally, using the arbitrary self types feature, APIs can be designed to explicitly read untrusted data:

     impl MyCustomDataSource {
         pub fn read(self: &Untrusted<Self>) -> &Untrusted<[u8]>;
     }

Cc Simona Vetter <[email protected]>

Signed-off-by Benno Lossin <[email protected]>

Message-ID: <[email protected]>

---
  rust/kernel/lib.rs      |   1 +
  rust/kernel/validate.rs | 605 ++++++++++++++++++++++++++++++++++++++++
  2 files changed, 606 insertions(+)

create mode 100644 rust/kernel/validate.rs

rust/kernel/lib.rs +1 -0

index b72b2fbe046d..fe580fde609f 100644
@@ -133,6 +133,7 @@
  pub mod uaccess;
  #[cfg(CONFIG_USB = "y")]
  pub mod usb;
+pub mod validate;
  pub mod workqueue;
  pub mod xarray;
 

rust/kernel/validate.rs new file +605 -0

new file mode 100644
index 000000000000..ae0aa20e27b4
@@ -0,0 +1,605 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Types for handling and validating untrusted data.
+//!
+//! # 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; but it is possible to [`Untrusted::write`] into it.
+//!
+//! The only way to "access" the data inside an [`Untrusted<T>`] is to [`Untrusted::validate`] it;
+//! turning it into a different form using the [`Validate`] trait. That trait receives the data in
+//! the form of [`Unvalidated<T>`], which in contrast to [`Untrusted<T>`], allows access to the
+//! underlying data. It additionally provides several utility functions to simplify 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 one userspace buffer into another is allowed for untrusted data.
+
+use crate::prelude::Init;
+use core::{
+    mem::MaybeUninit,
+    ops::{Index, IndexMut},
+    ptr, slice,
+};
+
+/// Untrusted data of type `T`.
+///
+/// When reading data from userspace, hardware or other external untrusted sources, the data must
+/// be validated before it is used for logic within the kernel. To do so, the [`validate()`]
+/// function exists and uses the [`Validate`] trait.
+///
+/// Also see the [module] description.
+///
+/// [`validate()`]: Self::validate
+/// [module]: self
+#[repr(transparent)]
+pub struct Untrusted<T: ?Sized>(Unvalidated<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(Unvalidated::new(value))
+    }
+
+    /// Marks the value behind the reference as untrusted.
+    ///
+    /// # Examples
+    ///
+    /// In this imaginary example there exists the `foo_hardware` struct on the C side, as well as
+    /// a `foo_hardware_read` function that reads some data directly from the hardware.
+    /// ```
+    /// use kernel::{error, types::Opaque, validate::Untrusted};
+    /// use core::ptr;
+    ///
+    /// # #[allow(non_camel_case_types)]
+    /// # mod bindings {
+    /// #     pub(crate) struct foo_hardware;
+    /// #     pub(crate) unsafe fn foo_hardware_read(_foo: *mut foo_hardware, _len: &mut usize) -> *mut u8 {
+    /// #         todo!()
+    /// #     }
+    /// # }
+    /// struct Foo(Opaque<bindings::foo_hardware>);
+    ///
+    /// impl Foo {
+    ///     fn read(&mut self, mut len: usize) -> Result<&Untrusted<[u8]>> {
+    ///         // SAFETY: just an FFI call without preconditions.
+    ///         let data: *mut u8 = unsafe { bindings::foo_hardware_read(self.0.get(), &mut len) };
+    ///         let data = error::from_err_ptr(data)?;
+    ///         let data = ptr::slice_from_raw_parts(data, len);
+    ///         // SAFETY: `data` returned by `foo_hardware_read` is valid for reads as long as the
+    ///         // `foo_hardware` object exists. That function updated the
+    ///         let data = unsafe { &*data };
+    ///         Ok(Untrusted::new_ref(data))
+    ///     }
+    /// }
+    /// ```
+    pub fn new_ref(value: &T) -> &Self {
+        let ptr: *const T = value;
+        // CAST: `Self` and `Unvalidated` are `repr(transparent)` and contain a `T`.
+        let ptr = ptr as *const Self;
+        // SAFETY: `ptr` came from a shared reference valid for `'a`.
+        unsafe { &*ptr }
+    }
+
+    /// Marks the value behind the reference as untrusted.
+    ///
+    /// # Examples
+    ///
+    /// In this imaginary example there exists the `foo_hardware` struct on the C side, as well as
+    /// a `foo_hardware_read` function that reads some data directly from the hardware.
+    /// ```
+    /// use kernel::{error, types::Opaque, validate::Untrusted};
+    /// use core::ptr;
+    ///
+    /// # #[allow(non_camel_case_types)]
+    /// # mod bindings {
+    /// #     pub(crate) struct foo_hardware;
+    /// #     pub(crate) unsafe fn foo_hardware_read(_foo: *mut foo_hardware, _len: &mut usize) -> *mut u8 {
+    /// #         todo!()
+    /// #     }
+    /// # }
+    /// struct Foo(Opaque<bindings::foo_hardware>);
+    ///
+    /// impl Foo {
+    ///     fn read(&mut self, mut len: usize) -> Result<&mut Untrusted<[u8]>> {
+    ///         // SAFETY: just an FFI call without preconditions.
+    ///         let data: *mut u8 = unsafe { bindings::foo_hardware_read(self.0.get(), &mut len) };
+    ///         let data = error::from_err_ptr(data)?;
+    ///         let data = ptr::slice_from_raw_parts_mut(data, len);
+    ///         // SAFETY: `data` returned by `foo_hardware_read` is valid for reads as long as the
+    ///         // `foo_hardware` object exists. That function updated the
+    ///         let data = unsafe { &mut *data };
+    ///         Ok(Untrusted::new_mut(data))
+    ///     }
+    /// }
+    /// ```
+    pub fn new_mut(value: &mut T) -> &mut Self {
+        let ptr: *mut T = value;
+        // CAST: `Self` and `Unvalidated` are `repr(transparent)` and contain a `T`.
+        let ptr = ptr as *mut Self;
+        // SAFETY: `ptr` came from a mutable reference valid for `'a`.
+        unsafe { &mut *ptr }
+    }
+
+    /// Validates and parses the untrusted data.
+    ///
+    /// See the [`Validate`] trait on how to implement it.
+    pub fn validate<'a, V: Validate<&'a Unvalidated<T>>>(&'a self) -> Result<V, V::Err> {
+        V::validate(&self.0)
+    }
+
+    /// Validates and parses the untrusted data.
+    ///
+    /// See the [`Validate`] trait on how to implement it.
+    pub fn validate_mut<'a, V: Validate<&'a mut Unvalidated<T>>>(
+        &'a mut self,
+    ) -> Result<V, V::Err> {
+        V::validate(&mut self.0)
+    }
+
+    /// Sets the underlying untrusted value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::validate::Untrusted;
+    ///
+    /// let mut untrusted = Untrusted::new(42);
+    /// untrusted.write(24);
+    /// ```
+    pub fn write(&mut self, value: impl Init<T>) {
+        let ptr: *mut T = &mut self.0 .0;
+        // SAFETY: `ptr` came from a mutable reference and the value is overwritten before it is
+        // read.
+        unsafe { ptr::drop_in_place(ptr) };
+        // SAFETY: `ptr` came from a mutable reference and the initializer cannot error.
+        match unsafe { value.__init(ptr) } {
+            Ok(()) => {}
+            Err(_) => unreachable!(),
+        }
+    }
+
+    /// Turns a slice of untrusted values into an untrusted slice of values.
+    pub fn transpose_slice(slice: &[Untrusted<T>]) -> &Untrusted<[T]>
+    where
+        T: Sized,
+    {
+        let ptr = slice.as_ptr().cast::<T>();
+        // SAFETY: `ptr` and `len` come from the same slice reference.
+        let slice = unsafe { slice::from_raw_parts(ptr, slice.len()) };
+        Untrusted::new_ref(slice)
+    }
+
+    /// Turns a slice of uninitialized, untrusted values into an untrusted slice of uninitialized
+    /// values.
+    pub fn transpose_slice_uninit(
+        slice: &[MaybeUninit<Untrusted<T>>],
+    ) -> &Untrusted<[MaybeUninit<T>]>
+    where
+        T: Sized,
+    {
+        let ptr = slice.as_ptr().cast::<MaybeUninit<T>>();
+        // SAFETY: `ptr` and `len` come from the same mutable slice reference.
+        let slice = unsafe { slice::from_raw_parts(ptr, slice.len()) };
+        Untrusted::new_ref(slice)
+    }
+
+    /// Turns a slice of uninitialized, untrusted values into an untrusted slice of uninitialized
+    /// values.
+    pub fn transpose_slice_uninit_mut(
+        slice: &mut [MaybeUninit<Untrusted<T>>],
+    ) -> &mut Untrusted<[MaybeUninit<T>]>
+    where
+        T: Sized,
+    {
+        // CAST: `MaybeUninit<T>` and `MaybeUninit<Untrusted<T>>` have the same layout.
+        let ptr = slice.as_mut_ptr().cast::<MaybeUninit<T>>();
+        // SAFETY: `ptr` and `len` come from the same mutable slice reference.
+        let slice = unsafe { slice::from_raw_parts_mut(ptr, slice.len()) };
+        Untrusted::new_mut(slice)
+    }
+}
+
+impl<T> Untrusted<MaybeUninit<T>> {
+    /// Sets the underlying untrusted value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::validate::Untrusted;
+    ///
+    /// let mut untrusted = Untrusted::new(42);
+    /// untrusted.write(24);
+    /// ```
+    pub fn write_uninit<E>(&mut self, value: impl Init<T, E>) -> Result<&mut Untrusted<T>, E> {
+        let ptr: *mut MaybeUninit<T> = &mut self.0 .0;
+        // CAST: `MaybeUninit<T>` is `repr(transparent)`.
+        let ptr = ptr.cast::<T>();
+        // SAFETY: `ptr` came from a reference and if `Err` is returned, the underlying memory is
+        // considered uninitialized.
+        unsafe { value.__init(ptr) }.map(|()| {
+            let this = self.0.raw_mut();
+            // SAFETY: we initialized the memory above.
+            Untrusted::new_mut(unsafe { this.assume_init_mut() })
+        })
+    }
+}
+
+impl<T> Untrusted<[MaybeUninit<T>]> {
+    /// Sets the underlying untrusted value.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use kernel::validate::Untrusted;
+    ///
+    /// let mut untrusted = Untrusted::new(42);
+    /// untrusted.write(24);
+    /// ```
+    pub fn write_uninit_slice<E>(
+        &mut self,
+        value: impl Init<[T], E>,
+    ) -> Result<&mut Untrusted<[T]>, E> {
+        let ptr: *mut [MaybeUninit<T>] = &mut self.0 .0;
+        // CAST: `MaybeUninit<T>` is `repr(transparent)`.
+        let ptr = ptr as *mut [T];
+        // SAFETY: `ptr` came from a reference and if `Err` is returned, the underlying memory is
+        // considered uninitialized.
+        unsafe { value.__init(ptr) }.map(|()| {
+            let this = self.0.raw_mut().as_mut_ptr();
+            // CAST: `MaybeUninit<T>` is `repr(transparent)`.
+            let this = this.cast::<T>();
+            // SAFETY: `this` and `len` came from the same slice reference.
+            let this = unsafe { slice::from_raw_parts_mut(this, self.0.len()) };
+            Untrusted::new_mut(this)
+        })
+    }
+}
+
+/// Marks types that can be used as input to [`Validate::validate`].
+pub trait ValidateInput: private::Sealed + Sized {}
+
+mod private {
+    pub trait Sealed {}
+}
+
+impl<'a, T: ?Sized> private::Sealed for &'a Unvalidated<T> {}
+impl<'a, T: ?Sized> ValidateInput for &'a Unvalidated<T> {}
+
+impl<'a, T: ?Sized> private::Sealed for &'a mut Unvalidated<T> {}
+impl<'a, T: ?Sized> ValidateInput for &'a mut Unvalidated<T> {}
+
+/// Validates untrusted data.
+///
+/// # Examples
+///
+/// The simplest way to validate data is to just implement `Validate<&Unvalidated<[u8]>>` for the
+/// type that you wish to validate:
+///
+/// ```
+/// use kernel::{
+///     error::{code::EINVAL, Error},
+///     str::{CStr, CString},
+///     validate::{Unvalidated, Validate},
+/// };
+///
+/// struct Data {
+///     flags: u8,
+///     name: CString,
+/// }
+///
+/// impl Validate<&Unvalidated<[u8]>> for Data {
+///     type Err = Error;
+///
+///     fn validate(unvalidated: &Unvalidated<[u8]>) -> Result<Self, Self::Err> {
+///         let raw = unvalidated.raw();
+///         let (&flags, name) = raw.split_first().ok_or(EINVAL)?;
+///         let name = CStr::from_bytes_with_nul(name)?.to_cstring()?;
+///         Ok(Data { flags, name })
+///     }
+/// }
+/// ```
+///
+/// This approach copies the data and requires allocation. If you want to avoid the allocation and
+/// copying the data, you can borrow from the input like this:
+///
+/// ```
+/// use kernel::{
+///     error::{code::EINVAL, Error},
+///     str::CStr,
+///     validate::{Unvalidated, Validate},
+/// };
+///
+/// struct Data<'a> {
+///     flags: u8,
+///     name: &'a CStr,
+/// }
+///
+/// impl<'a> Validate<&'a Unvalidated<[u8]>> for Data<'a> {
+///     type Err = Error;
+///
+///     fn validate(unvalidated: &'a Unvalidated<[u8]>) -> Result<Self, Self::Err> {
+///         let raw = unvalidated.raw();
+///         let (&flags, name) = raw.split_first().ok_or(EINVAL)?;
+///         let name = CStr::from_bytes_with_nul(name)?;
+///         Ok(Data { flags, name })
+///     }
+/// }
+/// ```
+///
+/// If you need to in-place validate your data, you currently need to resort to `unsafe`:
+///
+/// ```
+/// use kernel::{
+///     error::{code::EINVAL, Error},
+///     str::CStr,
+///     validate::{Unvalidated, Validate},
+/// };
+/// use core::mem;
+///
+/// // Important: use `repr(C)`, this ensures a linear layout of this type.
+/// #[repr(C)]
+/// struct Data {
+///     version: u8,
+///     flags: u8,
+///     _reserved: [u8; 2],
+///     count: u64,
+///     // lots of other fields...
+/// }
+///
+/// impl Validate<&Unvalidated<[u8]>> for &Data {
+///     type Err = Error;
+///
+///     fn validate(unvalidated: &Unvalidated<[u8]>) -> Result<Self, Self::Err> {
+///         let raw = unvalidated.raw();
+///         if raw.len() < mem::size_of::<Data>() {
+///             return Err(EINVAL);
+///         }
+///         // can only handle version 0
+///         if raw[0] != 0 {
+///             return Err(EINVAL);
+///         }
+///         // version 0 only uses the lower 4 bits of flags
+///         if raw[1] & 0xf0 != 0 {
+///             return Err(EINVAL);
+///         }
+///         let ptr = raw.as_ptr();
+///         // CAST: `Data` only contains integers and has `repr(C)`.
+///         let ptr = ptr.cast::<Data>();
+///         // SAFETY: `ptr` came from a reference and the cast above is valid.
+///         Ok(unsafe { &*ptr })
+///     }
+/// }
+/// ```
+///
+/// To be able to modify the parsed data, while still supporting zero-copy, you can implement
+/// `Validate<&mut Unvalidated<[u8]>>`:
+///
+/// ```
+/// use kernel::{
+///     error::{code::EINVAL, Error},
+///     str::CStr,
+///     validate::{Unvalidated, Validate},
+/// };
+/// use core::mem;
+///
+/// // Important: use `repr(C)`, this ensures a linear layout of this type.
+/// #[repr(C)]
+/// struct Data {
+///     version: u8,
+///     flags: u8,
+///     _reserved: [u8; 2],
+///     count: u64,
+///     // lots of other fields...
+/// }
+///
+/// impl Validate<&mut Unvalidated<[u8]>> for &Data {
+///     type Err = Error;
+///
+///     fn validate(unvalidated: &mut Unvalidated<[u8]>) -> Result<Self, Self::Err> {
+///         let raw = unvalidated.raw_mut();
+///         if raw.len() < mem::size_of::<Data>() {
+///             return Err(EINVAL);
+///         }
+///         match raw[0] {
+///             0 => {},
+///             1 => {
+///                 // version 1 implicitly sets the first bit.
+///                 raw[1] |= 1;
+///             },
+///             // can only handle version 0 and 1
+///             _ => return Err(EINVAL),
+///         }
+///         // version 0 and 1 only use the lower 4 bits of flags
+///         if raw[1] & 0xf0 != 0 {
+///             return Err(EINVAL);
+///         }
+///         if raw[1] == 0 {}
+///         let ptr = raw.as_ptr();
+///         // CAST: `Data` only contains integers and has `repr(C)`.
+///         let ptr = ptr.cast::<Data>();
+///         // SAFETY: `ptr` came from a reference and the cast above is valid.
+///         Ok(unsafe { &*ptr })
+///     }
+/// }
+/// ```
+pub trait Validate<I: ValidateInput>: Sized {
+    /// Validation error.
+    type Err;
+
+    /// Validate the given untrusted data and parse it into the output type.
+    fn validate(unvalidated: I) -> Result<Self, Self::Err>;
+}
+
+/// Unvalidated data of type `T`.
+#[repr(transparent)]
+pub struct Unvalidated<T: ?Sized>(T);
+
+impl<T: ?Sized> Unvalidated<T> {
+    fn new(value: T) -> Self
+    where
+        T: Sized,
+    {
+        Self(value)
+    }
+
+    fn new_ref(value: &T) -> &Self {
+        let ptr: *const T = value;
+        // CAST: `Self` is `repr(transparent)` and contains a `T`.
+        let ptr = ptr as *const Self;
+        // SAFETY: `ptr` came from a mutable reference valid for `'a`.
+        unsafe { &*ptr }
+    }
+
+    fn new_mut(value: &mut T) -> &mut Self {
+        let ptr: *mut T = value;
+        // CAST: `Self` is `repr(transparent)` and contains a `T`.
+        let ptr = ptr as *mut Self;
+        // SAFETY: `ptr` came from a mutable reference valid for `'a`.
+        unsafe { &mut *ptr }
+    }
+
+    /// Validates and parses the untrusted data.
+    ///
+    /// See the [`Validate`] trait on how to implement it.
+    pub fn validate_ref<'a, V: Validate<&'a Unvalidated<T>>>(&'a self) -> Result<V, V::Err> {
+        V::validate(self)
+    }
+
+    /// Validates and parses the untrusted data.
+    ///
+    /// See the [`Validate`] trait on how to implement it.
+    pub fn validate_mut<'a, V: Validate<&'a mut Unvalidated<T>>>(
+        &'a mut self,
+    ) -> Result<V, V::Err> {
+        V::validate(self)
+    }
+
+    /// Gives immutable access to the underlying value.
+    pub fn raw(&self) -> &T {
+        &self.0
+    }
+
+    /// Gives mutable access to the underlying value.
+    pub fn raw_mut(&mut self) -> &mut T {
+        &mut self.0
+    }
+}
+
+impl<T, I> Index<I> for Unvalidated<[T]>
+where
+    I: slice::SliceIndex<[T]>,
+{
+    type Output = Unvalidated<I::Output>;
+
+    fn index(&self, index: I) -> &Self::Output {
+        Unvalidated::new_ref(self.0.index(index))
+    }
+}
+
+impl<T, I> IndexMut<I> for Unvalidated<[T]>
+where
+    I: slice::SliceIndex<[T]>,
+{
+    fn index_mut(&mut self, index: I) -> &mut Self::Output {
+        Unvalidated::new_mut(self.0.index_mut(index))
+    }
+}
+
+/// Immutable unvalidated slice iterator.
+pub struct Iter<'a, T>(slice::Iter<'a, T>);
+
+/// Mutable unvalidated slice iterator.
+pub struct IterMut<'a, T>(slice::IterMut<'a, T>);
+
+impl<'a, T> Iterator for Iter<'a, T> {
+    type Item = &'a Unvalidated<T>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next().map(Unvalidated::new_ref)
+    }
+}
+
+impl<'a, T> IntoIterator for &'a Unvalidated<[T]> {
+    type Item = &'a Unvalidated<T>;
+    type IntoIter = Iter<'a, T>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        Iter(self.0.iter())
+    }
+}
+
+impl<'a, T> Iterator for IterMut<'a, T> {
+    type Item = &'a mut Unvalidated<T>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next().map(Unvalidated::new_mut)
+    }
+}
+
+impl<'a, T> IntoIterator for &'a mut Unvalidated<[T]> {
+    type Item = &'a mut Unvalidated<T>;
+    type IntoIter = IterMut<'a, T>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        IterMut(self.0.iter_mut())
+    }
+}
+
+impl<T> Unvalidated<[T]> {
+    /// Returns the number of elements in the underlying slice.
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+
+    /// Returns true if the underlying slice has a length of 0.
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    /// Iterates over all items and validates each of them individually.
+    pub fn validate_iter<'a, V: Validate<&'a Unvalidated<T>>>(
+        &'a self,
+    ) -> impl Iterator<Item = Result<V, V::Err>> + 'a {
+        self.into_iter().map(|item| V::validate(item))
+    }
+
+    /// Iterates over all items and validates each of them individually.
+    pub fn validate_iter_mut<'a, V: Validate<&'a mut Unvalidated<T>>>(
+        &'a mut self,
+    ) -> impl Iterator<Item = Result<V, V::Err>> + 'a {
+        self.into_iter().map(|item| V::validate(item))
+    }
+}

Hi Alistair,

Quoted 29 lines · On 08.05.2026 05:16, [email protected] wrote:

From: Benno Lossin <[email protected]>

When reading data from userspace, hardware or other external untrusted sources, the data must be validated before it is used for logic within the kernel. This abstraction provides a generic newtype wrapper Untrusted; it prevents direct access to the inner type. The only way to use the underlying data is to call .validate() on such a value.

Doing so utilizes the new Validate trait that is responsible for all of the validation logic. This trait gives access to the inner value of Untrusted by means of another newtype wrapper Unvalidated. In contrast to Untrusted, Unvalidated allows direct access and additionally provides several helper functions for slices.

Having these two different newtype wrappers is an idea from Simona Vetter. It has several benefits: it fully prevents safe access to the underlying value of Untrusted without going through the Validate API. Additionally, it allows one to grep for validation logic by simply looking for Unvalidated<.

Any API that reads data from an untrusted source should return Untrusted<T> where T is the type of the underlying untrusted data. This generic allows other abstractions to return their custom type wrapped by Untrusted, signaling to the caller that the data must be validated before use. This allows those abstractions to be used both in a trusted and untrusted manner, increasing their generality. Additionally, using the arbitrary self types feature, APIs can be designed to explicitly read untrusted data:

Quoted 7 lines

     impl MyCustomDataSource {
         pub fn read(self: &Untrusted<Self>) -> &Untrusted<[u8]>;
     }

Cc Simona Vetter <[email protected]>

Signed-off-by Benno Lossin <[email protected]>

Message-ID: <[email protected]>

Today, randomly, I was about to ask Benno if there are any plans to continue with his great untrusted work. Just to find that you are carrying this forward. Many thanks!

While trying to figure out some history of this patch, it looks to me that the version you are using is Benno's v2:

https://lore.kernel.org/rust-for-linux/[email protected]/

?

While the latest version from Benno I found is a v4:

https://lore.kernel.org/rust-for-linux/[email protected]/

Is there any reason not to use that?

While at this, do you have any plan to add the Validate trait as well?

https://lore.kernel.org/rust-for-linux/[email protected]/

Thanks

Dirk

P.S.: Carrying this patch forward, please check if it needs an update regarding new "rules". E.g. it looks to me that there should be an update regarding the vertical style for imports?

https://docs.kernel.org/rust/coding-guidelines.html#imports

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims to adjust the #include <linux/time.h> directive to #include <linux/time64.h>, but the patch fails to do so.

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The exported helper x509_get_certificate_length() uses a flawed logical condition (&& instead of ||) that fails to properly validate ASN.1 headers, leading to parser differentials, incorrect length calculations, and functional failures.

From: Alistair Francis <[email protected]>

In the next patch we are going to add a PCIe CMA TSM driver, as such we need to ensure that is_pci_tsm_pf0() will allow us to connect to CMA capable devices. These devices don't necessarily has DEVCAP_TEE or IDE support.

As such for Root Complex Integrated Endpoint (PCI_EXP_TYPE_RC_END) we also check for the CMA DOE feature.

Signed-off-by Alistair Francis <[email protected]>

---
 include/linux/pci-tsm.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

include/linux/pci-tsm.h +10 -2

index a6435aba03f9..5059954e4853 100644
@@ -3,6 +3,7 @@
 #define __PCI_TSM_H
 #include <linux/mutex.h>
 #include <linux/pci.h>
+#include <linux/pci-doe.h>
 #include <linux/sockptr.h>
 
 struct pci_tsm;

@@ -129,6 +130,8 @@ struct pci_tsm_pf0 {
 /* physical function0 and capable of 'connect' */
 static inline bool is_pci_tsm_pf0(struct pci_dev *pdev)
 {
+       struct pci_doe_mb *doe;
+
        if (!pdev)
                return false;
 

@@ -146,9 +149,15 @@ static inline bool is_pci_tsm_pf0(struct pci_dev *pdev)
         * switch.
         */
        switch (pci_pcie_type(pdev)) {
+       case PCI_EXP_TYPE_RC_END:
+               doe = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
+                                          PCI_DOE_FEATURE_CMA);
+
+               if (doe)
+                       break;
+               fallthrough;
        case PCI_EXP_TYPE_ENDPOINT:
        case PCI_EXP_TYPE_UPSTREAM:
-       case PCI_EXP_TYPE_RC_END:
                if (pdev->ide_cap || (pdev->devcap & PCI_EXP_DEVCAP_TEE))
                        break;
                fallthrough;
--

2.52.0

Quoted 10 lines · On Fri, May 8, 2026 at 1:18 PM <[email protected]> wrote:

From: Alistair Francis <[email protected]>

In the next patch we are going to add a PCIe CMA TSM driver, as such we need to ensure that is_pci_tsm_pf0() will allow us to connect to CMA capable devices. These devices don't necessarily has DEVCAP_TEE or IDE support.

As such for Root Complex Integrated Endpoint (PCI_EXP_TYPE_RC_END) we also check for the CMA DOE feature.

@Dan Williams this is the patch I really need your thoughts on.

The current upstream pci_tsm code only works if IDE or TDISP is supported, which isn't true for CMA support.

This patch works around that, but the more I think about it the hackier it is. I have a local change that reverts this and updatestsm.c to work with a DSM (->dsm_dev is NULL), but that doesn't feel right either. Do you have a better idea or how to enable a CMA TSM driver?

Alistair

Quoted 18 lines · Alistair Francis

Quoted 10 lines · On Fri, May 8, 2026 at 1:18 PM <[email protected]> wrote:

From: Alistair Francis <[email protected]>

In the next patch we are going to add a PCIe CMA TSM driver, as such we need to ensure that is_pci_tsm_pf0() will allow us to connect to CMA capable devices. These devices don't necessarily has DEVCAP_TEE or IDE support.

As such for Root Complex Integrated Endpoint (PCI_EXP_TYPE_RC_END) we also check for the CMA DOE feature.

@Dan Williams this is the patch I really need your thoughts on.

The current upstream pci_tsm code only works if IDE or TDISP is supported, which isn't true for CMA support.

Quoted 5 lines

This patch works around that, but the more I think about it the hackier it is. I have a local change that reverts this and updatestsm.c to work with a DSM (->dsm_dev is NULL), but that doesn't feel right either. Do you have a better idea or how to enable a CMA TSM driver?

Gentle ping on this. Any thoughts here?

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Valid devices with a DSN of 0 are incorrectly rejected
- [High] SSVID and SSID fields are incorrectly omitted for PCIe bridges
- [High] Torn read of device identity prevents reauthentication after runtime mutations
- [High] Kbuild linkage failure when compiled as a loadable module
- [Medium] Chaining snprintf without length checks risks logic errors and out-of-bounds stack reads
- [Medium] Strict case-sensitive comparison of the Subject Alternative Name risks rejecting valid certificates --

commit 72435325efe665bb825f47731ca5c0081b7acc14
Author: Lukas Wunner <[email protected]>
Subject: PCI/CMA: Validate Subject Alternative Name in certificates

This commit introduces functionality to validate the Subject Alternative Name (SAN) in PCIe Leaf Certificates. It parses the SAN using an ASN.1 decoder and verifies that the identity encoded in the certificate matches the device's identity in Config Space.

Quoted 4 lines

drivers/pci/Makefile +0 -0

index 16abfd0e17e19..15512512fce7d 100644

[ ... ]

drivers/pci/Makefile obj-$(CONFIG_PCI_NPEM) += npem.o · line 46

Quoted 4 lines

drivers/pci/Makefile +3 -1

-obj-$(CONFIG_PCI_CMA)                += cma.o
+obj-$(CONFIG_PCI_CMA)                += cma.o cma.asn1.o
+$(obj)/cma.o:                        $(obj)/cma.asn1.h
+$(obj)/cma.asn1.o:           $(obj)/cma.asn1.c $(obj)/cma.asn1.h

If CONFIG_PCI_CMA is configured as a loadable module, will Kbuild compile two separate modules (cma.ko and cma.asn1.ko)?

Because the ASN.1 compiler does not export the cma_decoder symbol, would the cma.ko module fail to link during modpost with an undefined symbol error?

Should these objects be bundled into a composite object instead? For example:

cma_module-y := cma.o cma.asn1.o
obj-$(CONFIG_PCI_CMA) += cma_module.o

Quoted 5 lines

drivers/pci/cma.c +0 -0

index 998fde6366fb8..ee186f3619409 100644

[ ... ]

drivers/pci/cma.c line 76

Quoted 12 lines

drivers/pci/cma.c +12 -0

+int pci_cma_note_san(void *context, size_t hdrlen, unsigned char tag,
+                  const void *value, size_t vlen)
+{
+     struct pci_cma_x509_context *ctx = context;
+
+     /* These aren't the drOIDs we're looking for. */
+     if (ctx->last_oid != OID_CMA)
+             return 0;
+
+     if (tag != ASN1_UTF8STR ||
+         vlen != ctx->expected_len ||
+         memcmp(value, ctx->expected_name, vlen) != 0) {

Does this perform a strict case-sensitive comparison of the Subject Alternative Name?

If a device vendor provisions the certificate using uppercase hexadecimal letters (a common standard for hardware IDs), could this strict memcmp() erroneously reject a valid device?

Quoted 15 lines

drivers/pci/cma.c +14 -0

+             pci_err(ctx->pdev, "Leaf certificate of slot %u "
+                     "has invalid Subject Alternative Name\n", ctx->slot);
+             return -EINVAL;
+     }
+
+     ctx->found = true;
+
+     return 0;
+}
+
+static unsigned int pci_cma_construct_san(struct pci_dev *pdev, char *name)
+{
+     unsigned int len;
+     u64 serial;

drivers/pci/cma.c line 94

Quoted 4 lines

drivers/pci/cma.c +4 -0

+
+     len = snprintf(name, CMA_NAME_MAX,
+                    "Vendor=%04hx:Device=%04hx:CC=%06x:REV=%02hhx",
+                    pdev->vendor, pdev->device, pdev->class, pdev->revision);

The commit message mentions handling devices that mutate their identity at runtime, but this string is constructed using cached software state from probe time (like pdev->vendor).

Since pci_get_dsn() reads the live hardware state, could a mutated device result in an inconsistent string containing a stale vendor/device ID but a new DSN? Will reauthentication fail without a full PCI rescan?

Additionally, snprintf() returns the number of characters that would have been written, not the actual bytes written.

If the inputs ever exceed the 79-byte CMA_NAME_MAX buffer (for instance, if a quirk mutates pdev->class to exceed 6 hex characters), the string will be truncated but len will reflect the untruncated size.

Could this cause the later memcmp() check in pci_cma_note_san() to read out of bounds of the ctx->expected_name stack buffer? Should scnprintf() be used here instead to track the actual length written?

drivers/pci/cma.c line 99

Quoted 6 lines

drivers/pci/cma.c +5 -0

+
+     if (pdev->hdr_type == PCI_HEADER_TYPE_NORMAL)
+             len += snprintf(name + len, CMA_NAME_MAX - len,
+                             ":SSVID=%04hx:SSID=%04hx",
+                             pdev->subsystem_vendor, pdev->subsystem_device);

PCIe Type 1 devices (bridges) can also implement the Subsystem Vendor ID capability.

If a Type 1 bridge implements this capability and includes these fields in its certificate as required by the specification, will this condition unconditionally omit them and cause validation to fail?

drivers/pci/cma.c line 107

Quoted 8 lines

drivers/pci/cma.c +8 -0

+
+     serial = pci_get_dsn(pdev);
+     if (serial)
+             len += snprintf(name + len, CMA_NAME_MAX - len,
+                             ":%016llx", serial);
+
+     return len;
+}

Looking at pci_get_dsn(), it returns 0 both when the DSN capability is missing and when the device's legitimate serial number is 0.

If a device implements the capability with a valid DSN of 0, will this check evaluate to false, omit the field, and reject a valid device?

Would it be safer to explicitly check for the capability's presence via pci_find_ext_capability() instead?

Also, pci_get_dsn() performs two separate 32-bit reads without a lock:

pci_read_config_dword(dev, pos, &dword);
dsn = (u64)dword;
pci_read_config_dword(dev, pos + 4, &dword);
dsn |= ((u64)dword) << 32;

Could a concurrent mutation of the device identity result in a torn 64-bit read during this check?

--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10

Quoted 13 lines · On Fri, May 8, 2026 at 1:58 PM <[email protected]> wrote:

drivers/pci/Makefile obj-$(CONFIG_PCI_NPEM) += npem.o · line 46

Quoted 5 lines

+++ b/drivers/pci/Makefile

drivers/pci/Makefile +3 -1

-obj-$(CONFIG_PCI_CMA)                += cma.o
+obj-$(CONFIG_PCI_CMA)                += cma.o cma.asn1.o
+$(obj)/cma.o:                        $(obj)/cma.asn1.h
+$(obj)/cma.asn1.o:           $(obj)/cma.asn1.c $(obj)/cma.asn1.h

If CONFIG_PCI_CMA is configured as a loadable module, will Kbuild compile two separate modules (cma.ko and cma.asn1.ko)?

Because the ASN.1 compiler does not export the cma_decoder symbol, would the cma.ko module fail to link during modpost with an undefined symbol error?

Quoted 5 lines

Should these objects be bundled into a composite object instead? For example:

cma_module-y := cma.o cma.asn1.o
obj-$(CONFIG_PCI_CMA) += cma_module.o

This seems correct

Quoted 4 lines

Quoted 3 lines

drivers/pci/cma.c +3 -0

+     if (tag != ASN1_UTF8STR ||
+         vlen != ctx->expected_len ||
+         memcmp(value, ctx->expected_name, vlen) != 0) {

Quoted 6 lines

Does this perform a strict case-sensitive comparison of the Subject Alternative Name?

If a device vendor provisions the certificate using uppercase hexadecimal letters (a common standard for hardware IDs), could this strict memcmp() erroneously reject a valid device?

This is incorrect though, as the case is specified in the PCI spec

Quoted 7 lines

drivers/pci/cma.c line 94

Quoted 3 lines

drivers/pci/cma.c +3 -0

+     len = snprintf(name, CMA_NAME_MAX,
+                    "Vendor=%04hx:Device=%04hx:CC=%06x:REV=%02hhx",
+                    pdev->vendor, pdev->device, pdev->class, pdev->revision);

The commit message mentions handling devices that mutate their identity at runtime, but this string is constructed using cached software state from probe time (like pdev->vendor).

This I'm not clear on. Do we need to call pci_bus_read_dev_vendor_id() to get updated values?

Quoted 3 lines

Since pci_get_dsn() reads the live hardware state, could a mutated device result in an inconsistent string containing a stale vendor/device ID but a new DSN? Will reauthentication fail without a full PCI rescan?

Quoted 2 lines

Additionally, snprintf() returns the number of characters that would have been written, not the actual bytes written.

This is true

Quoted 4 lines

If the inputs ever exceed the 79-byte CMA_NAME_MAX buffer (for instance, if a quirk mutates pdev->class to exceed 6 hex characters), the string will be truncated but len will reflect the untruncated size.

That's true, but I don't see how the fixed length format string could exceed the size.

Quoted 3 lines

Could this cause the later memcmp() check in pci_cma_note_san() to read out of bounds of the ctx->expected_name stack buffer? Should scnprintf() be used here instead to track the actual length written?

scnprintf() should be used instead, just in case.

Quoted 7 lines

drivers/pci/cma.c line 99

Quoted 4 lines

drivers/pci/cma.c +4 -0

+     if (pdev->hdr_type == PCI_HEADER_TYPE_NORMAL)
+             len += snprintf(name + len, CMA_NAME_MAX - len,
+                             ":SSVID=%04hx:SSID=%04hx",
+                             pdev->subsystem_vendor, pdev->subsystem_device);

PCIe Type 1 devices (bridges) can also implement the Subsystem Vendor ID capability.

Quoted 11 lines

drivers/pci/cma.c line 107

Quoted 7 lines

drivers/pci/cma.c +7 -0

+     serial = pci_get_dsn(pdev);
+     if (serial)
+             len += snprintf(name + len, CMA_NAME_MAX - len,
+                             ":%016llx", serial);
+
+     return len;
+}

Looking at pci_get_dsn(), it returns 0 both when the DSN capability is missing and when the device's legitimate serial number is 0.

Quoted 5 lines

If a device implements the capability with a valid DSN of 0, will this check evaluate to false, omit the field, and reject a valid device?

Would it be safer to explicitly check for the capability's presence via pci_find_ext_capability() instead?

I see your point, but I don't think so.

Quoted 9 lines

Also, pci_get_dsn() performs two separate 32-bit reads without a lock:

pci_read_config_dword(dev, pos, &dword);
dsn = (u64)dword;
pci_read_config_dword(dev, pos + 4, &dword);
dsn |= ((u64)dword) << 32;

Could a concurrent mutation of the device identity result in a torn 64-bit read during this check?

Really?