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

[Benno Lossin <benno.lossin@proton.me>]

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_provenance.
> -    #[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.html
> -    #[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_provenance.
> -    #[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}