From: Gary Guo <gary@garyguo.net>
To: Benno Lossin <lossin@kernel.org>
Cc: "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>,
"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:36:42 +0000 [thread overview]
Message-ID: <60bb58cecdef3ec4cda77cfad5c620ed@garyguo.net> (raw)
In-Reply-To: <DGELDM5523KS.3EY7C7X5PC1V4@kernel.org>
On 2026-02-14 09:53, Benno Lossin wrote:
> 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.
I've verified codegen and haven't managed to get this to actually generate `T` on the stack.
LLVM always figures out that the offset is the only thing that matters and optimize away
everything. `memoffset` crate also creates a temporary `MaybeUninit`, and given that it was
very widely used before `offset_of!` is stable, I think we should be able to rely on this being
okay even for large types.
Note that I've taken care to mark everything `#[inline(always)]` when possible, even
closures passed to `proj`.
> 3. The `wrapping_byte_offset` function generates potentially worse
> codegen when `base` points into a real allocation.
I'm highly skeptical that we'll lose any optimization, but this is indeed
a possibility in theory.
>
>> +}
>> +
>> +// 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);
I think `<*const _>::cast_mut($ptr)` probably would also do.
Thanks a lot for the review.
Best,
Gary
>
> 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()
>> + }};
>> +}
next prev parent reply other threads:[~2026-02-14 10:36 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-14 5:33 [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA Gary Guo
2026-02-14 5:33 ` [PATCH 1/4] rust: add projection infrastructure Gary Guo
2026-02-14 9:53 ` Benno Lossin
2026-02-14 10:36 ` Gary Guo [this message]
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
2026-02-14 10:33 ` [PATCH 0/4] rust: add pointer projection infrastructure and convert DMA Danilo Krummrich
2026-02-14 10:50 ` Gary Guo
2026-02-14 11:00 ` Danilo Krummrich
2026-02-15 0:47 ` Miguel Ojeda
2026-02-15 11:06 ` Danilo Krummrich
2026-02-15 12:06 ` Miguel Ojeda
2026-02-15 12:56 ` Danilo Krummrich
2026-02-15 15:16 ` Miguel Ojeda
2026-02-15 17:02 ` Danilo Krummrich
2026-02-18 10:49 ` Miguel Ojeda
2026-02-18 18:25 ` Danilo Krummrich
2026-02-15 14:39 ` 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=60bb58cecdef3ec4cda77cfad5c620ed@garyguo.net \
--to=gary@garyguo.net \
--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=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lossin@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