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

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

[Gary Guo]

From: Gary Guo <gary@garyguo.net>

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.

This projection macro also stops projecting into unaligned fields (i.e.
fields of `#[repr(packed)]` structs), as misaligned pointers require
special handling. This is implemented by attempting to create reference to
projected field inside a `if false` block. Despite being unreachable, Rust
still checks that they're not unaligned fields.

The projection macro supports both fallible and infallible index
projections. These are described in detail inside the documentation.

Reviewed-by: Aditya Rajan <adi.dev.github@gmail.com>
Signed-off-by: Gary Guo <gary@garyguo.net>
---
 rust/kernel/lib.rs        |   5 +
 rust/kernel/projection.rs | 284 ++++++++++++++++++++++++++++++++++++++
 scripts/Makefile.build    |   4 +-
 3 files changed, 292 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/projection.rs

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)]
 //
 // Stable since Rust 1.80.0.
 #![feature(slice_flatten)]
@@ -37,6 +38,9 @@
 #![feature(const_ptr_write)]
 #![feature(const_refs_to_cell)]
 //
+// Stable since Rust 1.84.0.
+#![feature(strict_provenance)]
+//
 // Expected to become stable.
 #![feature(arbitrary_self_types)]
 //
@@ -130,6 +134,7 @@
 pub mod prelude;
 pub mod print;
 pub mod processor;
+pub mod projection;
 pub mod ptr;
 #[cfg(CONFIG_RUST_PWM_ABSTRACTIONS)]
 pub mod pwm;
diff --git a/rust/kernel/projection.rs b/rust/kernel/projection.rs
new file mode 100644
index 000000000000..b9e2b174615c
--- /dev/null
+++ b/rust/kernel/projection.rs
@@ -0,0 +1,284 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Infrastructure for handling projections.
+
+use core::{
+    mem::MaybeUninit,
+    ops::Deref, //
+};
+
+use crate::prelude::*;
+
+/// Error raised when a projection is attempted on array or slices out of bounds.
+pub struct OutOfBound;
+
+impl From<OutOfBound> for Error {
+    #[inline(always)]
+    fn from(_: OutOfBound) -> Self {
+        ERANGE
+    }
+}
+
+/// 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.
+#[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>;
+
+    /// 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!())
+    }
+}
+
+// Forward array impl to slice impl.
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T, I, const N: usize> ProjectIndex<[T; N]> for I
+where
+    I: ProjectIndex<[T]>,
+{
+    type Output = <I as ProjectIndex<[T]>>::Output;
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T; N]) -> Option<*mut Self::Output> {
+        <I as ProjectIndex<[T]>>::get(self, slice)
+    }
+
+    #[inline(always)]
+    fn index(self, slice: *mut [T; N]) -> *mut Self::Output {
+        <I as ProjectIndex<[T]>>::index(self, slice)
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for usize {
+    type Output = T;
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut T> {
+        if self >= slice.len() {
+            None
+        } else {
+            Some(slice.cast::<T>().wrapping_add(self))
+        }
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for core::ops::Range<usize> {
+    type Output = [T];
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut [T]> {
+        let new_len = self.end.checked_sub(self.start)?;
+        if self.end > slice.len() {
+            return None;
+        }
+        Some(core::ptr::slice_from_raw_parts_mut(
+            slice.cast::<T>().wrapping_add(self.start),
+            new_len,
+        ))
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for core::ops::RangeTo<usize> {
+    type Output = [T];
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut [T]> {
+        (0..self.end).get(slice)
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for core::ops::RangeFrom<usize> {
+    type Output = [T];
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut [T]> {
+        (self.start..slice.len()).get(slice)
+    }
+}
+
+// SAFETY: `get` returned pointers are in bounds.
+unsafe impl<T> ProjectIndex<[T]> for core::ops::RangeFull {
+    type Output = [T];
+
+    #[inline(always)]
+    fn get(self, slice: *mut [T]) -> Option<*mut [T]> {
+        Some(slice)
+    }
+}
+
+/// 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.
+///
+/// # Safety
+///
+/// `proj` should invoke `f` with valid allocation, as documentation described.
+#[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.
+    unsafe fn proj<F>(base: *mut Self, f: impl FnOnce(*mut Self) -> *mut F) -> *mut F;
+}
+
+// NOTE: in theory, this API should work for `T: ?Sized` and `F: ?Sized`, too. However we cannot
+// currently support that as we need to obtain a valid allocation that `&raw const` can operate on.
+// 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. The
+        // memory is never actually used so it will be optimized out, so it should work even for
+        // very large `T` (`memoffset` crate also relies on this). To be extra certain, we also
+        // annotate `f` closure with `#[inline(always)]` in the macro.
+        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) };
+        // Use `wrapping_byte_offset` as `base` does not need to be of valid allocation.
+        base.wrapping_byte_offset(offset).cast()
+    }
+}
+
+// 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.
+///
+/// The projected pointer is within the memory region marked by the input pointer. There is no
+/// requirement that the input raw pointer needs to be valid, so this macro may be used for
+/// projecting pointers outside normal address space, e.g. I/O pointers. However, if the input
+/// pointer is valid, the projected pointer is also 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();
+/// let field_ptr: *const u32 = kernel::project_pointer!(ptr, .field);
+///
+/// struct MyTupleStruct(u32, u32);
+///
+/// fn proj(ptr: *const MyTupleStruct) {
+///     let field_ptr: *const u32 = kernel::project_pointer!(ptr, .1);
+/// }
+/// ```
+///
+/// Index projections are performed with `[index]`:
+/// ```
+/// fn proj(ptr: *const [u8; 32]) -> Result {
+///     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]?);
+///     Ok(())
+/// }
+/// ```
+///
+/// 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| {
+                // Check unaligned field. Not all users (e.g. DMA) can handle unaligned
+                // projections.
+                if false {
+                    let _ = &(*ptr).$field;
+                }
+                // 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: *mut _ = $ptr;
+        $crate::project_pointer!(@gen ptr, $($proj)*);
+        ptr
+    }};
+    ($ptr:expr, $($proj:tt)*) => {{
+        let ptr = <*const _>::cast_mut($ptr);
+        // 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()
+    }};
+}
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 32e209bc7985..3652b85be545 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -310,16 +310,18 @@ $(obj)/%.lst: $(obj)/%.c FORCE
 
 # The features in this list are the ones allowed for non-`rust/` code.
 #
+#   - Stable since Rust 1.79.0: `feature(slice_ptr_len)`.
 #   - Stable since Rust 1.81.0: `feature(lint_reasons)`.
 #   - Stable since Rust 1.82.0: `feature(asm_const)`,
 #     `feature(offset_of_nested)`, `feature(raw_ref_op)`.
+#   - Stable since Rust 1.84.0: `feature(strict_provenance)`.
 #   - Stable since Rust 1.87.0: `feature(asm_goto)`.
 #   - Expected to become stable: `feature(arbitrary_self_types)`.
 #   - To be determined: `feature(used_with_arg)`.
 #
 # Please see https://github.com/Rust-for-Linux/linux/issues/2 for details on
 # the unstable features in use.
-rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,offset_of_nested,raw_ref_op,used_with_arg
+rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,offset_of_nested,raw_ref_op,slice_ptr_len,strict_provenance,used_with_arg
 
 # `--out-dir` is required to avoid temporaries being created by `rustc` in the
 # current working directory, which may be not accessible in the out-of-tree
-- 
2.51.2

[PATCH v3 2/2] rust: dma: use pointer projection infra for `dma_{read,write}` macro

[Gary Guo]

From: Gary Guo <gary@garyguo.net>

Current `dma_read!`, `dma_write!` macros also use a custom
`addr_of!()`-based implementation for projecting pointers, which has
soundness issue as it relies on absence of `Deref` implementation on types.
It also has a soundness issue where it does not protect against unaligned
fields (when `#[repr(packed)]` is used) so it can generate misaligned
accesses.

This commit migrates them to use the general pointer projection
infrastructure, which handles these cases correctly.

As part of migration, the macro is updated to have an improved surface
syntax. The current macro have

    dma_read!(a.b.c[d].e.f)

to mean `a.b.c` is a DMA coherent allocation and it should project into it
with `[d].e.f` and do a read, which is confusing as it makes the indexing
operator integral to the macro (so it will break if you have an array of
`CoherentAllocation`, for example).

This also is problematic as we would like to generalize
`CoherentAllocation` from just slices to arbitrary types.

Make the macro expects `dma_read!(path.to.dma, .path.inside.dma)` as the
canonical syntax. The index operator is no longer special and is just one
type of projection (in additional to field projection). Similarly, make
`dma_write!(path.to.dma, .path.inside.dma, value)` become the canonical
syntax for writing.

Another issue of the current macro is that it is always fallible. This
makes sense with existing design of `CoherentAllocation`, but once we
support fixed size arrays with `CoherentAllocation`, it is desirable to
have the ability to perform infallible indexing as well, e.g. doing a `[0]`
index of `[Foo; 2]` is okay and can be checked at build-time, so forcing
falliblity is non-ideal. To capture this, the macro is changed to use
`[idx]` as infallible projection and `[idx]?` as fallible index projection
(those syntax are part of the general projection infra). A benefit of this
is that while individual indexing operation may fail, the overall
read/write operation is not fallible.

Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/gpu/nova-core/gsp.rs      |  14 ++--
 drivers/gpu/nova-core/gsp/boot.rs |   2 +-
 drivers/gpu/nova-core/gsp/cmdq.rs |  10 ++-
 rust/kernel/dma.rs                | 114 +++++++++++++-----------------
 samples/rust/rust_dma.rs          |  30 ++++----
 5 files changed, 81 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 174feaca0a6b..25cd48514c77 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -143,14 +143,14 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
                     // _kgspInitLibosLoggingStructures (allocates memory for buffers)
                     // kgspSetupLibosInitArgs_IMPL (creates pLibosInitArgs[] array)
                     dma_write!(
-                        libos[0] = LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0)
-                    )?;
+                        libos, [0]?, LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0)
+                    );
                     dma_write!(
-                        libos[1] = LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0)
-                    )?;
-                    dma_write!(libos[2] = LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
-                    dma_write!(rmargs[0].inner = fw::GspArgumentsCached::new(cmdq))?;
-                    dma_write!(libos[3] = LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
+                        libos, [1]?, LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0)
+                    );
+                    dma_write!(libos, [2]?, LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0));
+                    dma_write!(rmargs, [0]?.inner, fw::GspArgumentsCached::new(cmdq));
+                    dma_write!(libos, [3]?, LibosMemoryRegionInitArgument::new("RMARGS", rmargs));
                 },
             }))
         })
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index c56029f444cb..7f46fa5e9b50 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -157,7 +157,7 @@ pub(crate) fn boot(
 
         let wpr_meta =
             CoherentAllocation::<GspFwWprMeta>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
-        dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
+        dma_write!(wpr_meta, [0]?, GspFwWprMeta::new(&gsp_fw, &fb_layout));
 
         self.cmdq
             .send_command(bar, commands::SetSystemInfo::new(pdev))?;
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 87dbbd6d1be9..0056bfbf0a44 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -202,9 +202,13 @@ fn new(dev: &device::Device<device::Bound>) -> Result<Self> {
 
         let gsp_mem =
             CoherentAllocation::<GspMem>::alloc_coherent(dev, 1, GFP_KERNEL | __GFP_ZERO)?;
-        dma_write!(gsp_mem[0].ptes = PteArray::new(gsp_mem.dma_handle())?)?;
-        dma_write!(gsp_mem[0].cpuq.tx = MsgqTxHeader::new(MSGQ_SIZE, RX_HDR_OFF, MSGQ_NUM_PAGES))?;
-        dma_write!(gsp_mem[0].cpuq.rx = MsgqRxHeader::new())?;
+        dma_write!(gsp_mem, [0]?.ptes, PteArray::new(gsp_mem.dma_handle())?);
+        dma_write!(
+            gsp_mem,
+            [0]?.cpuq.tx,
+            MsgqTxHeader::new(MSGQ_SIZE, RX_HDR_OFF, MSGQ_NUM_PAGES)
+        );
+        dma_write!(gsp_mem, [0]?.cpuq.rx, MsgqRxHeader::new());
 
         Ok(Self(gsp_mem))
     }
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 909d56fd5118..07fa4912ea78 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -461,6 +461,19 @@ pub fn size(&self) -> usize {
         self.count * core::mem::size_of::<T>()
     }
 
+    /// Returns the raw pointer to the allocated region in the CPU's virtual address space.
+    #[inline]
+    pub fn as_ptr(&self) -> *const [T] {
+        core::ptr::slice_from_raw_parts(self.cpu_addr.as_ptr(), self.count)
+    }
+
+    /// Returns the raw pointer to the allocated region in the CPU's virtual address space as
+    /// a mutable pointer.
+    #[inline]
+    pub fn as_mut_ptr(&self) -> *mut [T] {
+        core::ptr::slice_from_raw_parts_mut(self.cpu_addr.as_ptr(), self.count)
+    }
+
     /// Returns the base address to the allocated region in the CPU's virtual address space.
     pub fn start_ptr(&self) -> *const T {
         self.cpu_addr.as_ptr()
@@ -581,23 +594,6 @@ pub unsafe fn write(&mut self, src: &[T], offset: usize) -> Result {
         Ok(())
     }
 
-    /// Returns a pointer to an element from the region with bounds checking. `offset` is in
-    /// units of `T`, not the number of bytes.
-    ///
-    /// Public but hidden since it should only be used from [`dma_read`] and [`dma_write`] macros.
-    #[doc(hidden)]
-    pub fn item_from_index(&self, offset: usize) -> Result<*mut T> {
-        if offset >= self.count {
-            return Err(EINVAL);
-        }
-        // SAFETY:
-        // - The pointer is valid due to type invariant on `CoherentAllocation`
-        // and we've just checked that the range and index is within bounds.
-        // - `offset` can't overflow since it is smaller than `self.count` and we've checked
-        // that `self.count` won't overflow early in the constructor.
-        Ok(unsafe { self.cpu_addr.as_ptr().add(offset) })
-    }
-
     /// Reads the value of `field` and ensures that its type is [`FromBytes`].
     ///
     /// # Safety
@@ -670,6 +666,9 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
 
 /// Reads a field of an item from an allocated region of structs.
 ///
+/// The syntax is of form `kernel::dma_read!(dma, proj)` where `dma` is an expression to an
+/// [`CoherentAllocation`] and `proj` is a [projection specification](kernel::project_pointer!).
+///
 /// # Examples
 ///
 /// ```
@@ -684,36 +683,29 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
 /// unsafe impl kernel::transmute::AsBytes for MyStruct{};
 ///
 /// # fn test(alloc: &kernel::dma::CoherentAllocation<MyStruct>) -> Result {
-/// let whole = kernel::dma_read!(alloc[2]);
-/// let field = kernel::dma_read!(alloc[1].field);
+/// let whole = kernel::dma_read!(alloc, [2]?);
+/// let field = kernel::dma_read!(alloc, [1]?.field);
 /// # Ok::<(), Error>(()) }
 /// ```
 #[macro_export]
 macro_rules! dma_read {
-    ($dma:expr, $idx: expr, $($field:tt)*) => {{
-        (|| -> ::core::result::Result<_, $crate::error::Error> {
-            let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
-            // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
-            // dereferenced. The compiler also further validates the expression on whether `field`
-            // is a member of `item` when expanded by the macro.
-            unsafe {
-                let ptr_field = ::core::ptr::addr_of!((*item) $($field)*);
-                ::core::result::Result::Ok(
-                    $crate::dma::CoherentAllocation::field_read(&$dma, ptr_field)
-                )
-            }
-        })()
+    ($dma:expr, $($proj:tt)*) => {{
+        let dma = &$dma;
+        let ptr = $crate::project_pointer!(
+            $crate::dma::CoherentAllocation::as_ptr(dma), $($proj)*
+        );
+        // SAFETY: pointer created by projection is within DMA region.
+        unsafe { $crate::dma::CoherentAllocation::field_read(dma, ptr) }
     }};
-    ($dma:ident [ $idx:expr ] $($field:tt)* ) => {
-        $crate::dma_read!($dma, $idx, $($field)*)
-    };
-    ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {
-        $crate::dma_read!($($dma).*, $idx, $($field)*)
-    };
 }
 
 /// Writes to a field of an item from an allocated region of structs.
 ///
+/// The syntax is of form `kernel::dma_write!(dma, proj, val)` where `dma` is an expression to an
+/// [`CoherentAllocation`] and `proj` is a [projection specification](kernel::project_pointer!),
+/// and `val` is the value to be written to the projected location.
+///
+///
 /// # Examples
 ///
 /// ```
@@ -728,37 +720,31 @@ macro_rules! dma_read {
 /// unsafe impl kernel::transmute::AsBytes for MyStruct{};
 ///
 /// # fn test(alloc: &kernel::dma::CoherentAllocation<MyStruct>) -> Result {
-/// kernel::dma_write!(alloc[2].member = 0xf);
-/// kernel::dma_write!(alloc[1] = MyStruct { member: 0xf });
+/// kernel::dma_write!(alloc, [2]?.member, 0xf);
+/// kernel::dma_write!(alloc, [1]?, MyStruct { member: 0xf });
 /// # Ok::<(), Error>(()) }
 /// ```
 #[macro_export]
 macro_rules! dma_write {
-    ($dma:ident [ $idx:expr ] $($field:tt)*) => {{
-        $crate::dma_write!($dma, $idx, $($field)*)
-    }};
-    ($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {{
-        $crate::dma_write!($($dma).*, $idx, $($field)*)
+    (@parse [$dma:expr] [$($proj:tt)*] [, $val:expr]) => {{
+        let dma = &$dma;
+        let ptr = $crate::project_pointer!(
+            mut $crate::dma::CoherentAllocation::as_mut_ptr(dma), $($proj)*
+        );
+        let val = $val;
+        // SAFETY: pointer created by projection is within DMA region.
+        unsafe { $crate::dma::CoherentAllocation::field_write(dma, ptr, val) }
     }};
-    ($dma:expr, $idx: expr, = $val:expr) => {
-        (|| -> ::core::result::Result<_, $crate::error::Error> {
-            let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
-            // SAFETY: `item_from_index` ensures that `item` is always a valid item.
-            unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, item, $val) }
-            ::core::result::Result::Ok(())
-        })()
+    (@parse [$dma:expr] [$($proj:tt)*] [.$field:tt $($rest:tt)*]) => {
+        $crate::dma_write!(@parse [$dma] [$($proj)* .$field] [$($rest)*])
+    };
+    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr]? $($rest:tt)*]) => {
+        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]?] [$($rest)*])
+    };
+    (@parse [$dma:expr] [$($proj:tt)*] [[$index:expr] $($rest:tt)*]) => {
+        $crate::dma_write!(@parse [$dma] [$($proj)* [$index]] [$($rest)*])
     };
-    ($dma:expr, $idx: expr, $(.$field:ident)* = $val:expr) => {
-        (|| -> ::core::result::Result<_, $crate::error::Error> {
-            let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
-            // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
-            // dereferenced. The compiler also further validates the expression on whether `field`
-            // is a member of `item` when expanded by the macro.
-            unsafe {
-                let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*);
-                $crate::dma::CoherentAllocation::field_write(&$dma, ptr_field, $val)
-            }
-            ::core::result::Result::Ok(())
-        })()
+    ($dma:expr, $($rest:tt)*) => {
+        $crate::dma_write!(@parse [$dma] [] [$($rest)*])
     };
 }
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 9c45851c876e..ce39b5545097 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -68,7 +68,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
                 CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;
 
             for (i, value) in TEST_VALUES.into_iter().enumerate() {
-                kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;
+                kernel::dma_write!(ca, [i]?, MyStruct::new(value.0, value.1));
             }
 
             let size = 4 * page::PAGE_SIZE;
@@ -85,24 +85,26 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
     }
 }
 
+impl DmaSampleDriver {
+    fn check_dma(&self) -> Result {
+        for (i, value) in TEST_VALUES.into_iter().enumerate() {
+            let val0 = kernel::dma_read!(self.ca, [i]?.h);
+            let val1 = kernel::dma_read!(self.ca, [i]?.b);
+
+            assert_eq!(val0, value.0);
+            assert_eq!(val1, value.1);
+        }
+
+        Ok(())
+    }
+}
+
 #[pinned_drop]
 impl PinnedDrop for DmaSampleDriver {
     fn drop(self: Pin<&mut Self>) {
         dev_info!(self.pdev, "Unload DMA test driver.\n");
 
-        for (i, value) in TEST_VALUES.into_iter().enumerate() {
-            let val0 = kernel::dma_read!(self.ca[i].h);
-            let val1 = kernel::dma_read!(self.ca[i].b);
-            assert!(val0.is_ok());
-            assert!(val1.is_ok());
-
-            if let Ok(val0) = val0 {
-                assert_eq!(val0, value.0);
-            }
-            if let Ok(val1) = val1 {
-                assert_eq!(val1, value.1);
-            }
-        }
+        assert!(self.check_dma().is_ok());
 
         for (i, entry) in self.sgt.iter().enumerate() {
             dev_info!(
-- 
2.51.2

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

[Benno Lossin]

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

Re: [PATCH v3 2/2] rust: dma: use pointer projection infra for `dma_{read, write}` macro

[Benno Lossin]

On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
> From: Gary Guo <gary@garyguo.net>
>
> Current `dma_read!`, `dma_write!` macros also use a custom
> `addr_of!()`-based implementation for projecting pointers, which has
> soundness issue as it relies on absence of `Deref` implementation on types.
> It also has a soundness issue where it does not protect against unaligned
> fields (when `#[repr(packed)]` is used) so it can generate misaligned
> accesses.
>
> This commit migrates them to use the general pointer projection
> infrastructure, which handles these cases correctly.
>
> As part of migration, the macro is updated to have an improved surface
> syntax. The current macro have
>
>     dma_read!(a.b.c[d].e.f)
>
> to mean `a.b.c` is a DMA coherent allocation and it should project into it
> with `[d].e.f` and do a read, which is confusing as it makes the indexing
> operator integral to the macro (so it will break if you have an array of
> `CoherentAllocation`, for example).
>
> This also is problematic as we would like to generalize
> `CoherentAllocation` from just slices to arbitrary types.
>
> Make the macro expects `dma_read!(path.to.dma, .path.inside.dma)` as the
> canonical syntax. The index operator is no longer special and is just one
> type of projection (in additional to field projection). Similarly, make
> `dma_write!(path.to.dma, .path.inside.dma, value)` become the canonical
> syntax for writing.
>
> Another issue of the current macro is that it is always fallible. This
> makes sense with existing design of `CoherentAllocation`, but once we
> support fixed size arrays with `CoherentAllocation`, it is desirable to
> have the ability to perform infallible indexing as well, e.g. doing a `[0]`
> index of `[Foo; 2]` is okay and can be checked at build-time, so forcing
> falliblity is non-ideal. To capture this, the macro is changed to use
> `[idx]` as infallible projection and `[idx]?` as fallible index projection
> (those syntax are part of the general projection infra). A benefit of this
> is that while individual indexing operation may fail, the overall
> read/write operation is not fallible.
>
> Fixes: ad2907b4e308 ("rust: add dma coherent allocator abstraction")
> Signed-off-by: Gary Guo <gary@garyguo.net>

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

Cheers,
Benno

> ---
>  drivers/gpu/nova-core/gsp.rs      |  14 ++--
>  drivers/gpu/nova-core/gsp/boot.rs |   2 +-
>  drivers/gpu/nova-core/gsp/cmdq.rs |  10 ++-
>  rust/kernel/dma.rs                | 114 +++++++++++++-----------------
>  samples/rust/rust_dma.rs          |  30 ++++----
>  5 files changed, 81 insertions(+), 89 deletions(-)

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

[Danilo Krummrich]

On Mon Mar 2, 2026 at 3:38 PM CET, Benno Lossin wrote:
> [1]: https://doc.rust-lang.org/std/mem/fn.size_of_val_raw.html

I recently would have had a use-case for this as well, but besides the reasons
you can't use it in this patch, I think it's also still unstable?

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

[Gary Guo]

On Mon Mar 2, 2026 at 2:38 PM GMT, Benno Lossin wrote:
> 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.

The "in bounds" here is the conceptual bounds of the pointer. So, for a pointer
with size `x`, the address of the returned pointer lies between `ptr .. ptr +
x`.

>
> 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

I have a `KnownSize` trait for this in my I/O projection series that is
implemented for `T: Sized` and `[T]`, and it returns the size when given a raw
pointer.

>
> 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.

Given that I'm not introducing `KnownSize` trait in this patch, this is why I
haven't used this kind of wording. Perhaps I can just bring `KnownSize` in early
and use it first for documentation purpose only?

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

Hmm, wouldn't this give impression that "you can do whatever you want if the
input pointer is not valid"?

>
>> +#[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`?

I'm following the name on `SliceIndex`:
https://doc.rust-lang.org/stable/std/slice/trait.SliceIndex.html.

Best,
Gary

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

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

[Benno Lossin]

On Mon Mar 2, 2026 at 3:49 PM CET, Gary Guo wrote:
> On Mon Mar 2, 2026 at 2:38 PM GMT, Benno Lossin wrote:
>> On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
>>> +/// 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.
>
> The "in bounds" here is the conceptual bounds of the pointer. So, for a pointer
> with size `x`, the address of the returned pointer lies between `ptr .. ptr +
> x`.

Okay, I haven't really seen that as a concept. Also, what is the size of
an invalid pointer?

>> 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
>
> I have a `KnownSize` trait for this in my I/O projection series that is
> implemented for `T: Sized` and `[T]`, and it returns the size when given a raw
> pointer.
>
>>
>> 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.
>
> Given that I'm not introducing `KnownSize` trait in this patch, this is why I
> haven't used this kind of wording. Perhaps I can just bring `KnownSize` in early
> and use it first for documentation purpose only?

That sounds great.

>> Or something simpler that says "if the input pointer is valid, then
>> `get` must return a valid output pointer"?
>
> Hmm, wouldn't this give impression that "you can do whatever you want if the
> input pointer is not valid"?

Yes that's true, but why is that a problem?

>>> +#[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`?
>
> I'm following the name on `SliceIndex`:
> https://doc.rust-lang.org/stable/std/slice/trait.SliceIndex.html.

Hmm, the methods in that trait are marked as unstable under
`slice_index_methods`, which doesn't have a tracking issue, so are
perma-unstable? I'll suggest the rename upstream as well.

Cheers,
Benno

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

[Benno Lossin]

On Mon Mar 2, 2026 at 3:48 PM CET, Danilo Krummrich wrote:
> On Mon Mar 2, 2026 at 3:38 PM CET, Benno Lossin wrote:
>> [1]: https://doc.rust-lang.org/std/mem/fn.size_of_val_raw.html
>
> I recently would have had a use-case for this as well, but besides the reasons
> you can't use it in this patch, I think it's also still unstable?

Oh yeah that's right.

Cheers,
Benno

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

[Gary Guo]

On Mon Mar 2, 2026 at 6:49 PM GMT, Benno Lossin wrote:
> On Mon Mar 2, 2026 at 3:49 PM CET, Gary Guo wrote:
>> On Mon Mar 2, 2026 at 2:38 PM GMT, Benno Lossin wrote:
>>> On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
>>>> +/// 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.
>>
>> The "in bounds" here is the conceptual bounds of the pointer. So, for a pointer
>> with size `x`, the address of the returned pointer lies between `ptr .. ptr +
>> x`.
>
> Okay, I haven't really seen that as a concept. Also, what is the size of
> an invalid pointer?

It's `size_of::<T>()` for sized types, and `size_of::<T>() * slice.len()` for a
raw slice pointer.

The projection semantics is same regardless whether it's valid or not. The I/O
projection work will rely on this, as many I/O impls will act on pointers that
are not "valid" in Rust sense because they refer to a different address space.
But they're still legit pointers with proper meaning.

>
>>> 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
>>
>> I have a `KnownSize` trait for this in my I/O projection series that is
>> implemented for `T: Sized` and `[T]`, and it returns the size when given a raw
>> pointer.
>>
>>>
>>> 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.
>>
>> Given that I'm not introducing `KnownSize` trait in this patch, this is why I
>> haven't used this kind of wording. Perhaps I can just bring `KnownSize` in early
>> and use it first for documentation purpose only?
>
> That sounds great.
>
>>> Or something simpler that says "if the input pointer is valid, then
>>> `get` must return a valid output pointer"?
>>
>> Hmm, wouldn't this give impression that "you can do whatever you want if the
>> input pointer is not valid"?
>
> Yes that's true, but why is that a problem?

A impl that returns an arbitrary pointer when given a null pointer is not valid.

I/O projection will use the ability to project on null pointers, too. An example
is PCI config space code, which will project using null pointer as starting
pointer.

The "bounds" projected pointer must still be with in `0..KnownSize::size(ptr)`.

>
>>>> +#[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`?
>>
>> I'm following the name on `SliceIndex`:
>> https://doc.rust-lang.org/stable/std/slice/trait.SliceIndex.html.
>
> Hmm, the methods in that trait are marked as unstable under
> `slice_index_methods`, which doesn't have a tracking issue, so are
> perma-unstable? I'll suggest the rename upstream as well.

I mean, they're named after the `slice::get` method [1] and `Index::index`
method [2], so I don't really see the naming issue here.

[1]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.get
[2]: https://doc.rust-lang.org/stable/std/primitive.slice.html#method.index-24

Best,
Gary

>
> Cheers,
> Benno

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

[Benno Lossin]

On Mon Mar 2, 2026 at 9:14 PM CET, Gary Guo wrote:
> On Mon Mar 2, 2026 at 6:49 PM GMT, Benno Lossin wrote:
>> On Mon Mar 2, 2026 at 3:49 PM CET, Gary Guo wrote:
>>> On Mon Mar 2, 2026 at 2:38 PM GMT, Benno Lossin wrote:
>>>> On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
>>>>> +/// 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.
>>>
>>> The "in bounds" here is the conceptual bounds of the pointer. So, for a pointer
>>> with size `x`, the address of the returned pointer lies between `ptr .. ptr +
>>> x`.
>>
>> Okay, I haven't really seen that as a concept. Also, what is the size of
>> an invalid pointer?
>
> It's `size_of::<T>()` for sized types, and `size_of::<T>() * slice.len()` for a
> raw slice pointer.

And for `dyn Trait`?

Do you have a link to somewhere?

> The projection semantics is same regardless whether it's valid or not. The I/O
> projection work will rely on this, as many I/O impls will act on pointers that
> are not "valid" in Rust sense because they refer to a different address space.
> But they're still legit pointers with proper meaning.

I like the solution with `KnownSize`. The previous safety requirement
was nebulous IMO.

>>>> 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
>>>
>>> I have a `KnownSize` trait for this in my I/O projection series that is
>>> implemented for `T: Sized` and `[T]`, and it returns the size when given a raw
>>> pointer.
>>>
>>>>
>>>> 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.
>>>
>>> Given that I'm not introducing `KnownSize` trait in this patch, this is why I
>>> haven't used this kind of wording. Perhaps I can just bring `KnownSize` in early
>>> and use it first for documentation purpose only?
>>
>> That sounds great.
>>
>>>> Or something simpler that says "if the input pointer is valid, then
>>>> `get` must return a valid output pointer"?
>>>
>>> Hmm, wouldn't this give impression that "you can do whatever you want if the
>>> input pointer is not valid"?
>>
>> Yes that's true, but why is that a problem?
>
> A impl that returns an arbitrary pointer when given a null pointer is not valid.
>
> I/O projection will use the ability to project on null pointers, too. An example
> is PCI config space code, which will project using null pointer as starting
> pointer.
>
> The "bounds" projected pointer must still be with in `0..KnownSize::size(ptr)`.

I would not have assumed this to be valid with the previous comment.

Cheers,
Benno

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

[Gary Guo]

On Mon Mar 2, 2026 at 10:01 PM GMT, Benno Lossin wrote:
> On Mon Mar 2, 2026 at 9:14 PM CET, Gary Guo wrote:
>> On Mon Mar 2, 2026 at 6:49 PM GMT, Benno Lossin wrote:
>>> On Mon Mar 2, 2026 at 3:49 PM CET, Gary Guo wrote:
>>>> On Mon Mar 2, 2026 at 2:38 PM GMT, Benno Lossin wrote:
>>>>> On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
>>>>>> +/// 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.
>>>>
>>>> The "in bounds" here is the conceptual bounds of the pointer. So, for a pointer
>>>> with size `x`, the address of the returned pointer lies between `ptr .. ptr +
>>>> x`.
>>>
>>> Okay, I haven't really seen that as a concept. Also, what is the size of
>>> an invalid pointer?
>>
>> It's `size_of::<T>()` for sized types, and `size_of::<T>() * slice.len()` for a
>> raw slice pointer.
>
> And for `dyn Trait`?
>
> Do you have a link to somewhere?

For `dyn Trait` it would be the size in the vtable, which is always available as
vtable metadata on a raw pointer is required to be valid anyway (this is
something that lang team has already decided so that trait upcasting could work
for raw pointers).

I am basically just having `size_of_val_raw` in mind when writing this. So the
current `KnownSize` comment in v4 is something that I am happy about.

>
>> The projection semantics is same regardless whether it's valid or not. The I/O
>> projection work will rely on this, as many I/O impls will act on pointers that
>> are not "valid" in Rust sense because they refer to a different address space.
>> But they're still legit pointers with proper meaning.
>
> I like the solution with `KnownSize`. The previous safety requirement
> was nebulous IMO.

Best,
Gary

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

[Benno Lossin]

On Mon Mar 2, 2026 at 11:19 PM CET, Gary Guo wrote:
> On Mon Mar 2, 2026 at 10:01 PM GMT, Benno Lossin wrote:
>> On Mon Mar 2, 2026 at 9:14 PM CET, Gary Guo wrote:
>>> On Mon Mar 2, 2026 at 6:49 PM GMT, Benno Lossin wrote:
>>>> On Mon Mar 2, 2026 at 3:49 PM CET, Gary Guo wrote:
>>>>> On Mon Mar 2, 2026 at 2:38 PM GMT, Benno Lossin wrote:
>>>>>> On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
>>>>>>> +/// 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.
>>>>>
>>>>> The "in bounds" here is the conceptual bounds of the pointer. So, for a pointer
>>>>> with size `x`, the address of the returned pointer lies between `ptr .. ptr +
>>>>> x`.
>>>>
>>>> Okay, I haven't really seen that as a concept. Also, what is the size of
>>>> an invalid pointer?
>>>
>>> It's `size_of::<T>()` for sized types, and `size_of::<T>() * slice.len()` for a
>>> raw slice pointer.
>>
>> And for `dyn Trait`?
>>
>> Do you have a link to somewhere?
>
> For `dyn Trait` it would be the size in the vtable, which is always available as
> vtable metadata on a raw pointer is required to be valid anyway (this is
> something that lang team has already decided so that trait upcasting could work
> for raw pointers).

I really would like to see some docs of that, I didn't find anything in
the reference, official docs, or nomicon. The reference does say [1]
that:

    `dyn Trait` metadata must be a pointer to a compiler-generated
    vtable for Trait. (For raw pointers, this requirement remains a
    subject of some debate.)

Do you know where it was decided? I did find this [2] UCG issue that
covers it, but that doesn't seem like the decision, just the discussion.

[1]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html#r-undefined.validity.wide
[2]: https://github.com/rust-lang/unsafe-code-guidelines/issues/516

> I am basically just having `size_of_val_raw` in mind when writing this. So the
> current `KnownSize` comment in v4 is something that I am happy about.

Well size_of_val_raw is `unsafe` and only valid to call in certain
conditions. It asks in the case of slices that the length is an
initialized integer and that the entire value must fit into `isize`.
This to me just further indicates that `*mut T` has safety
requirements to obtaining the size of an arbitrary pointer.

In the special cases of `T: Sized` and `T == [U]`, we have safe ways of
getting their size.

Cheers,
Benno

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

[Gary Guo]

On Tue Mar 3, 2026 at 9:14 AM GMT, Benno Lossin wrote:
> On Mon Mar 2, 2026 at 11:19 PM CET, Gary Guo wrote:
>> On Mon Mar 2, 2026 at 10:01 PM GMT, Benno Lossin wrote:
>>> On Mon Mar 2, 2026 at 9:14 PM CET, Gary Guo wrote:
>>>> On Mon Mar 2, 2026 at 6:49 PM GMT, Benno Lossin wrote:
>>>>> On Mon Mar 2, 2026 at 3:49 PM CET, Gary Guo wrote:
>>>>>> On Mon Mar 2, 2026 at 2:38 PM GMT, Benno Lossin wrote:
>>>>>>> On Mon Mar 2, 2026 at 2:02 PM CET, Gary Guo wrote:
>>>>>>>> +/// 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.
>>>>>>
>>>>>> The "in bounds" here is the conceptual bounds of the pointer. So, for a pointer
>>>>>> with size `x`, the address of the returned pointer lies between `ptr .. ptr +
>>>>>> x`.
>>>>>
>>>>> Okay, I haven't really seen that as a concept. Also, what is the size of
>>>>> an invalid pointer?
>>>>
>>>> It's `size_of::<T>()` for sized types, and `size_of::<T>() * slice.len()` for a
>>>> raw slice pointer.
>>>
>>> And for `dyn Trait`?
>>>
>>> Do you have a link to somewhere?
>>
>> For `dyn Trait` it would be the size in the vtable, which is always available as
>> vtable metadata on a raw pointer is required to be valid anyway (this is
>> something that lang team has already decided so that trait upcasting could work
>> for raw pointers).
>
> I really would like to see some docs of that, I didn't find anything in
> the reference, official docs, or nomicon. The reference does say [1]
> that:
>
>     `dyn Trait` metadata must be a pointer to a compiler-generated
>     vtable for Trait. (For raw pointers, this requirement remains a
>     subject of some debate.)
>
> Do you know where it was decided? I did find this [2] UCG issue that
> covers it, but that doesn't seem like the decision, just the discussion.

The FCP is here: https://github.com/rust-lang/rust/issues/101336

The gist: for a fat raw pointer with `dyn Trait` metadata, the safety invariant
requires a fully valid vtable. Validity invariant is not yet decided.

>
> [1]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html#r-undefined.validity.wide
> [2]: https://github.com/rust-lang/unsafe-code-guidelines/issues/516
>
>> I am basically just having `size_of_val_raw` in mind when writing this. So the
>> current `KnownSize` comment in v4 is something that I am happy about.
>
> Well size_of_val_raw is `unsafe` and only valid to call in certain
> conditions. It asks in the case of slices that the length is an
> initialized integer and that the entire value must fit into `isize`.
> This to me just further indicates that `*mut T` has safety
> requirements to obtaining the size of an arbitrary pointer.
>
> In the special cases of `T: Sized` and `T == [U]`, we have safe ways of
> getting their size.

Hmm, the `isize` fitting requirement is problematic indeed. It's broken code if
pointer projection is used with an allocation that exceeds the limit, but I want
the API to be safe, so it'll be good if the API is defined to just be wrapping
and safe (it may return values that doesn't make sense, but that'll be on the
user).

Anyhow this is moot as we're going the `KnownSize` route.

Best,
Gary

>
> Cheers,
> Benno

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

[Alice Ryhl]

On Tue, Mar 03, 2026 at 10:17:01AM +0000, Gary Guo wrote:
> On Tue Mar 3, 2026 at 9:14 AM GMT, Benno Lossin wrote:
> > On Mon Mar 2, 2026 at 11:19 PM CET, Gary Guo wrote:
> >> I am basically just having `size_of_val_raw` in mind when writing this. So the
> >> current `KnownSize` comment in v4 is something that I am happy about.
> >
> > Well size_of_val_raw is `unsafe` and only valid to call in certain
> > conditions. It asks in the case of slices that the length is an
> > initialized integer and that the entire value must fit into `isize`.
> > This to me just further indicates that `*mut T` has safety
> > requirements to obtaining the size of an arbitrary pointer.
> >
> > In the special cases of `T: Sized` and `T == [U]`, we have safe ways of
> > getting their size.
> 
> Hmm, the `isize` fitting requirement is problematic indeed. It's broken code if
> pointer projection is used with an allocation that exceeds the limit, but I want
> the API to be safe, so it'll be good if the API is defined to just be wrapping
> and safe (it may return values that doesn't make sense, but that'll be on the
> user).
> 
> Anyhow this is moot as we're going the `KnownSize` route.

It sounds like that's no different from dyn trait vtable case. Fat
pointer must have valid metadata, even if raw pointer.

Alice

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

[Gary Guo]

On Tue Mar 3, 2026 at 11:39 AM GMT, Alice Ryhl wrote:
> On Tue, Mar 03, 2026 at 10:17:01AM +0000, Gary Guo wrote:
>> On Tue Mar 3, 2026 at 9:14 AM GMT, Benno Lossin wrote:
>> > On Mon Mar 2, 2026 at 11:19 PM CET, Gary Guo wrote:
>> >> I am basically just having `size_of_val_raw` in mind when writing this. So the
>> >> current `KnownSize` comment in v4 is something that I am happy about.
>> >
>> > Well size_of_val_raw is `unsafe` and only valid to call in certain
>> > conditions. It asks in the case of slices that the length is an
>> > initialized integer and that the entire value must fit into `isize`.
>> > This to me just further indicates that `*mut T` has safety
>> > requirements to obtaining the size of an arbitrary pointer.
>> >
>> > In the special cases of `T: Sized` and `T == [U]`, we have safe ways of
>> > getting their size.
>> 
>> Hmm, the `isize` fitting requirement is problematic indeed. It's broken code if
>> pointer projection is used with an allocation that exceeds the limit, but I want
>> the API to be safe, so it'll be good if the API is defined to just be wrapping
>> and safe (it may return values that doesn't make sense, but that'll be on the
>> user).
>> 
>> Anyhow this is moot as we're going the `KnownSize` route.
>
> It sounds like that's no different from dyn trait vtable case. Fat
> pointer must have valid metadata, even if raw pointer.

If you have a `*const dyn Trait`, then yes, it's all valid. However, the `isize`
fitting requirement means that if you have `*const TypeWithUnsizedTail<dyn
Trait>`, the fixed-sized prefix, when added together with the `dyn Trait` tail,
may exceed isize::MAX and violate the safety requirement of `size_of_val_raw`.

Best,
Gary

>
> Alice