Re: [PATCH v3 1/2] rust: add projection infrastructure

["Benno Lossin" <lossin@kernel.org>]

On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 3da92f18f4ee..50866b481bdb 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -20,6 +20,7 @@
>  #![feature(generic_nonzero)]
>  #![feature(inline_const)]
>  #![feature(pointer_is_aligned)]
> +#![feature(slice_ptr_len)]

This is missing a stability comment (stable since 1.79).

>  //
>  // Stable since Rust 1.80.0.
>  #![feature(slice_flatten)]
...
> +/// A helper trait to perform index projection.
> +///
> +/// This is similar to `core::slice::SliceIndex`, but operate on raw pointers safely and fallibly.
> +///
> +/// # Safety
> +///
> +/// `get` must return a pointer in bounds of the provided pointer.

This only makes sense when the provided pointer already points at an
allocation. But since the functions of this trait aren't `unsafe`, it
must be sound to pass `ptr::null` to them.

I first thought that we might be able to just use `mem::size_of_val_raw`
[1] to give an upper and lower bound on the address of the returned
pointer, but that is unsafe and cannot be called with an arbitrary
pointer. Interestingly, `ptr::metadata` [2] can be called safely & with
any pointer; I would expect them to be very similar (except of course
for extern types).

[1]: https://doc.rust-lang.org/std/mem/fn.size_of_val_raw.html
[2]: https://doc.rust-lang.org/std/ptr/fn.metadata.html

A pretty expensive solution would be to add a sealed trait `Indexable`
that we implement for all things that `T` is allowed to be; and then we
provide a safe function in that trait to query the maximum offset the
`get` function is allowed to make.

Alternatively, we could use something like this:

    The implementation of `get` must:
    - return a pointer obtained by offsetting the input pointer.
    - ensure that when the input pointer points at a valid value of type
      `T`, the offset must not be greater than [`mem::size_of_val_raw`]
      of the input pointer.

Or something simpler that says "if the input pointer is valid, then
`get` must return a valid output pointer"?

> +#[diagnostic::on_unimplemented(message = "`{Self}` cannot be used to index `{T}`")]
> +#[doc(hidden)]
> +pub unsafe trait ProjectIndex<T: ?Sized>: Sized {
> +    type Output: ?Sized;
> +
> +    /// Returns an index-projected pointer, if in bounds.
> +    fn get(self, slice: *mut T) -> Option<*mut Self::Output>;

How about we name this `try_index` instead of the general `get`?

> +
> +    /// Returns an index-projected pointer; fail the build if it cannot be proved to be in bounds.
> +    #[inline(always)]
> +    fn index(self, slice: *mut T) -> *mut Self::Output {
> +        Self::get(self, slice).unwrap_or_else(|| build_error!())
> +    }
> +}
...
> +/// A helper trait to perform field projection.
> +///
> +/// This trait has a `DEREF` generic parameter so it can be implemented twice for types that
> +/// implement `Deref`. This will cause an ambiguity error and thus block `Deref` types being used
> +/// as base of projection, as they can inject unsoundness.

I think it's important to also say that the ambiguity error only happens
when calling the function without specifying the `DEREF` constant.
Essentially it is a load-bearing part of the macro that it does this.

> +///
> +/// # Safety
> +///
> +/// `proj` should invoke `f` with valid allocation, as documentation described.

s/should invoke `f` with/may invoke `f` only with a/

"should" sounds like only a suggestion. If it is a requirement, then the
`build_error!` impl of the `DEREF = true` case would be violating it.

> +#[doc(hidden)]
> +pub unsafe trait ProjectField<const DEREF: bool> {
> +    /// Project a pointer to a type to a pointer of a field.
> +    ///
> +    /// `f` is always invoked with valid allocation so it can safely obtain raw pointers to fields
> +    /// using `&raw mut`.
> +    ///
> +    /// This is needed because `base` might not point to a valid allocation, while `&raw mut`
> +    /// requires pointers to be in bounds of a valid allocation.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `f` must returns a pointer in bounds of the provided pointer.

Typo: "must returns" -> "must return"

Cheers,
Benno

> +    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F;
> +}