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

[Boqun Feng]

On Tue, Mar 18, 2025 at 09:23:42AM +0000, Benno Lossin wrote:
[..]
> > +#![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.
> 

We still need to provide stubs for with_exposed_provenance() and its
friends for rustc == 1.78, so there are a few more functions in this
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.
> 

Right, I was missing this.

> > +        /// 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?
> 

We need re-export these for ructc >=1.79, because for rustc == 1.78 we
only have kernel::expose_provenance() and its friends, therefore
user-side can only use them.

Regards,
Boqun

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