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

Boqun Feng
2025-03-19T15:25:53Z

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