| Message ID | 20250317-ptr-as-ptr-v5-6-5b5f21fa230a@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rust: reduce pointer casts, enable related lints | expand |
On Mon, Mar 17, 2025 at 10:24 AM Tamir Duberstein <tamird@gmail.com> wrote: > > Throughout the tree, use the strict provenance APIs stabilized in Rust > 1.84.0[1]. Retain backwards-compatibility by introducing forwarding > functions at the `kernel` crate root along with polyfills for rustc < > 1.84.0. > > Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc < > 1.84.0 as our MSRV is 1.78.0. > > In the `kernel` crate, enable the strict provenance lints on rustc >= > 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing > compiler flags that are dependent on the rustc version in use. As Benno pointed out on v4, this should probably include: Note that the enablement of the strict provenance lints does not extend to the `kernel` crate's doctests.
On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote: [...] > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index fc6835cc36a3..c1b274c04a0f 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -17,6 +17,11 @@ > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] > +#![cfg_attr( > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE, > + feature(strict_provenance_lints), > + deny(fuzzy_provenance_casts, lossy_provenance_casts) > +)] > #![feature(inline_const)] > #![feature(lint_reasons)] > // Stable in Rust 1.83 > @@ -25,6 +30,109 @@ > #![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() > + } > + For addr(), I would just enable feature(strict_provenance) if CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is available for 1.78. Plus we may need with_addr() or map_addr() in the future. It saves the cost of maintaining our own *addr() and removing it when we bump to a strict_provenance stablized version as minimal verision in the future. Thoughts? Regards, Boqun [...] > // 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 40d1bd13682c..b70076d16008 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 { > - self.0.data as usize > + crate::addr(self.0.data) > } > } > [...]
On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote: > Throughout the tree, use the strict provenance APIs stabilized in Rust > 1.84.0[1]. Retain backwards-compatibility by introducing forwarding > functions at the `kernel` crate root along with polyfills for rustc < > 1.84.0. > > Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc < > 1.84.0 as our MSRV is 1.78.0. > > In the `kernel` crate, enable the strict provenance lints on rustc >= > 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing > compiler flags that are dependent on the rustc version in use. > > Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1] > Suggested-by: Benno Lossin <benno.lossin@proton.me> > Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/ > Signed-off-by: Tamir Duberstein <tamird@gmail.com> One comment below, with that fixed: Reviewed-by: Benno Lossin <benno.lossin@proton.me> > --- > init/Kconfig | 3 ++ > rust/kernel/alloc.rs | 2 +- > rust/kernel/devres.rs | 4 +- > rust/kernel/io.rs | 14 +++---- > rust/kernel/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++ > rust/kernel/of.rs | 2 +- > rust/kernel/pci.rs | 4 +- > rust/kernel/str.rs | 16 +++----- > rust/kernel/uaccess.rs | 12 ++++-- > 9 files changed, 138 insertions(+), 27 deletions(-) > +#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))] > +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 { > + // 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::<()>()) > + } I think we should just use `ptr as usize` here instead. It's going away at some point and it will only affect optimizations (I don't even know if they exist at the moment) of old versions. --- Cheers, Benno > + } > + > + /// 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 > + } > + > + /// 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 { > + addr as *const T > + } > + > + /// 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 { > + addr as *mut T > + } > + > + /// 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 { > + addr as *mut T > + } > +} > + > +pub use strict_provenance::*; > + > // Ensure conditional compilation based on the kernel configuration works; > // otherwise we may silently break things like initcall handling. > #[cfg(not(CONFIG_RUST))]
On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote: > [...] > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > index fc6835cc36a3..c1b274c04a0f 100644 > > --- a/rust/kernel/lib.rs > > +++ b/rust/kernel/lib.rs > > @@ -17,6 +17,11 @@ > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] > > +#![cfg_attr( > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE, > > + feature(strict_provenance_lints), > > + deny(fuzzy_provenance_casts, lossy_provenance_casts) > > +)] > > #![feature(inline_const)] > > #![feature(lint_reasons)] > > // Stable in Rust 1.83 > > @@ -25,6 +30,109 @@ > > #![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() > > + } > > + > > For addr(), I would just enable feature(strict_provenance) if > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is > available for 1.78. Plus we may need with_addr() or map_addr() in the > future. We still need these stubs to avoid `clippy::incompatible_msrv`, and we'll need those until MSRV is above 1.84. > > It saves the cost of maintaining our own *addr() and removing it when > we bump to a strict_provenance stablized version as minimal verision in > the future. Thoughts? > I can do this by making this particular stub unconditional. I'll do that.
On Mon, Mar 17, 2025 at 02:04:34PM -0400, Tamir Duberstein wrote: > On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote: > > [...] > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > > index fc6835cc36a3..c1b274c04a0f 100644 > > > --- a/rust/kernel/lib.rs > > > +++ b/rust/kernel/lib.rs > > > @@ -17,6 +17,11 @@ > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] > > > +#![cfg_attr( > > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE, > > > + feature(strict_provenance_lints), > > > + deny(fuzzy_provenance_casts, lossy_provenance_casts) > > > +)] > > > #![feature(inline_const)] > > > #![feature(lint_reasons)] > > > // Stable in Rust 1.83 > > > @@ -25,6 +30,109 @@ > > > #![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() > > > + } > > > + > > > > For addr(), I would just enable feature(strict_provenance) if > > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is > > available for 1.78. Plus we may need with_addr() or map_addr() in the > > future. > > We still need these stubs to avoid `clippy::incompatible_msrv`, and > we'll need those until MSRV is above 1.84. > Hmm.. why? Clippy cannot work with unstable features? Regards, Boqun > > > > It saves the cost of maintaining our own *addr() and removing it when > > we bump to a strict_provenance stablized version as minimal verision in > > the future. Thoughts? > > > > I can do this by making this particular stub unconditional. I'll do that.
On Mon, Mar 17, 2025 at 2:06 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Mon, Mar 17, 2025 at 02:04:34PM -0400, Tamir Duberstein wrote: > > On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote: > > > [...] > > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > > > index fc6835cc36a3..c1b274c04a0f 100644 > > > > --- a/rust/kernel/lib.rs > > > > +++ b/rust/kernel/lib.rs > > > > @@ -17,6 +17,11 @@ > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] > > > > +#![cfg_attr( > > > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE, > > > > + feature(strict_provenance_lints), > > > > + deny(fuzzy_provenance_casts, lossy_provenance_casts) > > > > +)] > > > > #![feature(inline_const)] > > > > #![feature(lint_reasons)] > > > > // Stable in Rust 1.83 > > > > @@ -25,6 +30,109 @@ > > > > #![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() > > > > + } > > > > + > > > > > > For addr(), I would just enable feature(strict_provenance) if > > > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is > > > available for 1.78. Plus we may need with_addr() or map_addr() in the > > > future. > > > > We still need these stubs to avoid `clippy::incompatible_msrv`, and > > we'll need those until MSRV is above 1.84. > > > > Hmm.. why? Clippy cannot work with unstable features? Yes, `clippy::incompatible_msrv` doesn't pay attention to enabled unstable features.
On Mon, Mar 17, 2025 at 02:10:42PM -0400, Tamir Duberstein wrote: > On Mon, Mar 17, 2025 at 2:06 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > On Mon, Mar 17, 2025 at 02:04:34PM -0400, Tamir Duberstein wrote: > > > On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote: > > > > [...] > > > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > > > > index fc6835cc36a3..c1b274c04a0f 100644 > > > > > --- a/rust/kernel/lib.rs > > > > > +++ b/rust/kernel/lib.rs > > > > > @@ -17,6 +17,11 @@ > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] > > > > > +#![cfg_attr( > > > > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE, > > > > > + feature(strict_provenance_lints), > > > > > + deny(fuzzy_provenance_casts, lossy_provenance_casts) > > > > > +)] > > > > > #![feature(inline_const)] > > > > > #![feature(lint_reasons)] > > > > > // Stable in Rust 1.83 > > > > > @@ -25,6 +30,109 @@ > > > > > #![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() > > > > > + } > > > > > + > > > > > > > > For addr(), I would just enable feature(strict_provenance) if > > > > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is > > > > available for 1.78. Plus we may need with_addr() or map_addr() in the > > > > future. > > > > > > We still need these stubs to avoid `clippy::incompatible_msrv`, and > > > we'll need those until MSRV is above 1.84. > > > > > > > Hmm.. why? Clippy cannot work with unstable features? > > Yes, `clippy::incompatible_msrv` doesn't pay attention to enabled > unstable features. Then we should fix clippy or how we set msrv rather adding the stub. @Miguel? Regards, Boqun
On Mon, Mar 17, 2025 at 1:50 PM Benno Lossin <benno.lossin@proton.me> wrote: > > On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote: > > Throughout the tree, use the strict provenance APIs stabilized in Rust > > 1.84.0[1]. Retain backwards-compatibility by introducing forwarding > > functions at the `kernel` crate root along with polyfills for rustc < > > 1.84.0. > > > > Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc < > > 1.84.0 as our MSRV is 1.78.0. > > > > In the `kernel` crate, enable the strict provenance lints on rustc >= > > 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing > > compiler flags that are dependent on the rustc version in use. > > > > Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1] > > Suggested-by: Benno Lossin <benno.lossin@proton.me> > > Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/ > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > One comment below, with that fixed: > > Reviewed-by: Benno Lossin <benno.lossin@proton.me> > > > --- > > init/Kconfig | 3 ++ > > rust/kernel/alloc.rs | 2 +- > > rust/kernel/devres.rs | 4 +- > > rust/kernel/io.rs | 14 +++---- > > rust/kernel/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++ > > rust/kernel/of.rs | 2 +- > > rust/kernel/pci.rs | 4 +- > > rust/kernel/str.rs | 16 +++----- > > rust/kernel/uaccess.rs | 12 ++++-- > > 9 files changed, 138 insertions(+), 27 deletions(-) > > > > +#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))] > > +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 { > > + // 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::<()>()) > > + } > > I think we should just use `ptr as usize` here instead. It's going away > at some point and it will only affect optimizations (I don't even know > if they exist at the moment) of old versions. Why get cute? I'd rather defer to the standard library.
On Mon, Mar 17, 2025 at 2:31 PM Tamir Duberstein <tamird@gmail.com> wrote: > > On Mon, Mar 17, 2025 at 1:50 PM Benno Lossin <benno.lossin@proton.me> wrote: > > > > On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote: > > > Throughout the tree, use the strict provenance APIs stabilized in Rust > > > 1.84.0[1]. Retain backwards-compatibility by introducing forwarding > > > functions at the `kernel` crate root along with polyfills for rustc < > > > 1.84.0. > > > > > > Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc < > > > 1.84.0 as our MSRV is 1.78.0. > > > > > > In the `kernel` crate, enable the strict provenance lints on rustc >= > > > 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing > > > compiler flags that are dependent on the rustc version in use. > > > > > > Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1] > > > Suggested-by: Benno Lossin <benno.lossin@proton.me> > > > Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/ > > > Signed-off-by: Tamir Duberstein <tamird@gmail.com> > > > > One comment below, with that fixed: > > > > Reviewed-by: Benno Lossin <benno.lossin@proton.me> > > > > > --- > > > init/Kconfig | 3 ++ > > > rust/kernel/alloc.rs | 2 +- > > > rust/kernel/devres.rs | 4 +- > > > rust/kernel/io.rs | 14 +++---- > > > rust/kernel/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > rust/kernel/of.rs | 2 +- > > > rust/kernel/pci.rs | 4 +- > > > rust/kernel/str.rs | 16 +++----- > > > rust/kernel/uaccess.rs | 12 ++++-- > > > 9 files changed, 138 insertions(+), 27 deletions(-) > > > > > > > +#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))] > > > +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 { > > > + // 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::<()>()) > > > + } > > > > I think we should just use `ptr as usize` here instead. It's going away > > at some point and it will only affect optimizations (I don't even know > > if they exist at the moment) of old versions. > > Why get cute? I'd rather defer to the standard library. Ah, this is gone anyway with Boqun's suggestion - this function exists in 1.78.
On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Mon, Mar 17, 2025 at 02:10:42PM -0400, Tamir Duberstein wrote: > > On Mon, Mar 17, 2025 at 2:06 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > On Mon, Mar 17, 2025 at 02:04:34PM -0400, Tamir Duberstein wrote: > > > > On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote: > > > > > [...] > > > > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > > > > > > index fc6835cc36a3..c1b274c04a0f 100644 > > > > > > --- a/rust/kernel/lib.rs > > > > > > +++ b/rust/kernel/lib.rs > > > > > > @@ -17,6 +17,11 @@ > > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] > > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] > > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] > > > > > > +#![cfg_attr( > > > > > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE, > > > > > > + feature(strict_provenance_lints), > > > > > > + deny(fuzzy_provenance_casts, lossy_provenance_casts) > > > > > > +)] > > > > > > #![feature(inline_const)] > > > > > > #![feature(lint_reasons)] > > > > > > // Stable in Rust 1.83 > > > > > > @@ -25,6 +30,109 @@ > > > > > > #![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() > > > > > > + } > > > > > > + > > > > > > > > > > For addr(), I would just enable feature(strict_provenance) if > > > > > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is > > > > > available for 1.78. Plus we may need with_addr() or map_addr() in the > > > > > future. > > > > > > > > We still need these stubs to avoid `clippy::incompatible_msrv`, and > > > > we'll need those until MSRV is above 1.84. > > > > > > > > > > Hmm.. why? Clippy cannot work with unstable features? > > > > Yes, `clippy::incompatible_msrv` doesn't pay attention to enabled > > unstable features. > > Then we should fix clippy or how we set msrv rather adding the stub. > @Miguel? I filed https://github.com/rust-lang/rust-clippy/issues/14425.
On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote: > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > Then we should fix clippy or how we set msrv rather adding the stub. > > @Miguel? > > I filed https://github.com/rust-lang/rust-clippy/issues/14425. I don't think we can wait for that to be fixed, though. Usually clippy is distributed with rustc via rustup, so even if this is eventually fixed, all versions between 1.84.0 and the fix will need this workaround until MSRV is >= 1.84.0.
On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote: > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > Then we should fix clippy or how we set msrv rather adding the stub. > > > @Miguel? > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425. > > I don't think we can wait for that to be fixed, though. Usually clippy > is distributed with rustc via rustup, so even if this is eventually > fixed, all versions between 1.84.0 and the fix will need this > workaround until MSRV is >= 1.84.0. We need to take one step back to evalute this "workaround". First, expose_provenance() and with_exposed_provenance{,_mut}() API are clearly defined as equavilent to `as` operation [1]. Therefore, the changes in this patch doing the conversion with expose_provenance() and with_exposed_provenance{,_mut}() don't change anything related to provenance in practice. I do agree we want to use the explicit provenance API, but I don't think we want to introduce some API that we know we will change them latter when we bump the rustc minimal version. So the question is: are these stubs what we want even though in the future our minimal rustc version stablizes provenance API? If not, then the cost of this patch cannot justify its benefits IMO. Now let's also look into why we choose a msrv for clippy, I would guess it's because we need to support all the versions of rustc starting at 1.78 and we want clippy to report a problem based on 1.78 even though we're using a higher version of rustc. But for this particular case, we use a feature that has already been stablized in a higher version of rustc, which means the problem reported by clippy doesn't help us, nor does it provide better code. Frankly speaking, I think we have other ways to ensure the support of all rustc versions without a msrv for clippy. If I was to choose, I would simply drop the msrv. But maybe I'm missing something. The point is tools should help us to write good and maintainable code, we shouldn't introduce complicated structure of code just because some tools fail to do its job. [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html Regards, Boqun
On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote: > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > Then we should fix clippy or how we set msrv rather adding the stub. > > > > @Miguel? > > > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425. > > > > I don't think we can wait for that to be fixed, though. Usually clippy > > is distributed with rustc via rustup, so even if this is eventually > > fixed, all versions between 1.84.0 and the fix will need this > > workaround until MSRV is >= 1.84.0. > > We need to take one step back to evalute this "workaround". > > First, expose_provenance() and with_exposed_provenance{,_mut}() API are > clearly defined as equavilent to `as` operation [1]. Therefore, the > changes in this patch doing the conversion with expose_provenance() and > with_exposed_provenance{,_mut}() don't change anything related to > provenance in practice. > > I do agree we want to use the explicit provenance API, but I don't think > we want to introduce some API that we know we will change them latter > when we bump the rustc minimal version. So the question is: are these > stubs what we want even though in the future our minimal rustc version > stablizes provenance API? If not, then the cost of this patch cannot > justify its benefits IMO. > > Now let's also look into why we choose a msrv for clippy, I would guess > it's because we need to support all the versions of rustc starting at > 1.78 and we want clippy to report a problem based on 1.78 even though > we're using a higher version of rustc. But for this particular case, we > use a feature that has already been stablized in a higher version of > rustc, which means the problem reported by clippy doesn't help us, nor > does it provide better code. Frankly speaking, I think we have other > ways to ensure the support of all rustc versions without a msrv for > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm > missing something. > > The point is tools should help us to write good and maintainable code, > we shouldn't introduce complicated structure of code just because some > tools fail to do its job. > > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html Even if we globally disable this clippy lint, we still need stubs because exposed_provenance was added in 1.79.0. Did your suggestion address this? Perhaps I missed it.
On Mon, Mar 17, 2025 at 04:35:42PM -0400, Tamir Duberstein wrote: > On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote: > > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > > > Then we should fix clippy or how we set msrv rather adding the stub. > > > > > @Miguel? > > > > > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425. > > > > > > I don't think we can wait for that to be fixed, though. Usually clippy > > > is distributed with rustc via rustup, so even if this is eventually > > > fixed, all versions between 1.84.0 and the fix will need this > > > workaround until MSRV is >= 1.84.0. > > > > We need to take one step back to evalute this "workaround". > > > > First, expose_provenance() and with_exposed_provenance{,_mut}() API are > > clearly defined as equavilent to `as` operation [1]. Therefore, the > > changes in this patch doing the conversion with expose_provenance() and > > with_exposed_provenance{,_mut}() don't change anything related to > > provenance in practice. > > > > I do agree we want to use the explicit provenance API, but I don't think > > we want to introduce some API that we know we will change them latter > > when we bump the rustc minimal version. So the question is: are these > > stubs what we want even though in the future our minimal rustc version > > stablizes provenance API? If not, then the cost of this patch cannot > > justify its benefits IMO. > > > > Now let's also look into why we choose a msrv for clippy, I would guess > > it's because we need to support all the versions of rustc starting at > > 1.78 and we want clippy to report a problem based on 1.78 even though > > we're using a higher version of rustc. But for this particular case, we > > use a feature that has already been stablized in a higher version of > > rustc, which means the problem reported by clippy doesn't help us, nor > > does it provide better code. Frankly speaking, I think we have other > > ways to ensure the support of all rustc versions without a msrv for > > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm > > missing something. > > > > The point is tools should help us to write good and maintainable code, > > we shouldn't introduce complicated structure of code just because some > > tools fail to do its job. > > > > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html > > Even if we globally disable this clippy lint, we still need stubs > because exposed_provenance was added in 1.79.0. Did your suggestion > address this? Perhaps I missed it. No, I didn't. That's a separate topic though, because I can see the argument that: because with_exposed_provenance() is a function rather than a method, it won't be very benefical to use ptr::with_exposed_provenance() instead of kernel::with_exposed_provenance(), therefor these stubs of exposed_provenance make sense to exist. But I don't think the same argument works for ptr::{with_,map_,}addr(). Regards, Boqun
On Mon, Mar 17, 2025 at 4:46 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Mon, Mar 17, 2025 at 04:35:42PM -0400, Tamir Duberstein wrote: > > On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote: > > > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > > > > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > > > > > Then we should fix clippy or how we set msrv rather adding the stub. > > > > > > @Miguel? > > > > > > > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425. > > > > > > > > I don't think we can wait for that to be fixed, though. Usually clippy > > > > is distributed with rustc via rustup, so even if this is eventually > > > > fixed, all versions between 1.84.0 and the fix will need this > > > > workaround until MSRV is >= 1.84.0. > > > > > > We need to take one step back to evalute this "workaround". > > > > > > First, expose_provenance() and with_exposed_provenance{,_mut}() API are > > > clearly defined as equavilent to `as` operation [1]. Therefore, the > > > changes in this patch doing the conversion with expose_provenance() and > > > with_exposed_provenance{,_mut}() don't change anything related to > > > provenance in practice. > > > > > > I do agree we want to use the explicit provenance API, but I don't think > > > we want to introduce some API that we know we will change them latter > > > when we bump the rustc minimal version. So the question is: are these > > > stubs what we want even though in the future our minimal rustc version > > > stablizes provenance API? If not, then the cost of this patch cannot > > > justify its benefits IMO. > > > > > > Now let's also look into why we choose a msrv for clippy, I would guess > > > it's because we need to support all the versions of rustc starting at > > > 1.78 and we want clippy to report a problem based on 1.78 even though > > > we're using a higher version of rustc. But for this particular case, we > > > use a feature that has already been stablized in a higher version of > > > rustc, which means the problem reported by clippy doesn't help us, nor > > > does it provide better code. Frankly speaking, I think we have other > > > ways to ensure the support of all rustc versions without a msrv for > > > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm > > > missing something. > > > > > > The point is tools should help us to write good and maintainable code, > > > we shouldn't introduce complicated structure of code just because some > > > tools fail to do its job. > > > > > > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html > > > > Even if we globally disable this clippy lint, we still need stubs > > because exposed_provenance was added in 1.79.0. Did your suggestion > > address this? Perhaps I missed it. > > No, I didn't. > > That's a separate topic though, because I can see the argument that: > because with_exposed_provenance() is a function rather than a method, it > won't be very benefical to use ptr::with_exposed_provenance() instead of > kernel::with_exposed_provenance(), therefor these stubs of > exposed_provenance make sense to exist. But I don't think the same > argument works for ptr::{with_,map_,}addr(). What about `pointer::expose_provenance`? It's a method that was added in 1.79.0. We can certainly disable the clippy lint rather than add stubs for `pointer::{with_,map_,}addr`, but it doesn't bring us to a solution where only free functions require stubs.
On Mon, Mar 17, 2025 at 04:53:18PM -0400, Tamir Duberstein wrote: > On Mon, Mar 17, 2025 at 4:46 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > On Mon, Mar 17, 2025 at 04:35:42PM -0400, Tamir Duberstein wrote: > > > On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote: > > > > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > > > > > > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > > > > > > > Then we should fix clippy or how we set msrv rather adding the stub. > > > > > > > @Miguel? > > > > > > > > > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425. > > > > > > > > > > I don't think we can wait for that to be fixed, though. Usually clippy > > > > > is distributed with rustc via rustup, so even if this is eventually > > > > > fixed, all versions between 1.84.0 and the fix will need this > > > > > workaround until MSRV is >= 1.84.0. > > > > > > > > We need to take one step back to evalute this "workaround". > > > > > > > > First, expose_provenance() and with_exposed_provenance{,_mut}() API are > > > > clearly defined as equavilent to `as` operation [1]. Therefore, the > > > > changes in this patch doing the conversion with expose_provenance() and > > > > with_exposed_provenance{,_mut}() don't change anything related to > > > > provenance in practice. > > > > > > > > I do agree we want to use the explicit provenance API, but I don't think > > > > we want to introduce some API that we know we will change them latter > > > > when we bump the rustc minimal version. So the question is: are these > > > > stubs what we want even though in the future our minimal rustc version > > > > stablizes provenance API? If not, then the cost of this patch cannot > > > > justify its benefits IMO. > > > > > > > > Now let's also look into why we choose a msrv for clippy, I would guess > > > > it's because we need to support all the versions of rustc starting at > > > > 1.78 and we want clippy to report a problem based on 1.78 even though > > > > we're using a higher version of rustc. But for this particular case, we > > > > use a feature that has already been stablized in a higher version of > > > > rustc, which means the problem reported by clippy doesn't help us, nor > > > > does it provide better code. Frankly speaking, I think we have other > > > > ways to ensure the support of all rustc versions without a msrv for > > > > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm > > > > missing something. > > > > > > > > The point is tools should help us to write good and maintainable code, > > > > we shouldn't introduce complicated structure of code just because some > > > > tools fail to do its job. > > > > > > > > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html > > > > > > Even if we globally disable this clippy lint, we still need stubs > > > because exposed_provenance was added in 1.79.0. Did your suggestion > > > address this? Perhaps I missed it. > > > > No, I didn't. > > > > That's a separate topic though, because I can see the argument that: > > because with_exposed_provenance() is a function rather than a method, it > > won't be very benefical to use ptr::with_exposed_provenance() instead of > > kernel::with_exposed_provenance(), therefor these stubs of > > exposed_provenance make sense to exist. But I don't think the same > > argument works for ptr::{with_,map_,}addr(). > > 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. Regards, Boqun > We can certainly disable the clippy lint rather than add stubs for > `pointer::{with_,map_,}addr`, but it doesn't bring us to a solution > where only free functions require stubs.
On Mon, Mar 17, 2025 at 5:36 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Mon, Mar 17, 2025 at 04:53:18PM -0400, Tamir Duberstein wrote: > > On Mon, Mar 17, 2025 at 4:46 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > On Mon, Mar 17, 2025 at 04:35:42PM -0400, Tamir Duberstein wrote: > > > > On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > > > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote: > > > > > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote: > > > > > > > > > > > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > > > > > > > > > > > Then we should fix clippy or how we set msrv rather adding the stub. > > > > > > > > @Miguel? > > > > > > > > > > > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425. > > > > > > > > > > > > I don't think we can wait for that to be fixed, though. Usually clippy > > > > > > is distributed with rustc via rustup, so even if this is eventually > > > > > > fixed, all versions between 1.84.0 and the fix will need this > > > > > > workaround until MSRV is >= 1.84.0. > > > > > > > > > > We need to take one step back to evalute this "workaround". > > > > > > > > > > First, expose_provenance() and with_exposed_provenance{,_mut}() API are > > > > > clearly defined as equavilent to `as` operation [1]. Therefore, the > > > > > changes in this patch doing the conversion with expose_provenance() and > > > > > with_exposed_provenance{,_mut}() don't change anything related to > > > > > provenance in practice. > > > > > > > > > > I do agree we want to use the explicit provenance API, but I don't think > > > > > we want to introduce some API that we know we will change them latter > > > > > when we bump the rustc minimal version. So the question is: are these > > > > > stubs what we want even though in the future our minimal rustc version > > > > > stablizes provenance API? If not, then the cost of this patch cannot > > > > > justify its benefits IMO. > > > > > > > > > > Now let's also look into why we choose a msrv for clippy, I would guess > > > > > it's because we need to support all the versions of rustc starting at > > > > > 1.78 and we want clippy to report a problem based on 1.78 even though > > > > > we're using a higher version of rustc. But for this particular case, we > > > > > use a feature that has already been stablized in a higher version of > > > > > rustc, which means the problem reported by clippy doesn't help us, nor > > > > > does it provide better code. Frankly speaking, I think we have other > > > > > ways to ensure the support of all rustc versions without a msrv for > > > > > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm > > > > > missing something. > > > > > > > > > > The point is tools should help us to write good and maintainable code, > > > > > we shouldn't introduce complicated structure of code just because some > > > > > tools fail to do its job. > > > > > > > > > > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html > > > > > > > > Even if we globally disable this clippy lint, we still need stubs > > > > because exposed_provenance was added in 1.79.0. Did your suggestion > > > > address this? Perhaps I missed it. > > > > > > No, I didn't. > > > > > > That's a separate topic though, because I can see the argument that: > > > because with_exposed_provenance() is a function rather than a method, it > > > won't be very benefical to use ptr::with_exposed_provenance() instead of > > > kernel::with_exposed_provenance(), therefor these stubs of > > > exposed_provenance make sense to exist. But I don't think the same > > > argument works for ptr::{with_,map_,}addr(). > > > > 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(). I don't think we want these stubs forever. > 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) I like this idea, but I can't get it to work. When both inherent and trait methods are available, the compiler seems to prefer the inherent method. > We can also make with_exposed_provenance() use the same *Ext trick, > and remove it when we bump the minimal rustc version. This part I don't understand. What would we impl the Ext on, given that `with_exposed_provenance` is a free function? Option 3) take this series without the last commit, and revisit when MSRV >= 1.79.0 or >= 1.84.0?
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 ;-) 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 { - /// 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> { + /// 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}; + // 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}
On Mon, Mar 17, 2025 at 07:56:09PM -0400, Tamir Duberstein wrote: [..] > > Option 3) take this series without the last commit, and revisit when > MSRV >= 1.79.0 or >= 1.84.0? Works for me as well. Regards, Boqun
On Mon, Mar 17, 2025 at 8:11 PM Boqun Feng <boqun.feng@gmail.com> wrote: > > 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)] Ah, this is the reason this works for you (and the one in the kernel root). When I said it didn't work, I was referring to not being able to convincingly avoid these lints without disabling the check altogether. Let's see what Miguel thinks. I agree that the options are: extension trait + stubs/reexports + suppressing `incompatible_msrv` or just dropping the last patch until MSRV bump.
diff --git a/init/Kconfig b/init/Kconfig index d0d021b3fa3b..82e28d6f7c3f 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -132,6 +132,9 @@ config CC_HAS_COUNTED_BY config RUSTC_HAS_COERCE_POINTEE def_bool RUSTC_VERSION >= 108400 +config RUSTC_HAS_STABLE_STRICT_PROVENANCE + def_bool RUSTC_VERSION >= 108400 + config PAHOLE_VERSION int default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE)) diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs index fc9c9c41cd79..a1d282e48249 100644 --- a/rust/kernel/alloc.rs +++ b/rust/kernel/alloc.rs @@ -217,7 +217,7 @@ unsafe fn free(ptr: NonNull<u8>, layout: Layout) { /// Returns a properly aligned dangling pointer from the given `layout`. pub(crate) fn dangling_from_layout(layout: Layout) -> NonNull<u8> { - let ptr = layout.align() as *mut u8; + let ptr = crate::without_provenance_mut(layout.align()); // SAFETY: `layout.align()` (and hence `ptr`) is guaranteed to be non-zero. unsafe { NonNull::new_unchecked(ptr) } diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs index 34571f992f0d..e8232bb771b2 100644 --- a/rust/kernel/devres.rs +++ b/rust/kernel/devres.rs @@ -64,14 +64,14 @@ struct DevresInner<T> { /// return Err(ENOMEM); /// } /// -/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?)) +/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?)) /// } /// } /// /// impl<const SIZE: usize> Drop for IoMem<SIZE> { /// fn drop(&mut self) { /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`. -/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); }; +/// unsafe { bindings::iounmap(kernel::with_exposed_provenance_mut(self.0.addr())); }; /// } /// } /// diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs index 9d2aadf40edf..0a018ad7478a 100644 --- a/rust/kernel/io.rs +++ b/rust/kernel/io.rs @@ -5,7 +5,7 @@ //! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h) use crate::error::{code::EINVAL, Result}; -use crate::{bindings, build_assert, ffi::c_void}; +use crate::{bindings, build_assert}; /// Raw representation of an MMIO region. /// @@ -75,14 +75,14 @@ pub fn maxsize(&self) -> usize { /// return Err(ENOMEM); /// } /// -/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?)) +/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?)) /// } /// } /// /// impl<const SIZE: usize> Drop for IoMem<SIZE> { /// fn drop(&mut self) { /// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`. -/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); }; +/// unsafe { bindings::iounmap(kernel::with_exposed_provenance_mut(self.0.addr())); }; /// } /// } /// @@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name { let addr = self.io_addr_assert::<$type_name>(offset); // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$name(addr as *const c_void) } + unsafe { bindings::$name(crate::with_exposed_provenance(addr)) } } /// Read IO data from a given offset. @@ -131,7 +131,7 @@ pub fn $try_name(&self, offset: usize) -> Result<$type_name> { let addr = self.io_addr::<$type_name>(offset)?; // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - Ok(unsafe { bindings::$name(addr as *const c_void) }) + Ok(unsafe { bindings::$name(crate::with_exposed_provenance(addr)) }) } }; } @@ -148,7 +148,7 @@ pub fn $name(&self, value: $type_name, offset: usize) { let addr = self.io_addr_assert::<$type_name>(offset); // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$name(value, addr as *mut c_void) } + unsafe { bindings::$name(value, crate::with_exposed_provenance_mut(addr)) } } /// Write IO data from a given offset. @@ -160,7 +160,7 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result { let addr = self.io_addr::<$type_name>(offset)?; // SAFETY: By the type invariant `addr` is a valid address for MMIO operations. - unsafe { bindings::$name(value, addr as *mut c_void) } + unsafe { bindings::$name(value, crate::with_exposed_provenance_mut(addr)) } Ok(()) } }; diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index fc6835cc36a3..c1b274c04a0f 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -17,6 +17,11 @@ #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))] #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))] #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))] +#![cfg_attr( + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE, + feature(strict_provenance_lints), + deny(fuzzy_provenance_casts, lossy_provenance_casts) +)] #![feature(inline_const)] #![feature(lint_reasons)] // Stable in Rust 1.83 @@ -25,6 +30,109 @@ #![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) + } +} + +#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))] +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 { + // 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::<()>()) + } + } + + /// 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 + } + + /// 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 { + addr as *const T + } + + /// 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 { + addr as *mut T + } + + /// 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 { + addr as *mut T + } +} + +pub use strict_provenance::*; + // 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 40d1bd13682c..b70076d16008 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 { - self.0.data as usize + crate::addr(self.0.data) } } diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index a26f154ae1b9..87c9f67b3f0f 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: usize = unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) } as usize; + let ioptr = crate::expose_provenance(unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) }); if ioptr == 0 { // SAFETY: // `pdev` valid by the invariants of `Device`. @@ -320,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) { // `ioptr` is valid by the safety requirements. // `num` is valid by the safety requirements. unsafe { - bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void); + bindings::pci_iounmap(pdev.as_raw(), crate::with_exposed_provenance_mut(ioptr)); bindings::pci_release_region(pdev.as_raw(), num); } } diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs index 0b80a119d5f0..6bc6357293e4 100644 --- a/rust/kernel/str.rs +++ b/rust/kernel/str.rs @@ -692,9 +692,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: pos as usize, - pos: pos as usize, - end: end as usize, + beg: crate::expose_provenance(pos), + pos: crate::expose_provenance(pos), + end: crate::expose_provenance(end), } } @@ -705,7 +705,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 = buf as usize; + let pos = crate::expose_provenance(buf); // INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements // guarantees that the memory region is valid for writes. Self { @@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self { /// /// N.B. It may point to invalid memory. pub(crate) fn pos(&self) -> *mut u8 { - self.pos as *mut u8 + crate::with_exposed_provenance_mut(self.pos) } /// Returns the number of bytes written to the formatter. @@ -741,11 +741,7 @@ fn write_str(&mut self, s: &str) -> fmt::Result { // SAFETY: If `len_to_copy` is non-zero, then we know `pos` has not gone past `end` // yet, so it is valid for write per the type invariants. unsafe { - core::ptr::copy_nonoverlapping( - s.as_bytes().as_ptr(), - self.pos as *mut u8, - len_to_copy, - ) + core::ptr::copy_nonoverlapping(s.as_bytes().as_ptr(), self.pos(), len_to_copy) }; } diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs index 719b0a48ff55..96393bcf6bd7 100644 --- a/rust/kernel/uaccess.rs +++ b/rust/kernel/uaccess.rs @@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result { } // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write // that many bytes to it. - let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) }; + let res = unsafe { + bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len) + }; if res != 0 { return Err(EFAULT); } @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> { let res = unsafe { bindings::_copy_from_user( out.as_mut_ptr().cast::<c_void>(), - self.ptr as *const c_void, + crate::with_exposed_provenance(self.ptr), len, ) }; @@ -330,7 +332,9 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result { } // SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read // that many bytes from it. - let res = unsafe { bindings::copy_to_user(self.ptr as *mut c_void, data_ptr, len) }; + let res = unsafe { + bindings::copy_to_user(crate::with_exposed_provenance_mut(self.ptr), data_ptr, len) + }; if res != 0 { return Err(EFAULT); } @@ -357,7 +361,7 @@ pub fn write<T: AsBytes>(&mut self, value: &T) -> Result { // is a compile-time constant. let res = unsafe { bindings::_copy_to_user( - self.ptr as *mut c_void, + crate::with_exposed_provenance_mut(self.ptr), (value as *const T).cast::<c_void>(), len, )
Throughout the tree, use the strict provenance APIs stabilized in Rust 1.84.0[1]. Retain backwards-compatibility by introducing forwarding functions at the `kernel` crate root along with polyfills for rustc < 1.84.0. Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc < 1.84.0 as our MSRV is 1.78.0. In the `kernel` crate, enable the strict provenance lints on rustc >= 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing compiler flags that are dependent on the rustc version in use. Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1] Suggested-by: Benno Lossin <benno.lossin@proton.me> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/ Signed-off-by: Tamir Duberstein <tamird@gmail.com> --- init/Kconfig | 3 ++ rust/kernel/alloc.rs | 2 +- rust/kernel/devres.rs | 4 +- rust/kernel/io.rs | 14 +++---- rust/kernel/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/of.rs | 2 +- rust/kernel/pci.rs | 4 +- rust/kernel/str.rs | 16 +++----- rust/kernel/uaccess.rs | 12 ++++-- 9 files changed, 138 insertions(+), 27 deletions(-)