[PATCH v3 0/6] Rust: support `CPU_MITIGATIONS` and enable `objtool`

[PATCH v3 0/6] Rust: support `CPU_MITIGATIONS` and enable `objtool`

[Miguel Ojeda]

Hi,

This is just v2 with the helper function suggested by Peter.

I dropped Benno's and Alice's Tested-bys from the modified patch, just
in case, but the logic should be equivalent.

Cheers,
Miguel

v3:
  - Added `is_rust_noreturn()` helper function (Peter).
  - Reworded a couple bits.

v2: https://lore.kernel.org/rust-for-linux/20240724161501.1319115-1-ojeda@kernel.org/
v1: https://lore.kernel.org/rust-for-linux/20231023174449.251550-1-ojeda@kernel.org/

Miguel Ojeda (6):
  rust: module: add static pointer to `{init,cleanup}_module()`
  x86/rust: support MITIGATION_RETPOLINE
  x86/rust: support MITIGATION_RETHUNK
  x86/rust: support MITIGATION_SLS
  objtool/rust: list `noreturn` Rust functions
  objtool/kbuild/rust: enable objtool for Rust

 arch/x86/Makefile               |  7 ++++-
 rust/Makefile                   | 22 +++++++++------
 rust/macros/module.rs           | 12 +++++++++
 scripts/Makefile.build          |  9 +++++--
 scripts/generate_rust_target.rs | 15 +++++++++++
 tools/objtool/check.c           | 48 ++++++++++++++++++++++++++++++++-
 tools/objtool/noreturns.h       |  2 ++
 7 files changed, 103 insertions(+), 12 deletions(-)


base-commit: b1263411112305acf2af728728591465becb45b0
--
2.45.2

[PATCH v3 1/6] rust: module: add static pointer to `{init,cleanup}_module()`

[Miguel Ojeda]

Add the equivalent of the `___ADDRESSABLE()` annotation in the
`module_{init,exit}` macros to the Rust `module!` macro.

Without this, `objtool` would complain if enabled for Rust (under IBT
builds), e.g.:

    samples/rust/rust_print.o: warning: objtool: cleanup_module(): not an indirect call target
    samples/rust/rust_print.o: warning: objtool: init_module(): not an indirect call target

Tested-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/macros/module.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 411dc103d82e..571ffa2e189c 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -256,6 +256,12 @@ mod __module_init {{
                         unsafe {{ __init() }}
                     }}
 
+                    #[cfg(MODULE)]
+                    #[doc(hidden)]
+                    #[used]
+                    #[link_section = \".init.data\"]
+                    static __UNIQUE_ID___addressable_init_module: unsafe extern \"C\" fn() -> i32 = init_module;
+
                     #[cfg(MODULE)]
                     #[doc(hidden)]
                     #[no_mangle]
@@ -269,6 +275,12 @@ mod __module_init {{
                         unsafe {{ __exit() }}
                     }}
 
+                    #[cfg(MODULE)]
+                    #[doc(hidden)]
+                    #[used]
+                    #[link_section = \".exit.data\"]
+                    static __UNIQUE_ID___addressable_cleanup_module: extern \"C\" fn() = cleanup_module;
+
                     // Built-in modules are initialized through an initcall pointer
                     // and the identifiers need to be unique.
                     #[cfg(not(MODULE))]
-- 
2.45.2

Re: [PATCH v3 1/6] rust: module: add static pointer to `{init,cleanup}_module()`

[Gary Guo]

On Thu, 25 Jul 2024 20:33:18 +0200
Miguel Ojeda <ojeda@kernel.org> wrote:

> Add the equivalent of the `___ADDRESSABLE()` annotation in the
> `module_{init,exit}` macros to the Rust `module!` macro.
> 
> Without this, `objtool` would complain if enabled for Rust (under IBT
> builds), e.g.:
> 
>     samples/rust/rust_print.o: warning: objtool: cleanup_module(): not an indirect call target
>     samples/rust/rust_print.o: warning: objtool: init_module(): not an indirect call target
> 
> Tested-by: Alice Ryhl <aliceryhl@google.com>
> Tested-by: Benno Lossin <benno.lossin@proton.me>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  rust/macros/module.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)

[PATCH v3 2/6] x86/rust: support MITIGATION_RETPOLINE

[Miguel Ojeda]

Support `MITIGATION_RETPOLINE` by enabling the target features that
Clang does.

The existing target feature being enabled was a leftover from
our old `rust` branch, and it is not enough: the target feature
`retpoline-external-thunk` only implies `retpoline-indirect-calls`, but
not `retpoline-indirect-branches` (see LLVM's `X86.td`), unlike Clang's
flag of the same name `-mretpoline-external-thunk` which does imply both
(see Clang's `lib/Driver/ToolChains/Arch/X86.cpp`).

Without this, `objtool` would complain if enabled for Rust, e.g.:

    rust/core.o: warning: objtool:
    _R...escape_default+0x13: indirect jump found in RETPOLINE build

In addition, change the comment to note that LLVM is the one disabling
jump tables when retpoline is enabled, thus we do not need to use
`-Zno-jump-tables` for Rust here -- see commit c58f2166ab39 ("Introduce
the "retpoline" x86 mitigation technique ...") [1]:

    The goal is simple: avoid generating code which contains an indirect
    branch that could have its prediction poisoned by an attacker. In
    many cases, the compiler can simply use directed conditional
    branches and a small search tree. LLVM already has support for
    lowering switches in this way and the first step of this patch is
    to disable jump-table lowering of switches and introduce a pass to
    rewrite explicit indirectbr sequences into a switch over integers.

As well as a live example at [2].

These should be eventually enabled via `-Ctarget-feature` when `rustc`
starts recognizing them (or via a new dedicated flag) [3].

Cc: Daniel Borkmann <daniel@iogearbox.net>
Link: https://github.com/llvm/llvm-project/commit/c58f2166ab3987f37cb0d7815b561bff5a20a69a [1]
Link: https://godbolt.org/z/G4YPr58qG [2]
Link: https://github.com/rust-lang/rust/issues/116852 [3]
Reviewed-by: Gary Guo <gary@garyguo.net>
Tested-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 arch/x86/Makefile               | 2 +-
 scripts/generate_rust_target.rs | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 801fd85c3ef6..e8214bff1aeb 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -220,7 +220,7 @@ ifdef CONFIG_MITIGATION_RETPOLINE
   KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
   # Additionally, avoid generating expensive indirect jumps which
   # are subject to retpolines for small number of switch cases.
-  # clang turns off jump table generation by default when under
+  # LLVM turns off jump table generation by default when under
   # retpoline builds, however, gcc does not for x86. This has
   # only been fixed starting from gcc stable version 8.4.0 and
   # onwards, but not for older ones. See gcc bug #86952.
diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs
index 641b713a033a..44952f0a3aac 100644
--- a/scripts/generate_rust_target.rs
+++ b/scripts/generate_rust_target.rs
@@ -164,7 +164,14 @@ fn main() {
         );
         let mut features = "-3dnow,-3dnowa,-mmx,+soft-float".to_string();
         if cfg.has("MITIGATION_RETPOLINE") {
+            // The kernel uses `-mretpoline-external-thunk` (for Clang), which Clang maps to the
+            // target feature of the same name plus the other two target features in
+            // `clang/lib/Driver/ToolChains/Arch/X86.cpp`. These should be eventually enabled via
+            // `-Ctarget-feature` when `rustc` starts recognizing them (or via a new dedicated
+            // flag); see https://github.com/rust-lang/rust/issues/116852.
             features += ",+retpoline-external-thunk";
+            features += ",+retpoline-indirect-branches";
+            features += ",+retpoline-indirect-calls";
         }
         ts.push("features", features);
         ts.push("llvm-target", "x86_64-linux-gnu");
-- 
2.45.2

[PATCH v3 3/6] x86/rust: support MITIGATION_RETHUNK

[Miguel Ojeda]

The Rust compiler added support for `-Zfunction-return=thunk-extern` [1]
in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`.
Thus add support for `MITIGATION_RETHUNK`.

Without this, `objtool` would warn if enabled for Rust and already warns
under IBT builds, e.g.:

    samples/rust/rust_print.o: warning: objtool:
    _R...init+0xa5c: 'naked' return found in RETHUNK build

Link: https://github.com/rust-lang/rust/issues/116853 [1]
Link: https://github.com/rust-lang/rust/pull/116892 [2]
Reviewed-by: Gary Guo <gary@garyguo.net>
Tested-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 arch/x86/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index e8214bff1aeb..a1883a30a5d8 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -24,11 +24,15 @@ RETPOLINE_CFLAGS	+= $(call cc-option,-mindirect-branch-cs-prefix)
 
 ifdef CONFIG_MITIGATION_RETHUNK
 RETHUNK_CFLAGS		:= -mfunction-return=thunk-extern
+RETHUNK_RUSTFLAGS	:= -Zfunction-return=thunk-extern
 RETPOLINE_CFLAGS	+= $(RETHUNK_CFLAGS)
+RETPOLINE_RUSTFLAGS	+= $(RETHUNK_RUSTFLAGS)
 endif
 
 export RETHUNK_CFLAGS
+export RETHUNK_RUSTFLAGS
 export RETPOLINE_CFLAGS
+export RETPOLINE_RUSTFLAGS
 export RETPOLINE_VDSO_CFLAGS
 
 # For gcc stack alignment is specified with -mpreferred-stack-boundary,
@@ -218,6 +222,7 @@ KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 # Avoid indirect branches in kernel to deal with Spectre
 ifdef CONFIG_MITIGATION_RETPOLINE
   KBUILD_CFLAGS += $(RETPOLINE_CFLAGS)
+  KBUILD_RUSTFLAGS += $(RETPOLINE_RUSTFLAGS)
   # Additionally, avoid generating expensive indirect jumps which
   # are subject to retpolines for small number of switch cases.
   # LLVM turns off jump table generation by default when under
-- 
2.45.2

Re: [PATCH v3 3/6] x86/rust: support MITIGATION_RETHUNK

[Thomas Gleixner]

On Thu, Jul 25 2024 at 20:33, Miguel Ojeda wrote:
> The Rust compiler added support for `-Zfunction-return=thunk-extern` [1]
> in 1.76.0 [2], i.e. the equivalent of `-mfunction-return=thunk-extern`.
> Thus add support for `MITIGATION_RETHUNK`.
>
> Without this, `objtool` would warn if enabled for Rust and already warns
> under IBT builds, e.g.:
>
>     samples/rust/rust_print.o: warning: objtool:
>     _R...init+0xa5c: 'naked' return found in RETHUNK build
>
> Link: https://github.com/rust-lang/rust/issues/116853 [1]
> Link: https://github.com/rust-lang/rust/pull/116892 [2]
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Tested-by: Alice Ryhl <aliceryhl@google.com>
> Tested-by: Benno Lossin <benno.lossin@proton.me>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

[PATCH v3 4/6] x86/rust: support MITIGATION_SLS

[Miguel Ojeda]

Support `MITIGATION_SLS` by enabling the target features that Clang does.

Without this, `objtool` would complain if enabled for Rust, e.g.:

    rust/core.o: warning: objtool:
    _R...next_up+0x44: missing int3 after ret

These should be eventually enabled via `-Ctarget-feature` when `rustc`
starts recognizing them (or via a new dedicated flag) [1].

Link: https://github.com/rust-lang/rust/issues/116851 [1]
Reviewed-by: Gary Guo <gary@garyguo.net>
Tested-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 scripts/generate_rust_target.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/generate_rust_target.rs b/scripts/generate_rust_target.rs
index 44952f0a3aac..ba1bd455e160 100644
--- a/scripts/generate_rust_target.rs
+++ b/scripts/generate_rust_target.rs
@@ -173,6 +173,14 @@ fn main() {
             features += ",+retpoline-indirect-branches";
             features += ",+retpoline-indirect-calls";
         }
+        if cfg.has("MITIGATION_SLS") {
+            // The kernel uses `-mharden-sls=all`, which Clang maps to both these target features in
+            // `clang/lib/Driver/ToolChains/Arch/X86.cpp`. These should be eventually enabled via
+            // `-Ctarget-feature` when `rustc` starts recognizing them (or via a new dedicated
+            // flag); see https://github.com/rust-lang/rust/issues/116851.
+            features += ",+harden-sls-ijmp";
+            features += ",+harden-sls-ret";
+        }
         ts.push("features", features);
         ts.push("llvm-target", "x86_64-linux-gnu");
         ts.push("target-pointer-width", "64");
-- 
2.45.2

[PATCH v3 5/6] objtool/rust: list `noreturn` Rust functions

[Miguel Ojeda]

Rust functions may be `noreturn` (i.e. diverging) by returning the
"never" type, `!`, e.g.

    fn f() -> ! {
        loop {}
    }

Thus list the known `noreturn` functions to avoid such warnings.

Without this, `objtool` would complain if enabled for Rust, e.g.:

    rust/core.o: warning: objtool:
    _R...9panic_fmt() falls through to next function _R...18panic_nounwind_fmt()

    rust/alloc.o: warning: objtool:
    .text: unexpected end of section

In order to do so, we cannot match symbols' names exactly, for two
reasons:

  - Rust mangling scheme [1] contains disambiguators [2] which we
    cannot predict (e.g. they may vary depending on the compiler version).

    One possibility to solve this would be to parse v0 and ignore/zero
    those before comparison.

  - Some of the diverging functions come from `core`, i.e. the Rust
    standard library, which may change with each compiler version
    since they are implementation details (e.g. `panic_internals`).

Thus, to workaround both issues, only part of the symbols are matched,
instead of using the `NORETURN` macro in `noreturns.h`.

Ideally, just like for the C side, we should have a better solution. For
instance, the compiler could give us the list via something like:

    $ rustc --emit=noreturns ...

Link: https://rust-lang.github.io/rfcs/2603-rust-symbol-name-mangling-v0.html [1]
Link: https://doc.rust-lang.org/rustc/symbol-mangling/v0.html#disambiguator [2]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 tools/objtool/check.c     | 48 ++++++++++++++++++++++++++++++++++++++-
 tools/objtool/noreturns.h |  2 ++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0a33d9195b7a..deace6fca2ed 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -177,6 +177,48 @@ static bool is_sibling_call(struct instruction *insn)
 	return (is_static_jump(insn) && insn_call_dest(insn));
 }
 
+/*
+ * Checks if a string ends with another.
+ */
+static bool str_ends_with(const char *s, const char *sub)
+{
+	const int slen = strlen(s);
+	const int sublen = strlen(sub);
+
+	if (sublen > slen)
+		return 0;
+
+	return !memcmp(s + slen - sublen, sub, sublen);
+}
+
+/*
+ * Checks if a function is a Rust "noreturn" one.
+ */
+static bool is_rust_noreturn(const struct symbol *func)
+{
+	/*
+	 * If it does not start with "_R", then it is not a Rust symbol.
+	 */
+	if (strncmp(func->name, "_R", 2))
+		return false;
+
+	/*
+	 * These are just heuristics -- we do not control the precise symbol
+	 * name, due to the crate disambiguators (which depend on the compiler)
+	 * as well as changes to the source code itself between versions (since
+	 * these come from the Rust standard library).
+	 */
+	return str_ends_with(func->name, "_4core6option13unwrap_failed")			||
+	       str_ends_with(func->name, "_4core6result13unwrap_failed")			||
+	       str_ends_with(func->name, "_4core9panicking5panic")				||
+	       str_ends_with(func->name, "_4core9panicking9panic_fmt")				||
+	       str_ends_with(func->name, "_4core9panicking14panic_explicit")			||
+	       str_ends_with(func->name, "_4core9panicking18panic_bounds_check")		||
+	       strstr(func->name, "_4core9panicking11panic_const24panic_const_")		||
+	       (strstr(func->name, "_4core5slice5index24slice_") &&
+		str_ends_with(func->name, "_fail"));
+}
+
 /*
  * This checks to see if the given function is a "noreturn" function.
  *
@@ -202,10 +244,14 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
 	if (!func)
 		return false;
 
-	if (func->bind == STB_GLOBAL || func->bind == STB_WEAK)
+	if (func->bind == STB_GLOBAL || func->bind == STB_WEAK) {
+		if (is_rust_noreturn(func))
+			return true;
+
 		for (i = 0; i < ARRAY_SIZE(global_noreturns); i++)
 			if (!strcmp(func->name, global_noreturns[i]))
 				return true;
+	}
 
 	if (func->bind == STB_WEAK)
 		return false;
diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
index 7ebf29c91184..82a001ac433b 100644
--- a/tools/objtool/noreturns.h
+++ b/tools/objtool/noreturns.h
@@ -35,6 +35,8 @@ NORETURN(panic)
 NORETURN(panic_smp_self_stop)
 NORETURN(rest_init)
 NORETURN(rewind_stack_and_make_dead)
+NORETURN(rust_begin_unwind)
+NORETURN(rust_helper_BUG)
 NORETURN(sev_es_terminate)
 NORETURN(snp_abort)
 NORETURN(start_kernel)
-- 
2.45.2

Re: [PATCH v3 5/6] objtool/rust: list `noreturn` Rust functions

[Peter Zijlstra]

On Thu, Jul 25, 2024 at 08:33:22PM +0200, Miguel Ojeda wrote:
> Rust functions may be `noreturn` (i.e. diverging) by returning the
> "never" type, `!`, e.g.
> 
>     fn f() -> ! {
>         loop {}
>     }
> 
> Thus list the known `noreturn` functions to avoid such warnings.
> 
> Without this, `objtool` would complain if enabled for Rust, e.g.:
> 
>     rust/core.o: warning: objtool:
>     _R...9panic_fmt() falls through to next function _R...18panic_nounwind_fmt()
> 
>     rust/alloc.o: warning: objtool:
>     .text: unexpected end of section
> 
> In order to do so, we cannot match symbols' names exactly, for two
> reasons:
> 
>   - Rust mangling scheme [1] contains disambiguators [2] which we
>     cannot predict (e.g. they may vary depending on the compiler version).
> 
>     One possibility to solve this would be to parse v0 and ignore/zero
>     those before comparison.
> 
>   - Some of the diverging functions come from `core`, i.e. the Rust
>     standard library, which may change with each compiler version
>     since they are implementation details (e.g. `panic_internals`).
> 
> Thus, to workaround both issues, only part of the symbols are matched,
> instead of using the `NORETURN` macro in `noreturns.h`.
> 
> Ideally, just like for the C side, we should have a better solution. For
> instance, the compiler could give us the list via something like:
> 
>     $ rustc --emit=noreturns ...
> 
> Link: https://rust-lang.github.io/rfcs/2603-rust-symbol-name-mangling-v0.html [1]
> Link: https://doc.rust-lang.org/rustc/symbol-mangling/v0.html#disambiguator [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

> ---
>  tools/objtool/check.c     | 48 ++++++++++++++++++++++++++++++++++++++-
>  tools/objtool/noreturns.h |  2 ++
>  2 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 0a33d9195b7a..deace6fca2ed 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -177,6 +177,48 @@ static bool is_sibling_call(struct instruction *insn)
>  	return (is_static_jump(insn) && insn_call_dest(insn));
>  }
>  
> +/*
> + * Checks if a string ends with another.
> + */
> +static bool str_ends_with(const char *s, const char *sub)
> +{
> +	const int slen = strlen(s);
> +	const int sublen = strlen(sub);
> +
> +	if (sublen > slen)
> +		return 0;
> +
> +	return !memcmp(s + slen - sublen, sub, sublen);
> +}
> +
> +/*
> + * Checks if a function is a Rust "noreturn" one.
> + */
> +static bool is_rust_noreturn(const struct symbol *func)
> +{
> +	/*
> +	 * If it does not start with "_R", then it is not a Rust symbol.
> +	 */
> +	if (strncmp(func->name, "_R", 2))
> +		return false;
> +
> +	/*
> +	 * These are just heuristics -- we do not control the precise symbol
> +	 * name, due to the crate disambiguators (which depend on the compiler)
> +	 * as well as changes to the source code itself between versions (since
> +	 * these come from the Rust standard library).
> +	 */
> +	return str_ends_with(func->name, "_4core6option13unwrap_failed")			||
> +	       str_ends_with(func->name, "_4core6result13unwrap_failed")			||
> +	       str_ends_with(func->name, "_4core9panicking5panic")				||
> +	       str_ends_with(func->name, "_4core9panicking9panic_fmt")				||
> +	       str_ends_with(func->name, "_4core9panicking14panic_explicit")			||
> +	       str_ends_with(func->name, "_4core9panicking18panic_bounds_check")		||
> +	       strstr(func->name, "_4core9panicking11panic_const24panic_const_")		||
> +	       (strstr(func->name, "_4core5slice5index24slice_") &&
> +		str_ends_with(func->name, "_fail"));
> +}
> +
>  /*
>   * This checks to see if the given function is a "noreturn" function.
>   *
> @@ -202,10 +244,14 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
>  	if (!func)
>  		return false;
>  
> -	if (func->bind == STB_GLOBAL || func->bind == STB_WEAK)
> +	if (func->bind == STB_GLOBAL || func->bind == STB_WEAK) {
> +		if (is_rust_noreturn(func))
> +			return true;
> +
>  		for (i = 0; i < ARRAY_SIZE(global_noreturns); i++)
>  			if (!strcmp(func->name, global_noreturns[i]))
>  				return true;
> +	}
>  
>  	if (func->bind == STB_WEAK)
>  		return false;
> diff --git a/tools/objtool/noreturns.h b/tools/objtool/noreturns.h
> index 7ebf29c91184..82a001ac433b 100644
> --- a/tools/objtool/noreturns.h
> +++ b/tools/objtool/noreturns.h
> @@ -35,6 +35,8 @@ NORETURN(panic)
>  NORETURN(panic_smp_self_stop)
>  NORETURN(rest_init)
>  NORETURN(rewind_stack_and_make_dead)
> +NORETURN(rust_begin_unwind)
> +NORETURN(rust_helper_BUG)
>  NORETURN(sev_es_terminate)
>  NORETURN(snp_abort)
>  NORETURN(start_kernel)
> -- 
> 2.45.2
> 

Re: [PATCH v3 5/6] objtool/rust: list `noreturn` Rust functions

[Alice Ryhl]

On Thu, Jul 25, 2024 at 8:35 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Rust functions may be `noreturn` (i.e. diverging) by returning the
> "never" type, `!`, e.g.
>
>     fn f() -> ! {
>         loop {}
>     }
>
> Thus list the known `noreturn` functions to avoid such warnings.
>
> Without this, `objtool` would complain if enabled for Rust, e.g.:
>
>     rust/core.o: warning: objtool:
>     _R...9panic_fmt() falls through to next function _R...18panic_nounwind_fmt()
>
>     rust/alloc.o: warning: objtool:
>     .text: unexpected end of section
>
> In order to do so, we cannot match symbols' names exactly, for two
> reasons:
>
>   - Rust mangling scheme [1] contains disambiguators [2] which we
>     cannot predict (e.g. they may vary depending on the compiler version).
>
>     One possibility to solve this would be to parse v0 and ignore/zero
>     those before comparison.
>
>   - Some of the diverging functions come from `core`, i.e. the Rust
>     standard library, which may change with each compiler version
>     since they are implementation details (e.g. `panic_internals`).
>
> Thus, to workaround both issues, only part of the symbols are matched,
> instead of using the `NORETURN` macro in `noreturns.h`.
>
> Ideally, just like for the C side, we should have a better solution. For
> instance, the compiler could give us the list via something like:
>
>     $ rustc --emit=noreturns ...
>
> Link: https://rust-lang.github.io/rfcs/2603-rust-symbol-name-mangling-v0.html [1]
> Link: https://doc.rust-lang.org/rustc/symbol-mangling/v0.html#disambiguator [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Tested-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v3 5/6] objtool/rust: list `noreturn` Rust functions

[Kees Cook]

On Thu, Jul 25, 2024 at 08:33:22PM +0200, Miguel Ojeda wrote:
> Rust functions may be `noreturn` (i.e. diverging) by returning the
> "never" type, `!`, e.g.
> 
>     fn f() -> ! {
>         loop {}
>     }
> 
> Thus list the known `noreturn` functions to avoid such warnings.
> 
> Without this, `objtool` would complain if enabled for Rust, e.g.:
> 
>     rust/core.o: warning: objtool:
>     _R...9panic_fmt() falls through to next function _R...18panic_nounwind_fmt()
> 
>     rust/alloc.o: warning: objtool:
>     .text: unexpected end of section
> 
> In order to do so, we cannot match symbols' names exactly, for two
> reasons:
> 
>   - Rust mangling scheme [1] contains disambiguators [2] which we
>     cannot predict (e.g. they may vary depending on the compiler version).
> 
>     One possibility to solve this would be to parse v0 and ignore/zero
>     those before comparison.
> 
>   - Some of the diverging functions come from `core`, i.e. the Rust
>     standard library, which may change with each compiler version
>     since they are implementation details (e.g. `panic_internals`).
> 
> Thus, to workaround both issues, only part of the symbols are matched,
> instead of using the `NORETURN` macro in `noreturns.h`.
> 
> Ideally, just like for the C side, we should have a better solution. For
> instance, the compiler could give us the list via something like:
> 
>     $ rustc --emit=noreturns ...

Yeah, having added noreturns to objtool myself a few times, it'd be nice
to have a way to make these manual lists go away some day.

> 
> Link: https://rust-lang.github.io/rfcs/2603-rust-symbol-name-mangling-v0.html [1]
> Link: https://doc.rust-lang.org/rustc/symbol-mangling/v0.html#disambiguator [2]
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

Re: [PATCH v3 5/6] objtool/rust: list `noreturn` Rust functions

[Peter Zijlstra]

On Tue, Aug 06, 2024 at 12:42:41PM -0700, Kees Cook wrote:
> On Thu, Jul 25, 2024 at 08:33:22PM +0200, Miguel Ojeda wrote:
> > Rust functions may be `noreturn` (i.e. diverging) by returning the
> > "never" type, `!`, e.g.
> > 
> >     fn f() -> ! {
> >         loop {}
> >     }
> > 
> > Thus list the known `noreturn` functions to avoid such warnings.
> > 
> > Without this, `objtool` would complain if enabled for Rust, e.g.:
> > 
> >     rust/core.o: warning: objtool:
> >     _R...9panic_fmt() falls through to next function _R...18panic_nounwind_fmt()
> > 
> >     rust/alloc.o: warning: objtool:
> >     .text: unexpected end of section
> > 
> > In order to do so, we cannot match symbols' names exactly, for two
> > reasons:
> > 
> >   - Rust mangling scheme [1] contains disambiguators [2] which we
> >     cannot predict (e.g. they may vary depending on the compiler version).
> > 
> >     One possibility to solve this would be to parse v0 and ignore/zero
> >     those before comparison.
> > 
> >   - Some of the diverging functions come from `core`, i.e. the Rust
> >     standard library, which may change with each compiler version
> >     since they are implementation details (e.g. `panic_internals`).
> > 
> > Thus, to workaround both issues, only part of the symbols are matched,
> > instead of using the `NORETURN` macro in `noreturns.h`.
> > 
> > Ideally, just like for the C side, we should have a better solution. For
> > instance, the compiler could give us the list via something like:
> > 
> >     $ rustc --emit=noreturns ...
> 
> Yeah, having added noreturns to objtool myself a few times, it'd be nice
> to have a way to make these manual lists go away some day.

So it would be fairly simple to make objtool consume a magic section
emitted by the compiler.. I think we've asked the compiler folks for
that at some point even, but I don't have clear recollections.

Re: [PATCH v3 5/6] objtool/rust: list `noreturn` Rust functions

[Miguel Ojeda]

On Tue, Aug 6, 2024 at 10:22 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> So it would be fairly simple to make objtool consume a magic section
> emitted by the compiler.. I think we've asked the compiler folks for
> that at some point even, but I don't have clear recollections.

The section sounds like a good approach -- we will ask the Rust team
about it. Then perhaps we can get Clang/GCC to implement something
similar too -- for this sort of thing we can use the shorter cycles of
`rustc` (and their unstable features concept too) to experiment with
these things :)

I have also added it to our `rustc` sublist of things we need.

Cheers,
Miguel

[PATCH v3 6/6] objtool/kbuild/rust: enable objtool for Rust

[Miguel Ojeda]

Now that we should be `objtool`-warning free, enable `objtool` for
Rust too.

Before this patch series, we were already getting warnings under e.g. IBT
builds, since those would see Rust code via `vmlinux.o`.

Tested-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/Makefile          | 22 ++++++++++++++--------
 scripts/Makefile.build |  9 +++++++--
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/rust/Makefile b/rust/Makefile
index bf05e65365da..1756238b641d 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -344,7 +344,8 @@ quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L
 		--crate-type rlib -L$(objtree)/$(obj) \
 		--crate-name $(patsubst %.o,%,$(notdir $@)) $< \
 		--sysroot=/dev/null \
-	$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@)
+	$(if $(rustc_objcopy),;$(OBJCOPY) $(rustc_objcopy) $@) \
+	$(cmd_objtool)
 
 rust-analyzer:
 	$(Q)$(srctree)/scripts/generate_rust_analyzer.py \
@@ -366,44 +367,49 @@ ifneq ($(or $(CONFIG_ARM64),$(and $(CONFIG_RISCV),$(CONFIG_64BIT))),)
 		__ashlti3 __lshrti3
 endif
 
+define rule_rustc_library
+	$(call cmd_and_fixdep,rustc_library)
+	$(call cmd,gen_objtooldep)
+endef
+
 $(obj)/core.o: private skip_clippy = 1
 $(obj)/core.o: private skip_flags = -Wunreachable_pub
 $(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 FORCE
-	+$(call if_changed_dep,rustc_library)
+	+$(call if_changed_rule,rustc_library)
 ifdef CONFIG_X86_64
 $(obj)/core.o: scripts/target.json
 endif
 
 $(obj)/compiler_builtins.o: private rustc_objcopy = -w -W '__*'
 $(obj)/compiler_builtins.o: $(src)/compiler_builtins.rs $(obj)/core.o FORCE
-	+$(call if_changed_dep,rustc_library)
+	+$(call if_changed_rule,rustc_library)
 
 $(obj)/alloc.o: private skip_clippy = 1
 $(obj)/alloc.o: private skip_flags = -Wunreachable_pub
 $(obj)/alloc.o: private rustc_target_flags = $(alloc-cfgs)
 $(obj)/alloc.o: $(RUST_LIB_SRC)/alloc/src/lib.rs $(obj)/compiler_builtins.o FORCE
-	+$(call if_changed_dep,rustc_library)
+	+$(call if_changed_rule,rustc_library)
 
 $(obj)/build_error.o: $(src)/build_error.rs $(obj)/compiler_builtins.o FORCE
-	+$(call if_changed_dep,rustc_library)
+	+$(call if_changed_rule,rustc_library)
 
 $(obj)/bindings.o: $(src)/bindings/lib.rs \
     $(obj)/compiler_builtins.o \
     $(obj)/bindings/bindings_generated.rs \
     $(obj)/bindings/bindings_helpers_generated.rs FORCE
-	+$(call if_changed_dep,rustc_library)
+	+$(call if_changed_rule,rustc_library)
 
 $(obj)/uapi.o: $(src)/uapi/lib.rs \
     $(obj)/compiler_builtins.o \
     $(obj)/uapi/uapi_generated.rs FORCE
-	+$(call if_changed_dep,rustc_library)
+	+$(call if_changed_rule,rustc_library)
 
 $(obj)/kernel.o: private rustc_target_flags = --extern alloc \
     --extern build_error --extern macros --extern bindings --extern uapi
 $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/alloc.o $(obj)/build_error.o \
     $(obj)/libmacros.so $(obj)/bindings.o $(obj)/uapi.o FORCE
-	+$(call if_changed_dep,rustc_library)
+	+$(call if_changed_rule,rustc_library)
 
 endif # CONFIG_RUST
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index efacca63c897..72b1232b1f7d 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -288,10 +288,15 @@ rust_common_cmd = \
 # would not match each other.
 
 quiet_cmd_rustc_o_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
-      cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $<
+      cmd_rustc_o_rs = $(rust_common_cmd) --emit=obj=$@ $< $(cmd_objtool)
+
+define rule_rustc_o_rs
+	$(call cmd_and_fixdep,rustc_o_rs)
+	$(call cmd,gen_objtooldep)
+endef
 
 $(obj)/%.o: $(obj)/%.rs FORCE
-	+$(call if_changed_dep,rustc_o_rs)
+	+$(call if_changed_rule,rustc_o_rs)
 
 quiet_cmd_rustc_rsi_rs = $(RUSTC_OR_CLIPPY_QUIET) $(quiet_modtag) $@
       cmd_rustc_rsi_rs = \
-- 
2.45.2

Re: [PATCH v3 0/6] Rust: support `CPU_MITIGATIONS` and enable `objtool`

[Benno Lossin]

On 25.07.24 20:33, Miguel Ojeda wrote:
> Hi,
> 
> This is just v2 with the helper function suggested by Peter.
> 
> I dropped Benno's and Alice's Tested-bys from the modified patch, just
> in case, but the logic should be equivalent.

I re-ran my tests and the results are the same as with v1.

Tested-by: Benno Lossin <benno.lossin@proton.me>

---
Cheers,
Benno

> Cheers,
> Miguel
> 
> v3:
>   - Added `is_rust_noreturn()` helper function (Peter).
>   - Reworded a couple bits.
> 
> v2: https://lore.kernel.org/rust-for-linux/20240724161501.1319115-1-ojeda@kernel.org/
> v1: https://lore.kernel.org/rust-for-linux/20231023174449.251550-1-ojeda@kernel.org/
> 
> Miguel Ojeda (6):
>   rust: module: add static pointer to `{init,cleanup}_module()`
>   x86/rust: support MITIGATION_RETPOLINE
>   x86/rust: support MITIGATION_RETHUNK
>   x86/rust: support MITIGATION_SLS
>   objtool/rust: list `noreturn` Rust functions
>   objtool/kbuild/rust: enable objtool for Rust
> 
>  arch/x86/Makefile               |  7 ++++-
>  rust/Makefile                   | 22 +++++++++------
>  rust/macros/module.rs           | 12 +++++++++
>  scripts/Makefile.build          |  9 +++++--
>  scripts/generate_rust_target.rs | 15 +++++++++++
>  tools/objtool/check.c           | 48 ++++++++++++++++++++++++++++++++-
>  tools/objtool/noreturns.h       |  2 ++
>  7 files changed, 103 insertions(+), 12 deletions(-)
> 
> 
> base-commit: b1263411112305acf2af728728591465becb45b0
> --
> 2.45.2

Re: [PATCH v3 0/6] Rust: support `CPU_MITIGATIONS` and enable `objtool`

[Miguel Ojeda]

On Thu, Jul 25, 2024 at 8:35 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> Miguel Ojeda (6):
>   rust: module: add static pointer to `{init,cleanup}_module()`
>   x86/rust: support MITIGATION_RETPOLINE
>   x86/rust: support MITIGATION_RETHUNK
>   x86/rust: support MITIGATION_SLS
>   objtool/rust: list `noreturn` Rust functions
>   objtool/kbuild/rust: enable objtool for Rust
>
>  arch/x86/Makefile               |  7 ++++-
>  rust/Makefile                   | 22 +++++++++------
>  rust/macros/module.rs           | 12 +++++++++
>  scripts/Makefile.build          |  9 +++++--
>  scripts/generate_rust_target.rs | 15 +++++++++++
>  tools/objtool/check.c           | 48 ++++++++++++++++++++++++++++++++-
>  tools/objtool/noreturns.h       |  2 ++
>  7 files changed, 103 insertions(+), 12 deletions(-)

Applied to `rust-next` -- thanks everyone!

In the `noreturn` patch:

    [ Kees agrees this should be automated and Peter says:

        So it would be fairly simple to make objtool consume a magic section
        emitted by the compiler.. I think we've asked the compiler folks
        for that at some point even, but I don't have clear recollections.

      We will ask upstream Rust about it. And if they agree, then perhaps
      we can get Clang/GCC to implement something similar too -- for this
      sort of thing we can take advantage of the shorter cycles of `rustc`
      as well as their unstable features concept to experiment.

      Gary proposed using DWARF (though it would need to be available), and
      wrote a proof of concept script using the `object` and `gimli` crates:
      https://gist.github.com/nbdd0121/449692570622c2f46a29ad9f47c3379a

        - Miguel ]

    [ Added `len_mismatch_fail` symbol for new `kernel` crate code merged
      since then as well as 3 more `core::panicking` symbols that appear
      in `RUST_DEBUG_ASSERTIONS=y` builds.  - Miguel ]

Cheers,
Miguel