Re: [PATCH v2 2/6] x86/rust: support MITIGATION_RETPOLINE

[Gary Guo]

On Wed, 24 Jul 2024 18:14:55 +0200
Miguel Ojeda <ojeda@xxxxxxxxxx> wrote:

> 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@xxxxxxxxxxxxx>
> 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]
> Signed-off-by: Miguel Ojeda <ojeda@xxxxxxxxxx>

Reviewed-by: Gary Guo <gary@xxxxxxxxxxx>

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