Re: [PATCH v5 6/6] rust: use strict provenance APIs

On Tue Mar 18, 2025 at 1:11 AM CET, Boqun Feng wrote:

On Mon, Mar 17, 2025 at 02:36:08PM -0700, Boqun Feng wrote: [...]

...

...

What about `pointer::expose_provenance`? It's a method that was added in 1.79.0.

We have a few options:

1. we can decide to use funtion-version of expose_provenance() (i.e. the stub), if we feel the symmetry with with_exposed_provenance() is a strong rationale. This also means we won't likely use pointer::expose_provenance() in the future. That is, although kernel doesn't have stable internal API, but in the foreseeable future, we decide to use funtion-version of expose_provenance().

2. we can introduce a PtrExt trait for <1.79

   pub trait PtrExt<T> { fn expose_provenance(self) -> usize; }

   and

   impl<T> PtrExt<T> for *const T { ... }

   and `PtrExt` in kernel::prelude.

   (we need to #[allow(unstable_name_collisions)] to make that work)

   We can also make with_exposed_provenance() use the same *Ext trick, and remove it when we bump the minimal rustc version.

This is probably a wrong suggestion, because with_exposed_provenance() is a function instead of a method in Rust std.

Below is what I combined all together (based on your v5 patchset), and I did test on 1.78, 1.79, 1.84 and 1.85 and it seems working ;-)

I like this a lot, I also thought that we should just disable the `incompatible_msrv` lint. That we get rid of the stubs is a nice bonus :)

The only annoying thing that's left IMO is that we need to conditionally import the `PtrExt` trait, but that would need the built-in prelude feature :(

Couple notes below.

Regards, Boqun ------------------------------------->8 diff --git a/init/Kconfig b/init/Kconfig index 82e28d6f7c3f..e316b98b3612 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -135,6 +135,9 @@ config RUSTC_HAS_COERCE_POINTEE config RUSTC_HAS_STABLE_STRICT_PROVENANCE def_bool RUSTC_VERSION >= 108400 +config RUSTC_HAS_EXPOSED_PROVENANCE

• def_bool RUSTC_VERSION >= 107900

config PAHOLE_VERSION int default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE)) diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index e8232bb771b2..a87a437bd9ab 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -64,7 +64,7 @@ struct DevresInner<T> { /// return Err(ENOMEM); /// } /// -/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?)) +/// Ok(IoMem(IoRaw::new(addr.expose_provenance(), SIZE)?)) /// } /// } /// diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index 0a018ad7478a..60c71f26d29d 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -75,7 +75,7 @@ pub fn maxsize(&self) -> usize { /// return Err(ENOMEM); /// } /// -/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?)) +/// Ok(IoMem(IoRaw::new(addr.expose_provenance(), SIZE)?)) /// } /// } /// diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index c1b274c04a0f..79b19e601372 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -22,6 +22,9 @@ feature(strict_provenance_lints), deny(fuzzy_provenance_casts, lossy_provenance_casts) )] +#![cfg_attr(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE), feature(strict_provenance))] +#![cfg_attr(all(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE), CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE), feature(exposed_provenance))]

#![feature(inline_const)] #![feature(lint_reasons)] // Stable in Rust 1.83 @@ -30,78 +33,24 @@ #![feature(const_ptr_write)] #![feature(const_refs_to_cell)] -#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)] -#[allow(clippy::incompatible_msrv)] -mod strict_provenance {

• /// Gets the "address" portion of the pointer.
• ///
• /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
• #[inline]
• pub fn addr<T>(ptr: *const T) -> usize {

•    ptr.addr()
  

• }
• /// Exposes the "provenance" part of the pointer for future use in
• /// [`with_exposed_provenance`] and returns the "address" portion.
• ///
• /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_p....
• #[inline]
• pub fn expose_provenance<T>(ptr: *const T) -> usize {

•    ptr.expose_provenance()
  

• }
• /// Converts an address back to a pointer, picking up some previously 'exposed'
• /// provenance.
• ///
• /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance.html.
• #[inline]
• pub fn with_exposed_provenance<T>(addr: usize) -> *const T {

•    core::ptr::with_exposed_provenance(addr)
  

• }
• /// Converts an address back to a mutable pointer, picking up some previously 'exposed'
• /// provenance.
• ///
• /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance_mut.htm...
• #[inline]
• pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T {

•    core::ptr::with_exposed_provenance_mut(addr)
  

• }
• /// Creates a pointer with the given address and no [provenance][crate::ptr#provenance].
• ///
• /// See https://doc.rust-lang.org/stable/core/ptr/fn.without_provenance_mut.html.
• #[inline]
• pub fn without_provenance_mut<T>(addr: usize) -> *mut T {

•    core::ptr::without_provenance_mut(addr)
  

• }

-} +#![allow(clippy::incompatible_msrv)] -#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))] +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))] mod strict_provenance {

Since there is only a single trait and impl in here, I think we don't need a module.

• /// Gets the "address" portion of the pointer.
• ///
• /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
• #[inline]
• pub fn addr<T>(ptr: *const T) -> usize {

•    // This is core's implementation from
  

•    // https://github.com/rust-lang/rust/commit/4291332175d12e79e6061cdc3f5dccac2e28b969 through
  

•    // https://github.com/rust-lang/rust/blob/1.84.0/library/core/src/ptr/const_ptr.rs#L172
  

•    // which is the first version that satisfies `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE`.
  

•    #[allow(clippy::undocumented_unsafe_blocks)]
  

•    unsafe {
  

•        #[allow(clippy::transmutes_expressible_as_ptr_casts)]
  

•        core::mem::transmute(ptr.cast::<()>())
  

•    }
  

• #[doc(hidden)]
• pub trait PtrExt<T> {

The `T` here and in the impl below probably should have a `?Sized` bound, since that's also what the stdlib does.

•    /// Exposes the "provenance" part of the pointer for future use in
  

•    /// [`with_exposed_provenance`] and returns the "address" portion.
  

•    ///
  

•    /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
  

•    fn expose_provenance(self) -> usize;
  

  }

• /// Exposes the "provenance" part of the pointer for future use in
• /// [`with_exposed_provenance`] and returns the "address" portion.
• ///
• /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_p....
• #[inline]
• pub fn expose_provenance<T>(ptr: *const T) -> usize {

•    ptr.cast::<()>() as usize
  

• impl<T> PtrExt<T> for *const T {

•    #[inline]
  

•    fn expose_provenance(self) -> usize {
  

•        self.cast::<()>() as usize
  

•    }
  

  }

/// Converts an address back to a pointer, picking up some previously 'exposed' @@ -131,8 +80,12 @@ pub fn without_provenance_mut<T>(addr: usize) -> *mut T { } } +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))] pub use strict_provenance::*; +#[cfg(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE)] +pub use core::ptr::{with_exposed_provenance, with_exposed_provenance_mut, without_provenance_mut};

We shouldn't need this any longer, right?

--- Cheers, Benno

// Ensure conditional compilation based on the kernel configuration works; // otherwise we may silently break things like initcall handling. #[cfg(not(CONFIG_RUST))] diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs index b70076d16008..3670676071ff 100644 --- a/rust/kernel/of.rs +++ b/rust/kernel/of.rs @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId { const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data); fn index(&self) -> usize {

•    crate::addr(self.0.data)
  

•    self.0.data.addr()
  

  }

} diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index 87c9f67b3f0f..73958abdc522 100644 --- a/rust/kernel/pci.rs +++ b/rust/kernel/pci.rs @@ -287,7 +287,7 @@ fn new(pdev: Device, num: u32, name: &CStr) -> Result<Self> { // `pdev` is valid by the invariants of `Device`. // `num` is checked for validity by a previous call to `Device::resource_len`. // `name` is always valid.

•    let ioptr = crate::expose_provenance(unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) });
  

•    let ioptr = unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) }.expose_provenance();
     if ioptr == 0 {
         // SAFETY:
         // `pdev` valid by the invariants of `Device`.
  

diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs index baa774a351ce..3ea6aa9e40e5 100644 --- a/rust/kernel/prelude.rs +++ b/rust/kernel/prelude.rs @@ -41,3 +41,6 @@ pub use super::init::InPlaceInit; pub use super::current;

+#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))] +pub use super::PtrExt; diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 6bc6357293e4..d8e740267f14 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -8,6 +8,9 @@ use crate::error::{code::*, Error}; +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))] +use crate::PtrExt;

/// Byte string without UTF-8 validity guarantee. #[repr(transparent)] pub struct BStr([u8]); @@ -692,9 +695,9 @@ fn new() -> Self { pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self { // INVARIANT: The safety requirements guarantee the type invariants. Self {

•        beg: crate::expose_provenance(pos),
  

•        pos: crate::expose_provenance(pos),
  

•        end: crate::expose_provenance(end),
  

•        beg: pos.expose_provenance(),
  

•        pos: pos.expose_provenance(),
  

•        end: end.expose_provenance(),
     }
  

  }

@@ -705,7 +708,7 @@ pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self { /// The memory region starting at `buf` and extending for `len` bytes must be valid for writes /// for the lifetime of the returned [`RawFormatter`]. pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {

•    let pos = crate::expose_provenance(buf);
  

•    let pos = buf.expose_provenance();
     // INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements
     // guarantees that the memory region is valid for writes.
     Self {
  

diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 08b6380933f5..b070da0ea972 100644 --- a/scripts/Makefile.build +++ b/scripts/Makefile.build @@ -226,7 +226,7 @@ $(obj)/%.lst: $(obj)/%.c FORCE # Compile Rust sources (.rs) # --------------------------------------------------------------------------- -rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons +rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,exposed_provenance # `--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 diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs index 036635fb1621..331ed32adc35 100644 --- a/scripts/rustdoc_test_gen.rs +++ b/scripts/rustdoc_test_gen.rs @@ -224,6 +224,8 @@ macro_rules! assert_eq {{ BufWriter::new(File::create("rust/doctests_kernel_generated.rs").unwrap()), r#"//! `kernel` crate documentation tests. +#![allow(clippy::incompatible_msrv)]

const __LOG_PREFIX: &[u8] = b"rust_doctests_kernel\0"; {rust_tests}