| Message ID | 20250314-ptr-as-ptr-v3-6-e7ba61048f4a@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | rust: reduce pointer casts, enable related lints | expand |
On Fri Mar 14, 2025 at 1:28 PM CET, Tamir Duberstein wrote: > Rust 1.84.0 stabilized the strict provenance APIs[1]. > > This patch enables the (unstable) lints `fuzzy_provenance_casts` and > `lossy_provenance_casts` (available since Rust 1.61.0[2]) and uses > strict provenance APIs where these lints triggered. The `kernel` crate > is kept backwards-compatible by introducing forwarding functions at the > root which are marked `#[allow(clippy::incompatible_msrv)]` to avoid > warnings on rustc < 1.84.0. > > The discussion in the tracking Issue for strict_provenance_lints[3] > seems to be nearing resolution with the only open question being: > >> do we really want two separate lints for the two directions? > > which seems minor enough that this is unlikely to cause significant > churn when stabilized. > > This is limited to the `kernel` crate because adding these lints in the > root `Makefile` causes `core` itself to be compiled with them, which in > turn causes warnings on the implementations of the strict provenance > APIs themselves. This isn't the case anymore? (ie it isn't limited to the `kernel` crate?) > Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1] > Link: https://github.com/rust-lang/rust/blob/e71f9a9a98b0faf423844bf0ba7438f29dc27d58/compiler/rustc_feature/src/unstable.rs#L605 [2] > Link: https://github.com/rust-lang/rust/issues/130351 [3] > Suggested-by: Benno Lossin <benno.lossin@proton.me> > Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/ Missing SoB. > --- > Makefile | 9 ++++++++- > init/Kconfig | 3 +++ > rust/Makefile | 26 ++++++++++++++++++++------ > rust/kernel/alloc.rs | 2 +- > rust/kernel/devres.rs | 4 ++-- > rust/kernel/io.rs | 14 +++++++------- > rust/kernel/lib.rs | 20 ++++++++++++++++++++ > rust/kernel/of.rs | 2 +- > rust/kernel/pci.rs | 4 ++-- > rust/kernel/str.rs | 16 ++++++---------- > rust/kernel/uaccess.rs | 12 ++++++++---- > scripts/Makefile.build | 2 +- > scripts/Makefile.host | 4 ++++ > 13 files changed, 83 insertions(+), 35 deletions(-) Thanks for making the effort and getting this to work, unfortunately, I have checked if this compiles with 1.78 and sadly it doesn't. That's because that version doesn't have the `with_exposed_provenance` function! I am very sorry about that, I thought I had checked that before suggesting it to you. It does exist in >=1.79 && <1.84 under a different feature: `exposed_provenance`. (I haven't checked if everything works when also adding that) I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is going to be in debian trixie, so eventually we could bump it to that, but I'm not sure what the time frame will be for that. Maybe we can salvage this effort by gating both the lint and the unstable features on the versions where it works? @Miguel, what's your opinion? We could even make it simple, requiring 1.84 and not bothering with the older versions. > diff --git a/Makefile b/Makefile > index 2af40bfed9ce..bc12650783f1 100644 > --- a/Makefile > +++ b/Makefile > @@ -473,6 +473,8 @@ export rust_common_flags := --edition=2021 \ > -Astable_features \ > -Dnon_ascii_idents \ > -Dunsafe_op_in_unsafe_fn \ > + -Wfuzzy_provenance_casts \ > + -Wlossy_provenance_casts \ > -Wmissing_docs \ > -Wrust_2018_idioms \ > -Wunreachable_pub \ > @@ -498,7 +500,7 @@ KBUILD_HOSTCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \ > KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) \ > -I $(srctree)/scripts/include > KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \ > - -Zallow-features= $(HOSTRUSTFLAGS) > + $(HOSTRUSTFLAGS) This should be mentioned explicitly in the commit message. > KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS) > KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS) > KBUILD_PROCMACROLDFLAGS := $(or $(PROCMACROLDFLAGS),$(KBUILD_HOSTLDFLAGS)) > @@ -870,6 +872,11 @@ KBUILD_CFLAGS += -Os > KBUILD_RUSTFLAGS += -Copt-level=s > endif > > +# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized. > +# > +# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64. > +export rustc_strict_provenance_feature := $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance) > + > # Always set `debug-assertions` and `overflow-checks` because their default > # depends on `opt-level` and `debug-assertions`, respectively. > KBUILD_RUSTFLAGS += -Cdebug-assertions=$(if $(CONFIG_RUST_DEBUG_ASSERTIONS),y,n) > diff --git a/init/Kconfig b/init/Kconfig > index 324c2886b2ea..04df2893348c 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/Makefile b/rust/Makefile > index ea3849eb78f6..dad47bea19f3 100644 > --- a/rust/Makefile > +++ b/rust/Makefile > @@ -57,10 +57,12 @@ endif > core-cfgs = \ > --cfg no_fp_fmt_parse > > +rustc_strict_provenance_flags = -Zcrate-attr='feature($(rustc_strict_provenance_feature))' > + > quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $< > cmd_rustdoc = \ > OBJTREE=$(abspath $(objtree)) \ > - $(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags))) \ > + $(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) $(rustc_strict_provenance_flags)) \ > $(rustc_target_flags) -L$(objtree)/$(obj) \ > -Zunstable-options --generate-link-to-definition \ > --output $(rustdoc_output) \ > @@ -99,7 +101,7 @@ rustdoc-macros: $(src)/macros/lib.rs FORCE > > # Starting with Rust 1.82.0, skipping `-Wrustdoc::unescaped_backticks` should > # not be needed -- see https://github.com/rust-lang/rust/pull/128307. > -rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks > +rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks $(rustc_strict_provenance_flags) Why aren't the lints excluded? > rustdoc-core: private rustc_target_flags = $(core-cfgs) > rustdoc-core: $(RUST_LIB_SRC)/core/src/lib.rs FORCE > +$(call if_changed,rustdoc) > @@ -436,7 +450,7 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE > $(obj)/exports.o: private skip_gendwarfksyms = 1 > > $(obj)/core.o: private skip_clippy = 1 > -$(obj)/core.o: private skip_flags = -Wunreachable_pub > +$(obj)/core.o: private skip_flags = -Wunreachable_pub -Wlossy_provenance_casts $(rustc_strict_provenance_flags) Why not also the `-Wfuzzy_provenance_casts`? They could be introduced at any moment in core, so I don't think that we should enable the lint there. > $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym)) > $(obj)/core.o: private rustc_target_flags = $(core-cfgs) > $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \ > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index 9cd6b6864739..ebf7db3ad9ee 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -25,6 +25,26 @@ > #![feature(const_ptr_write)] > #![feature(const_refs_to_cell)] > > +#[allow(clippy::incompatible_msrv)] > +mod strict_provenance { > + #[doc(hidden)] > + pub fn expose_provenance<T>(addr: *const T) -> usize { > + addr.expose_provenance() > + } > + > + #[doc(hidden)] > + pub fn with_exposed_provenance<T>(addr: usize) -> *const T { > + core::ptr::with_exposed_provenance(addr) > + } > + > + #[doc(hidden)] > + pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T { > + core::ptr::with_exposed_provenance_mut(addr) > + } > +} Still, I don't think we should re-export them. It'll only confuse folks and the `incompatible_msrv` lint isn't useful at the moment. > + > +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/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), Just import it, you're also using it below in the file. --- Cheers, Benno > } > } >
On Fri, Mar 14, 2025 at 08:28:10AM -0400, Tamir Duberstein wrote: [...] > --- 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::with_exposed_provenance_mut(layout.align()); Dangling pointers don't have provenance, neither has its provenance been exposed. I think should use `without_provenance_mut()` here: https://doc.rust-lang.org/std/ptr/fn.without_provenance_mut.html see also the source of core::ptr::dangling(). The rest Rust code changes look good to me. Although I would suggest you to split this patch into several patches: you can do the conversion from "as" pattern to provenance API one file by one file, and this make it easier for people to review. And after the conversions are done, you can introduce the Makefile changes. Regards, Boqun [...]
On Fri, Mar 14, 2025 at 9:18 PM Benno Lossin <benno.lossin@proton.me> wrote: > > I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is > going to be in debian trixie, so eventually we could bump it to that, > but I'm not sure what the time frame will be for that. > > Maybe we can salvage this effort by gating both the lint and the > unstable features on the versions where it works? @Miguel, what's your > opinion? > > We could even make it simple, requiring 1.84 and not bothering with the > older versions. Regarding Debian Trixie: unknown, since my understanding is that it does not have a release date yet, but apparently mid May is the Hard Freeze and then it may take e.g. a month or two to the release. And when it releases, we may want to wait a while before bumping it, depending on how much time has passed since Rust 1.85.0 and depending on whether we managed to get e.g. Ubuntu LTSs to provide a versioned package etc. If something simple works, then let's just go for that -- we do not care too much about older versions for linting purposes, since people should be testing with the latest stable too anyway. Thanks! Cheers, Miguel
On Fri, Mar 14, 2025 at 6:00 PM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Fri, Mar 14, 2025 at 9:18 PM Benno Lossin <benno.lossin@proton.me> wrote: > > > > I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is > > going to be in debian trixie, so eventually we could bump it to that, > > but I'm not sure what the time frame will be for that. > > > > Maybe we can salvage this effort by gating both the lint and the > > unstable features on the versions where it works? @Miguel, what's your > > opinion? > > > > We could even make it simple, requiring 1.84 and not bothering with the > > older versions. > > Regarding Debian Trixie: unknown, since my understanding is that it > does not have a release date yet, but apparently mid May is the Hard > Freeze and then it may take e.g. a month or two to the release. > > And when it releases, we may want to wait a while before bumping it, > depending on how much time has passed since Rust 1.85.0 and depending > on whether we managed to get e.g. Ubuntu LTSs to provide a versioned > package etc. > > If something simple works, then let's just go for that -- we do not > care too much about older versions for linting purposes, since people > should be testing with the latest stable too anyway. It's not going to be simple because `rust_common_flags` is defined before the config is read, which means I'll have to sprinkle conditional logic in even more places to enable the lints. The most minimal version of this patch would drop all the build system changes and just have conditionally compiled polyfills for the strict provenance APIs. Are folks OK with that?
Hi Tamir,
kernel test robot noticed the following build errors:
[auto build test ERROR on a1eb95d6b5f4cf5cc7b081e85e374d1dd98a213b]
url: https://github.com/intel-lab-lkp/linux/commits/Tamir-Duberstein/rust-retain-pointer-mut-ness-in-container_of/20250315-003150
base: a1eb95d6b5f4cf5cc7b081e85e374d1dd98a213b
patch link: https://lore.kernel.org/r/20250314-ptr-as-ptr-v3-6-e7ba61048f4a%40gmail.com
patch subject: [PATCH v3 6/6] rust: use strict provenance APIs
config: x86_64-randconfig-002-20250315 (https://download.01.org/0day-ci/archive/20250315/202503151519.6bGsjUd3-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
rustc: rustc 1.78.0 (9b00956e5 2024-04-29)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250315/202503151519.6bGsjUd3-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503151519.6bGsjUd3-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from arch/x86/kernel/asm-offsets.c:14:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:21:
In file included from include/linux/mm.h:2224:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
***
*** Rust bindings generator 'bindgen' < 0.69.5 together with libclang >= 19.1
*** may not work due to a bug (https://github.com/rust-lang/rust-bindgen/pull/2824),
*** unless patched (like Debian's).
*** Your bindgen version: 0.65.1
*** Your libclang version: 19.1.7
***
***
*** Please see Documentation/rust/quick-start.rst for details
*** on how to set up the Rust support.
***
In file included from rust/helpers/helpers.c:10:
In file included from rust/helpers/blk.c:3:
In file included from include/linux/blk-mq.h:5:
In file included from include/linux/blkdev.h:9:
In file included from include/linux/blk_types.h:10:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:8:
In file included from include/linux/cacheflush.h:5:
In file included from arch/x86/include/asm/cacheflush.h:5:
In file included from include/linux/mm.h:2224:
include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
505 | item];
| ~~~~
include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
512 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.
clang diag: include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
clang diag: include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>> error[E0425]: cannot find function `with_exposed_provenance` in module `core::ptr`
--> rust/kernel/lib.rs:37:20
|
37 | core::ptr::with_exposed_provenance(addr)
| ^^^^^^^^^^^^^^^^^^^^^^^
|
::: /opt/cross/rustc-1.78.0-bindgen-0.65.1/rustup/toolchains/1.78.0-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:593:1
|
593 | pub const fn without_provenance<T>(addr: usize) -> *const T {
| ----------------------------------------------------------- similarly named function `without_provenance` defined here
|
help: a function with a similar name exists
|
37 | core::ptr::without_provenance(addr)
| ~~~~~~~~~~~~~~~~~~
help: consider importing this function through its public re-export
|
30 + use crate::with_exposed_provenance;
|
help: if you import `with_exposed_provenance`, refer to it directly
|
37 - core::ptr::with_exposed_provenance(addr)
37 + with_exposed_provenance(addr)
|
--
>> error[E0425]: cannot find function `with_exposed_provenance_mut` in module `core::ptr`
--> rust/kernel/lib.rs:42:20
|
42 | core::ptr::with_exposed_provenance_mut(addr)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
::: /opt/cross/rustc-1.78.0-bindgen-0.65.1/rustup/toolchains/1.78.0-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:637:1
|
637 | pub const fn without_provenance_mut<T>(addr: usize) -> *mut T {
| ------------------------------------------------------------- similarly named function `without_provenance_mut` defined here
|
help: a function with a similar name exists
|
42 | core::ptr::without_provenance_mut(addr)
| ~~~~~~~~~~~~~~~~~~~~~~
help: consider importing this function through its public re-export
|
30 + use crate::with_exposed_provenance_mut;
|
help: if you import `with_exposed_provenance_mut`, refer to it directly
|
42 - core::ptr::with_exposed_provenance_mut(addr)
42 + with_exposed_provenance_mut(addr)
|
--
>> error[E0599]: no method named `expose_provenance` found for raw pointer `*const T` in the current scope
--> rust/kernel/lib.rs:32:14
|
32 | addr.expose_provenance()
| ^^^^^^^^^^^^^^^^^ method not found in `*const T`
|
= note: try using `<*const T>::as_ref()` to get a reference to the type behind the pointer: https://doc.rust-lang.org/std/primitive.pointer.html#method.as_ref
= note: using `<*const T>::as_ref()` on a pointer which is unaligned or points to invalid or uninitialized memory is undefined behavior
On Fri Mar 14, 2025 at 10:54 PM CET, Boqun Feng wrote: > On Fri, Mar 14, 2025 at 08:28:10AM -0400, Tamir Duberstein wrote: > [...] >> --- 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::with_exposed_provenance_mut(layout.align()); > > Dangling pointers don't have provenance, neither has its provenance been > exposed. I think should use `without_provenance_mut()` here: > > https://doc.rust-lang.org/std/ptr/fn.without_provenance_mut.html > > see also the source of core::ptr::dangling(). Good catch. > The rest Rust code changes look good to me. Although I would suggest you > to split this patch into several patches: you can do the conversion from > "as" pattern to provenance API one file by one file, and this make it > easier for people to review. And after the conversions are done, you can > introduce the Makefile changes. I think it's fine to do several of the `as` conversions in a single patch, but splitting off the makefile changes is a good idea. --- Cheers, Benno
On Fri Mar 14, 2025 at 11:20 PM CET, Tamir Duberstein wrote: > On Fri, Mar 14, 2025 at 6:00 PM Miguel Ojeda > <miguel.ojeda.sandonis@gmail.com> wrote: >> >> On Fri, Mar 14, 2025 at 9:18 PM Benno Lossin <benno.lossin@proton.me> wrote: >> > >> > I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is >> > going to be in debian trixie, so eventually we could bump it to that, >> > but I'm not sure what the time frame will be for that. >> > >> > Maybe we can salvage this effort by gating both the lint and the >> > unstable features on the versions where it works? @Miguel, what's your >> > opinion? >> > >> > We could even make it simple, requiring 1.84 and not bothering with the >> > older versions. >> >> Regarding Debian Trixie: unknown, since my understanding is that it >> does not have a release date yet, but apparently mid May is the Hard >> Freeze and then it may take e.g. a month or two to the release. >> >> And when it releases, we may want to wait a while before bumping it, >> depending on how much time has passed since Rust 1.85.0 and depending >> on whether we managed to get e.g. Ubuntu LTSs to provide a versioned >> package etc. Yeah that's what I thought, thanks for confirming. >> If something simple works, then let's just go for that -- we do not >> care too much about older versions for linting purposes, since people >> should be testing with the latest stable too anyway. > > It's not going to be simple because `rust_common_flags` is defined > before the config is read, which means I'll have to sprinkle > conditional logic in even more places to enable the lints. > > The most minimal version of this patch would drop all the build system > changes and just have conditionally compiled polyfills for the strict > provenance APIs. Are folks OK with that? So you'd not enable the lint, but fix all occurrences? I think we should still have the lint (if it's too cumbersome, then let's only enable it in the kernel crate). --- Cheers, Benno
On Sat, Mar 15, 2025 at 5:44 AM Benno Lossin <benno.lossin@proton.me> wrote: > > On Fri Mar 14, 2025 at 11:20 PM CET, Tamir Duberstein wrote: > > On Fri, Mar 14, 2025 at 6:00 PM Miguel Ojeda > > <miguel.ojeda.sandonis@gmail.com> wrote: > >> > >> On Fri, Mar 14, 2025 at 9:18 PM Benno Lossin <benno.lossin@proton.me> wrote: > >> > > >> > I don't know when we'll be bumping the minimum version. IIRC 1.85.0 is > >> > going to be in debian trixie, so eventually we could bump it to that, > >> > but I'm not sure what the time frame will be for that. > >> > > >> > Maybe we can salvage this effort by gating both the lint and the > >> > unstable features on the versions where it works? @Miguel, what's your > >> > opinion? > >> > > >> > We could even make it simple, requiring 1.84 and not bothering with the > >> > older versions. > >> > >> Regarding Debian Trixie: unknown, since my understanding is that it > >> does not have a release date yet, but apparently mid May is the Hard > >> Freeze and then it may take e.g. a month or two to the release. > >> > >> And when it releases, we may want to wait a while before bumping it, > >> depending on how much time has passed since Rust 1.85.0 and depending > >> on whether we managed to get e.g. Ubuntu LTSs to provide a versioned > >> package etc. > > Yeah that's what I thought, thanks for confirming. > > >> If something simple works, then let's just go for that -- we do not > >> care too much about older versions for linting purposes, since people > >> should be testing with the latest stable too anyway. > > > > It's not going to be simple because `rust_common_flags` is defined > > before the config is read, which means I'll have to sprinkle > > conditional logic in even more places to enable the lints. > > > > The most minimal version of this patch would drop all the build system > > changes and just have conditionally compiled polyfills for the strict > > provenance APIs. Are folks OK with that? > > So you'd not enable the lint, but fix all occurrences? I think we should > still have the lint (if it's too cumbersome, then let's only enable it > in the kernel crate).
On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote: [...] > > The rest Rust code changes look good to me. Although I would suggest you > > to split this patch into several patches: you can do the conversion from > > "as" pattern to provenance API one file by one file, and this make it > > easier for people to review. And after the conversions are done, you can > > introduce the Makefile changes. > > I think it's fine to do several of the `as` conversions in a single Well, "fine" != "recommended", right? ;-) If the patch was split, reviewers would be able to give Reviewed-by to individual patches that looks fine trivially. Then it's easier to make progress every iteration, and also allows partially applying the changes. Of course it doesn't have to be file-by-file. Regards, Boqun > patch, but splitting off the makefile changes is a good idea. > > --- > Cheers, > Benno >
On Sat, Mar 15, 2025 at 8:37 AM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote: > [...] > > > The rest Rust code changes look good to me. Although I would suggest you > > > to split this patch into several patches: you can do the conversion from > > > "as" pattern to provenance API one file by one file, and this make it > > > easier for people to review. And after the conversions are done, you can > > > introduce the Makefile changes. > > > > I think it's fine to do several of the `as` conversions in a single > > Well, "fine" != "recommended", right? ;-) If the patch was split, > reviewers would be able to give Reviewed-by to individual patches that > looks fine trivially. Then it's easier to make progress every iteration, > and also allows partially applying the changes. Of course it doesn't > have to be file-by-file. I sent v4 a little while ago, hopefully the resulting complexity is manageable now that the build system is untouched. Cheers. Tamir
On Sat, Mar 15, 2025 at 08:41:49AM -0400, Tamir Duberstein wrote: > On Sat, Mar 15, 2025 at 8:37 AM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote: > > [...] > > > > The rest Rust code changes look good to me. Although I would suggest you > > > > to split this patch into several patches: you can do the conversion from > > > > "as" pattern to provenance API one file by one file, and this make it > > > > easier for people to review. And after the conversions are done, you can > > > > introduce the Makefile changes. > > > > > > I think it's fine to do several of the `as` conversions in a single > > > > Well, "fine" != "recommended", right? ;-) If the patch was split, > > reviewers would be able to give Reviewed-by to individual patches that > > looks fine trivially. Then it's easier to make progress every iteration, > > and also allows partially applying the changes. Of course it doesn't > > have to be file-by-file. > > I sent v4 a little while ago, hopefully the resulting complexity is > manageable now that the build system is untouched. > I have fun plans today (skiing!), so won't be able to take another detailed look. What I was trying to say is that: should you split the patches, I would have already given some Reviewed-bys ;-) But as Benno said, it's fine, so don't worry, I will take another look later. Thanks! Regards, Boqun > Cheers. > > Tamir
On Sat, Mar 15, 2025 at 8:59 AM Boqun Feng <boqun.feng@gmail.com> wrote: > > On Sat, Mar 15, 2025 at 08:41:49AM -0400, Tamir Duberstein wrote: > > On Sat, Mar 15, 2025 at 8:37 AM Boqun Feng <boqun.feng@gmail.com> wrote: > > > > > > On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote: > > > [...] > > > > > The rest Rust code changes look good to me. Although I would suggest you > > > > > to split this patch into several patches: you can do the conversion from > > > > > "as" pattern to provenance API one file by one file, and this make it > > > > > easier for people to review. And after the conversions are done, you can > > > > > introduce the Makefile changes. > > > > > > > > I think it's fine to do several of the `as` conversions in a single > > > > > > Well, "fine" != "recommended", right? ;-) If the patch was split, > > > reviewers would be able to give Reviewed-by to individual patches that > > > looks fine trivially. Then it's easier to make progress every iteration, > > > and also allows partially applying the changes. Of course it doesn't > > > have to be file-by-file. > > > > I sent v4 a little while ago, hopefully the resulting complexity is > > manageable now that the build system is untouched. > > > > I have fun plans today (skiing!), so won't be able to take another > detailed look. What I was trying to say is that: should you split the > patches, I would have already given some Reviewed-bys ;-) But as Benno > said, it's fine, so don't worry, I will take another look later. Thanks! Have fun! ⛷️
On Sat Mar 15, 2025 at 1:37 PM CET, Boqun Feng wrote: > On Sat, Mar 15, 2025 at 09:34:42AM +0000, Benno Lossin wrote: > [...] >> > The rest Rust code changes look good to me. Although I would suggest you >> > to split this patch into several patches: you can do the conversion from >> > "as" pattern to provenance API one file by one file, and this make it >> > easier for people to review. And after the conversions are done, you can >> > introduce the Makefile changes. >> >> I think it's fine to do several of the `as` conversions in a single > > Well, "fine" != "recommended", right? ;-) If the patch was split, > reviewers would be able to give Reviewed-by to individual patches that > looks fine trivially. Then it's easier to make progress every iteration, > and also allows partially applying the changes. Of course it doesn't > have to be file-by-file. While I see your point, in this case splitting file-by-file is too much. v4 has: 9 files changed, 82 insertions(+), 27 deletions(-). I've seen much bigger changes that do smaller things like this patch. At around 150 lines added + deleted I find it more and more difficult. --- Cheers, Benno
diff --git a/Makefile b/Makefile index 2af40bfed9ce..bc12650783f1 100644 --- a/Makefile +++ b/Makefile @@ -473,6 +473,8 @@ export rust_common_flags := --edition=2021 \ -Astable_features \ -Dnon_ascii_idents \ -Dunsafe_op_in_unsafe_fn \ + -Wfuzzy_provenance_casts \ + -Wlossy_provenance_casts \ -Wmissing_docs \ -Wrust_2018_idioms \ -Wunreachable_pub \ @@ -498,7 +500,7 @@ KBUILD_HOSTCFLAGS := $(KBUILD_USERHOSTCFLAGS) $(HOST_LFS_CFLAGS) \ KBUILD_HOSTCXXFLAGS := -Wall -O2 $(HOST_LFS_CFLAGS) $(HOSTCXXFLAGS) \ -I $(srctree)/scripts/include KBUILD_HOSTRUSTFLAGS := $(rust_common_flags) -O -Cstrip=debuginfo \ - -Zallow-features= $(HOSTRUSTFLAGS) + $(HOSTRUSTFLAGS) KBUILD_HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) $(HOSTLDFLAGS) KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS) KBUILD_PROCMACROLDFLAGS := $(or $(PROCMACROLDFLAGS),$(KBUILD_HOSTLDFLAGS)) @@ -870,6 +872,11 @@ KBUILD_CFLAGS += -Os KBUILD_RUSTFLAGS += -Copt-level=s endif +# Lints were moved to `strict_provenance_lints` when `strict_provenance` was stabilized. +# +# See https://github.com/rust-lang/rust/commit/56ee492a6e7a917b2b3f888e33dd52a13d3ecb64. +export rustc_strict_provenance_feature := $(if $(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE),strict_provenance_lints,strict_provenance) + # Always set `debug-assertions` and `overflow-checks` because their default # depends on `opt-level` and `debug-assertions`, respectively. KBUILD_RUSTFLAGS += -Cdebug-assertions=$(if $(CONFIG_RUST_DEBUG_ASSERTIONS),y,n) diff --git a/init/Kconfig b/init/Kconfig index 324c2886b2ea..04df2893348c 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/Makefile b/rust/Makefile index ea3849eb78f6..dad47bea19f3 100644 --- a/rust/Makefile +++ b/rust/Makefile @@ -57,10 +57,12 @@ endif core-cfgs = \ --cfg no_fp_fmt_parse +rustc_strict_provenance_flags = -Zcrate-attr='feature($(rustc_strict_provenance_feature))' + quiet_cmd_rustdoc = RUSTDOC $(if $(rustdoc_host),H, ) $< cmd_rustdoc = \ OBJTREE=$(abspath $(objtree)) \ - $(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags))) \ + $(RUSTDOC) $(filter-out $(skip_flags),$(if $(rustdoc_host),$(rust_common_flags),$(rust_flags)) $(rustc_strict_provenance_flags)) \ $(rustc_target_flags) -L$(objtree)/$(obj) \ -Zunstable-options --generate-link-to-definition \ --output $(rustdoc_output) \ @@ -99,7 +101,7 @@ rustdoc-macros: $(src)/macros/lib.rs FORCE # Starting with Rust 1.82.0, skipping `-Wrustdoc::unescaped_backticks` should # not be needed -- see https://github.com/rust-lang/rust/pull/128307. -rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks +rustdoc-core: private skip_flags = -Wrustdoc::unescaped_backticks $(rustc_strict_provenance_flags) rustdoc-core: private rustc_target_flags = $(core-cfgs) rustdoc-core: $(RUST_LIB_SRC)/core/src/lib.rs FORCE +$(call if_changed,rustdoc) @@ -122,6 +124,7 @@ quiet_cmd_rustc_test_library = $(RUSTC_OR_CLIPPY_QUIET) TL $< cmd_rustc_test_library = \ OBJTREE=$(abspath $(objtree)) \ $(RUSTC_OR_CLIPPY) $(rust_common_flags) \ + $(rustc_strict_provenance_flags) \ @$(objtree)/include/generated/rustc_cfg $(rustc_target_flags) \ --crate-type $(if $(rustc_test_library_proc),proc-macro,rlib) \ --out-dir $(objtree)/$(obj)/test --cfg testlib \ @@ -155,11 +158,19 @@ rusttestlib-uapi: private rustc_target_flags = --extern ffi rusttestlib-uapi: $(src)/uapi/lib.rs rusttestlib-ffi FORCE +$(call if_changed,rustc_test_library) +# `rustdoc --test` doesn't respect `-Zcrate-attr`, which means we can't use +# `rustc_strict_provenance_flags` below. Instead we filter out those lints to avoid unknown lint +# warnings. +# +# See https://github.com/rust-lang/rust/issues/138491. +rustc_strict_provenance_lints = -Wfuzzy_provenance_casts -Wlossy_provenance_casts + quiet_cmd_rustdoc_test = RUSTDOC T $< cmd_rustdoc_test = \ RUST_MODFILE=test.rs \ OBJTREE=$(abspath $(objtree)) \ - $(RUSTDOC) --test $(rust_common_flags) \ + $(RUSTDOC) --test \ + $(filter-out $(rustc_strict_provenance_lints),$(rust_common_flags)) \ @$(objtree)/include/generated/rustc_cfg \ $(rustc_target_flags) $(rustdoc_test_target_flags) \ $(rustdoc_test_quiet) \ @@ -171,7 +182,8 @@ quiet_cmd_rustdoc_test_kernel = RUSTDOC TK $< rm -rf $(objtree)/$(obj)/test/doctests/kernel; \ mkdir -p $(objtree)/$(obj)/test/doctests/kernel; \ OBJTREE=$(abspath $(objtree)) \ - $(RUSTDOC) --test $(rust_flags) \ + $(RUSTDOC) --test \ + $(filter-out $(rustc_strict_provenance_lints),$(rust_flags)) \ -L$(objtree)/$(obj) --extern ffi --extern kernel \ --extern build_error --extern macros \ --extern bindings --extern uapi \ @@ -193,6 +205,7 @@ quiet_cmd_rustc_test = $(RUSTC_OR_CLIPPY_QUIET) T $< cmd_rustc_test = \ OBJTREE=$(abspath $(objtree)) \ $(RUSTC_OR_CLIPPY) --test $(rust_common_flags) \ + $(rustc_strict_provenance_flags) \ @$(objtree)/include/generated/rustc_cfg \ $(rustc_target_flags) --out-dir $(objtree)/$(obj)/test \ -L$(objtree)/$(obj)/test \ @@ -362,6 +375,7 @@ $(obj)/exports_kernel_generated.h: $(obj)/kernel.o FORCE quiet_cmd_rustc_procmacro = $(RUSTC_OR_CLIPPY_QUIET) P $@ cmd_rustc_procmacro = \ $(RUSTC_OR_CLIPPY) $(rust_common_flags) \ + $(rustc_strict_provenance_flags) \ -Clinker-flavor=gcc -Clinker=$(HOSTCC) \ -Clink-args='$(call escsq,$(KBUILD_PROCMACROLDFLAGS))' \ --emit=dep-info=$(depfile) --emit=link=$@ --extern proc_macro \ @@ -376,7 +390,7 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L cmd_rustc_library = \ OBJTREE=$(abspath $(objtree)) \ $(if $(skip_clippy),$(RUSTC),$(RUSTC_OR_CLIPPY)) \ - $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags)) \ + $(filter-out $(skip_flags),$(rust_flags) $(rustc_target_flags) $(rustc_strict_provenance_flags)) \ --emit=dep-info=$(depfile) --emit=obj=$@ \ --emit=metadata=$(dir $@)$(patsubst %.o,lib%.rmeta,$(notdir $@)) \ --crate-type rlib -L$(objtree)/$(obj) \ @@ -436,7 +450,7 @@ $(obj)/helpers/helpers.o: $(src)/helpers/helpers.c $(recordmcount_source) FORCE $(obj)/exports.o: private skip_gendwarfksyms = 1 $(obj)/core.o: private skip_clippy = 1 -$(obj)/core.o: private skip_flags = -Wunreachable_pub +$(obj)/core.o: private skip_flags = -Wunreachable_pub -Wlossy_provenance_casts $(rustc_strict_provenance_flags) $(obj)/core.o: private rustc_objcopy = $(foreach sym,$(redirect-intrinsics),--redefine-sym $(sym)=__rust$(sym)) $(obj)/core.o: private rustc_target_flags = $(core-cfgs) $(obj)/core.o: $(RUST_LIB_SRC)/core/src/lib.rs \ diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs index fc9c9c41cd79..59199a6da2ed 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::with_exposed_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 9cd6b6864739..ebf7db3ad9ee 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -25,6 +25,26 @@ #![feature(const_ptr_write)] #![feature(const_refs_to_cell)] +#[allow(clippy::incompatible_msrv)] +mod strict_provenance { + #[doc(hidden)] + pub fn expose_provenance<T>(addr: *const T) -> usize { + addr.expose_provenance() + } + + #[doc(hidden)] + pub fn with_exposed_provenance<T>(addr: usize) -> *const T { + core::ptr::with_exposed_provenance(addr) + } + + #[doc(hidden)] + pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T { + core::ptr::with_exposed_provenance_mut(addr) + } +} + +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..f9459694cbdc 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::expose_provenance(self.0.data) } } diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs index a925732f6c7a..bb38d83e2608 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, ) diff --git a/scripts/Makefile.build b/scripts/Makefile.build index 993708d11874..34575f3be0fc 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,$(rustc_strict_provenance_feature) # `--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/Makefile.host b/scripts/Makefile.host index c1dedf646a39..7e2f35bff59c 100644 --- a/scripts/Makefile.host +++ b/scripts/Makefile.host @@ -87,10 +87,14 @@ hostcxx_flags = -Wp,-MMD,$(depfile) \ $(KBUILD_HOSTCXXFLAGS) $(HOST_EXTRACXXFLAGS) \ $(HOSTCXXFLAGS_$(target-stem).o) +rust_allowed_features := $(rustc_strict_provenance_feature) + # `--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 # modules case. hostrust_flags = --out-dir $(dir $@) --emit=dep-info=$(depfile) \ + -Zallow-features=$(rust_allowed_features) \ + -Zcrate-attr='feature($(rust_allowed_features))' \ -Clinker-flavor=gcc -Clinker=$(HOSTCC) \ -Clink-args='$(call escsq,$(KBUILD_HOSTLDFLAGS))' \ $(KBUILD_HOSTRUSTFLAGS) $(HOST_EXTRARUSTFLAGS) \
Rust 1.84.0 stabilized the strict provenance APIs[1]. This patch enables the (unstable) lints `fuzzy_provenance_casts` and `lossy_provenance_casts` (available since Rust 1.61.0[2]) and uses strict provenance APIs where these lints triggered. The `kernel` crate is kept backwards-compatible by introducing forwarding functions at the root which are marked `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc < 1.84.0. The discussion in the tracking Issue for strict_provenance_lints[3] seems to be nearing resolution with the only open question being: > do we really want two separate lints for the two directions? which seems minor enough that this is unlikely to cause significant churn when stabilized. This is limited to the `kernel` crate because adding these lints in the root `Makefile` causes `core` itself to be compiled with them, which in turn causes warnings on the implementations of the strict provenance APIs themselves. Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1] Link: https://github.com/rust-lang/rust/blob/e71f9a9a98b0faf423844bf0ba7438f29dc27d58/compiler/rustc_feature/src/unstable.rs#L605 [2] Link: https://github.com/rust-lang/rust/issues/130351 [3] Suggested-by: Benno Lossin <benno.lossin@proton.me> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/ --- Makefile | 9 ++++++++- init/Kconfig | 3 +++ rust/Makefile | 26 ++++++++++++++++++++------ rust/kernel/alloc.rs | 2 +- rust/kernel/devres.rs | 4 ++-- rust/kernel/io.rs | 14 +++++++------- rust/kernel/lib.rs | 20 ++++++++++++++++++++ rust/kernel/of.rs | 2 +- rust/kernel/pci.rs | 4 ++-- rust/kernel/str.rs | 16 ++++++---------- rust/kernel/uaccess.rs | 12 ++++++++---- scripts/Makefile.build | 2 +- scripts/Makefile.host | 4 ++++ 13 files changed, 83 insertions(+), 35 deletions(-)