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

Benno Lossin
2025-03-14T20:18:21Z

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

> }
> }
>