public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Benno Lossin" <lossin@kernel.org>
To: "Gary Guo" <gary@garyguo.net>, "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nicolas Schier" <nsc@kernel.org>
Cc: "Alexandre Courbot" <acourbot@nvidia.com>,
	<rust-for-linux@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-kbuild@vger.kernel.org>
Subject: Re: [PATCH 1/4] rust: add projection infrastructure
Date: Sat, 14 Feb 2026 10:53:54 +0100	[thread overview]
Message-ID: <DGELDM5523KS.3EY7C7X5PC1V4@kernel.org> (raw)
In-Reply-To: <20260214053344.1994776-2-gary@garyguo.net>

On Sat Feb 14, 2026 at 6:33 AM CET, Gary Guo wrote:
> Add a generic infrastructure for performing field and index projections on
> raw pointers. This will form the basis of performing I/O projections.
>
> Pointers manipulations are intentionally using the safe wrapping variants
> instead of the unsafe variants, as the latter requires pointers to be
> inside an allocation which is not necessarily true for I/O pointers.
>
> This projection macro protects against rogue `Deref` implementation, which
> can causes the projected pointer to be outside the bounds of starting
> pointer. This is extremely unlikely and Rust has a lint to catch this, but
> is unsoundness regardless. The protection works by inducing type inference
> ambiguity when `Deref` is implemented.
>
> The projection macro supports both fallible and infallible index
> projections. These are described in detail inside the documentation.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Cool work!

I was wondering how you'd make this safe and general, but just having a
primitive pointer projection macro makes a lot of sense. We'll have lots
of projection macros that use this under the hood instead of a single
one. I like this as a stop-gap solution until we have projections in the
language.

I have a few comments, with those addressed:

Reviewed-by: Benno Lossin <lossin@kernel.org>

> ---
>  rust/kernel/lib.rs        |   5 +
>  rust/kernel/projection.rs | 269 ++++++++++++++++++++++++++++++++++++++
>  scripts/Makefile.build    |   4 +-
>  3 files changed, 277 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/projection.rs

> +// SAFETY: `proj` invokes `f` with valid allocation.
> +unsafe impl<T> ProjectField<false> for T {
> +    #[inline(always)]
> +    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
> +        // Create a valid allocation to start projection, as `base` is not necessarily so.
> +        let mut place = MaybeUninit::uninit();
> +        let place_base = place.as_mut_ptr();
> +        let field = f(place_base);
> +        // SAFETY: `field` is in bounds from `base` per safety requirement.
> +        let offset = unsafe { field.byte_offset_from(place_base) };
> +        base.wrapping_byte_offset(offset).cast()
> +    }

There are several limitations with this impl. I don't think we can do
anything about them, but it's probably good to list them somewhere:
1. We do not support projecting fields of unsized types, so `MyStruct<dyn Trait>`.
   (note that slices are supported with `ProjectIndex`)
2. Since this creates a `MaybeUninit<T>` on the stack, only small `T`
   are supported. I'm not sure how much of this will be optimized away,
   but it might be the case that it is not. Projecting in the same
   function call stack multiple times might result in overrunning the
   stack pretty quickly.
3. The `wrapping_byte_offset` function generates potentially worse
   codegen when `base` points into a real allocation.

> +}
> +
> +// SAFETY: vacuously satisfied.
> +unsafe impl<T: Deref> ProjectField<true> for T {
> +    #[inline(always)]
> +    unsafe fn proj<F>(_: *mut Self, _: impl FnOnce(*mut Self) -> *mut F) -> *mut F {
> +        build_error!("this function is a guard against `Deref` impl and is never invoked");
> +    }
> +}
> +
> +/// Create a projection from a raw pointer.
> +///

I'd add a paragraph that explains that the pointer does not need to be
valid in any way. It should also explain that the returned pointer is
only valid when the original pointer was valid.

> +/// Supported projections include field projections and index projections.
> +/// It is not allowed to project into types that implement custom `Deref` or `Index`.
> +///
> +/// The macro has basic syntax of `kernel::project_pointer!(ptr, projection)`, where `ptr` is an
> +/// expression that evaluates to a raw pointer which serves as the base of projection. `projection`
> +/// can be a projection expression of form `.field` (normally identifer, or numeral in case of
> +/// tuple structs) or of form `[index]`.
> +///
> +/// If mutable pointer is needed, the macro input can be prefixed with `mut` keyword, i.e.
> +/// `kernel::project_pointer!(mut ptr, projection)`. By default, a const pointer is created.
> +///
> +/// `project_pointer!` macro can perform both fallible indexing and build-time checked indexing.
> +/// `[index]` form performs build-time bounds checking; if compiler fails to prove `[index]` is in
> +/// bounds, compilation will fail. `[index]?` can be used to perform runtime bounds checking;
> +/// `OutOfBound` error is raised via `?` if the index is out of bounds.
> +///
> +/// # Examples
> +///
> +/// Field projections are performed with `.field_name`:
> +/// ```
> +/// struct MyStruct { field: u32, }
> +/// let ptr: *const MyStruct = core::ptr::dangling();

I would only include one example that uses `dangling` and for the rest
just define a function that projects a raw pointer.

> +/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .field);
> +///
> +/// struct MyTupleStruct(u32, u32);
> +/// let ptr: *const MyTupleStruct = core::ptr::dangling();
> +/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .1);
> +/// ```
> +///
> +/// Index projections are performed with `[index]`:
> +/// ```
> +/// let ptr: *const [u8; 32] = core::ptr::dangling();
> +/// let field_ptr: *const u8 = kernel::project_pointer!(ptr, [1]);
> +/// // This will fail the build.
> +/// // kernel::project_pointer!(ptr, [128]);
> +/// // This will raise an `OutOfBound` error (which is convertable to `ERANGE`).
> +/// // kernel::project_pointer!(ptr, [128]?);
> +/// ```
> +///
> +/// If you need to match on the error instead of propagate, put the invocation inside a closure:
> +/// ```
> +/// let ptr: *const [u8; 32] = core::ptr::dangling();
> +/// let field_ptr: Result<*const u8> = (|| -> Result<_> {
> +///     Ok(kernel::project_pointer!(ptr, [128]?))
> +/// })();
> +/// assert!(field_ptr.is_err());
> +/// ```
> +///
> +/// For mutable pointers, put `mut` as the first token in macro invocation.
> +/// ```
> +/// let ptr: *mut [(u8, u16); 32] = core::ptr::dangling_mut();
> +/// let field_ptr: *mut u16 = kernel::project_pointer!(mut ptr, [1].1);
> +/// ```
> +#[macro_export]
> +macro_rules! project_pointer {
> +    (@gen $ptr:ident, ) => {};
> +    // Field projection. `$field` needs to be `tt` to support tuple index like `.0`.
> +    (@gen $ptr:ident, .$field:tt $($rest:tt)*) => {
> +        // SAFETY: the provided closure always return in bounds pointer.
> +        let $ptr = unsafe {
> +            $crate::projection::ProjectField::proj($ptr, #[inline(always)] |ptr| {
> +                // SAFETY: `$field` is in bounds, and no implicit `Deref` is possible (if the
> +                // type implements `Deref`, Rust cannot infer the generic parameter `DEREF`).
> +                &raw mut (*ptr).$field
> +            })
> +        };
> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
> +    };
> +    // Fallible index projection.
> +    (@gen $ptr:ident, [$index:expr]? $($rest:tt)*) => {
> +        let $ptr = $crate::projection::ProjectIndex::get($index, $ptr)
> +            .ok_or($crate::projection::OutOfBound)?;
> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
> +    };
> +    // Build-time checked index projection.
> +    (@gen $ptr:ident, [$index:expr] $($rest:tt)*) => {
> +        let $ptr = $crate::projection::ProjectIndex::index($index, $ptr);
> +        $crate::project_pointer!(@gen $ptr, $($rest)*)
> +    };
> +    (mut $ptr:expr, $($proj:tt)*) => {{
> +        let ptr = $ptr;

I'd add a type ascription `let ptr: *mut _ = $ptr;`

> +        $crate::project_pointer!(@gen ptr, $($proj)*);
> +        ptr
> +    }};
> +    ($ptr:expr, $($proj:tt)*) => {{
> +        let ptr = $ptr.cast_mut();

This allows `$ptr` to be a random type with a `cast_mut` function. How
about:

    let ptr: *const _ = $ptr;
    let ptr: *mut _ = ::core::ptr::cast_mut(ptr);

Cheers,
Benno

> +        // We currently always project using mutable pointer, as it is not decided whether `&raw
> +        // const` allows the resulting pointer to be mutated (see documentation of `addr_of!`).
> +        $crate::project_pointer!(@gen ptr, $($proj)*);
> +        ptr.cast_const()
> +    }};
> +}

  reply	other threads:[~2026-02-14  9:53 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260214053344.1994776-1-gary@garyguo.net>
2026-02-14  5:33 ` [PATCH 1/4] rust: add projection infrastructure Gary Guo
2026-02-14  9:53   ` Benno Lossin [this message]
2026-02-14 10:36     ` Gary Guo
2026-02-14 14:48       ` Benno Lossin
2026-02-14 10:27   ` Danilo Krummrich
2026-02-22  0:57   ` Benno Lossin
2026-02-22 10:52     ` Gary Guo
2026-02-14  5:33 ` [PATCH 2/4] rust: dma: generalize `dma_{read,write}` macro Gary Guo
2026-02-14 10:04   ` Benno Lossin
2026-02-14 10:46     ` Gary Guo
2026-02-14 14:53       ` Benno Lossin
2026-02-14  5:33 ` [PATCH 3/4] gpu: nova-core: convert to use new `dma_write!` syntax Gary Guo
2026-02-14 10:06   ` Benno Lossin
2026-02-14  5:33 ` [PATCH 4/4] rust: dma: remove old dma_{read,write} macro compatibility syntax Gary Guo
2026-02-14 10:05   ` Benno Lossin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DGELDM5523KS.3EY7C7X5PC1V4@kernel.org \
    --to=lossin@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=nsc@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox