From: Conor Dooley conor.dooley@microchip.com
On RISC-V and arm64, and presumably x86, if CFI_CLANG is enabled, loading a rust module will trigger a kernel panic. Support for sanitisers, including kcfi (CFI_CLANG), is in the works, but for now they're nightly-only options in rustc. Make RUST depend on !CFI_CLANG to prevent configuring a kernel without symmetrical support for kfi.
Fixes: 2f7ab1267dc9 ("Kbuild: add Rust support") cc: stable@vger.kernel.org Signed-off-by: Conor Dooley conor.dooley@microchip.com --- Sending this one on its own, there's no explicit dep on this for the riscv enabling patch, v3 to continue the numbering from there. Nothing has changed since v2.
CC: Miguel Ojeda ojeda@kernel.org CC: Alex Gaynor alex.gaynor@gmail.com CC: Wedson Almeida Filho wedsonaf@gmail.com CC: linux-kernel@vger.kernel.org (open list) CC: rust-for-linux@vger.kernel.org CC: Sami Tolvanen samitolvanen@google.com CC: Kees Cook keescook@chromium.org CC: Nathan Chancellor nathan@kernel.org CC: llvm@lists.linux.dev --- init/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/init/Kconfig b/init/Kconfig index aa02aec6aa7d..ad9a2da27dc9 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1899,6 +1899,7 @@ config RUST bool "Rust support" depends on HAVE_RUST depends on RUST_IS_AVAILABLE + depends on !CFI_CLANG depends on !MODVERSIONS depends on !GCC_PLUGINS depends on !RANDSTRUCT
On Thu, Apr 4, 2024 at 4:17 PM Conor Dooley conor@kernel.org wrote:
From: Conor Dooley conor.dooley@microchip.com
On RISC-V and arm64, and presumably x86, if CFI_CLANG is enabled, loading a rust module will trigger a kernel panic. Support for sanitisers, including kcfi (CFI_CLANG), is in the works, but for now they're nightly-only options in rustc. Make RUST depend on !CFI_CLANG to prevent configuring a kernel without symmetrical support for kfi.
Fixes: 2f7ab1267dc9 ("Kbuild: add Rust support") cc: stable@vger.kernel.org Signed-off-by: Conor Dooley conor.dooley@microchip.com
Sending this one on its own, there's no explicit dep on this for the riscv enabling patch, v3 to continue the numbering from there. Nothing has changed since v2.
CC: Miguel Ojeda ojeda@kernel.org CC: Alex Gaynor alex.gaynor@gmail.com CC: Wedson Almeida Filho wedsonaf@gmail.com CC: linux-kernel@vger.kernel.org (open list) CC: rust-for-linux@vger.kernel.org CC: Sami Tolvanen samitolvanen@google.com CC: Kees Cook keescook@chromium.org CC: Nathan Chancellor nathan@kernel.org CC: llvm@lists.linux.dev
Cc'ing Matthew & Ramon as well so that they are aware and in case they want to comment.
Cheers, Miguel
Cc'ing Matthew & Ramon as well so that they are aware and in case they want to comment.
Cheers, Miguel
This patch is fine by me - the last patch needed for KCFI to be functional in Rust just landed upstream last night, so we should revisit this (in the form of enabling it) once we move to `rustc-1.79.0` or later. In case anyone wants it for local experimentation, I have a backport branch [1] which applies these to the 1.73.0 compiler and enables it in the kernel [2] (not upstreamed because the feature isn't yet in kernel's `rustc`), which Android will be using for the Rust binder driver. This patch will require a recent (last year or so) clang, as it relies on `-fsanitize-cfi-icall-experimental-normalize-integers`.
[1]: https://github.com/maurer/rust/tree/1.73.0%2Bcfi [2]: https://android-review.git.corp.google.com/c/kernel/common/+/2930616
On Thu, Apr 04, 2024 at 08:25:11AM -0700, Matthew Maurer wrote:
Cc'ing Matthew & Ramon as well so that they are aware and in case they want to comment.
Ah crap, I knew I forgot to CC someone from the last conversation, thanks Miguel.
This patch is fine by me - the last patch needed for KCFI to be functional in Rust just landed upstream last night, so we should revisit this (in the form of enabling it) once we move to `rustc-1.79.0` or later.
That's great news, thanks for working on it :)
On Thu, Apr 4, 2024 at 5:25 PM Matthew Maurer mmaurer@google.com wrote:
This patch is fine by me - the last patch needed for KCFI to be functional in Rust just landed upstream last night, so we should revisit this (in the form of enabling it) once we move to `rustc-1.79.0` or later. In case anyone wants it for local experimentation, I have a backport branch [1] which applies these to the 1.73.0 compiler and enables it in the kernel [2] (not upstreamed because the feature isn't yet in kernel's `rustc`), which Android will be using for the Rust binder driver. This patch will require a recent (last year or so) clang, as it relies on `-fsanitize-cfi-icall-experimental-normalize-integers`.
Thanks for the update, Matthew!
I guess the public link is: https://android-review.googlesource.com/c/kernel/common/+/2930616
Cheers, Miguel
On Thu, Apr 04, 2024 at 03:17:02PM +0100, Conor Dooley wrote:
From: Conor Dooley conor.dooley@microchip.com
On RISC-V and arm64, and presumably x86, if CFI_CLANG is enabled, loading a rust module will trigger a kernel panic. Support for sanitisers, including kcfi (CFI_CLANG), is in the works, but for now they're nightly-only options in rustc. Make RUST depend on !CFI_CLANG to prevent configuring a kernel without symmetrical support for kfi.
Fixes: 2f7ab1267dc9 ("Kbuild: add Rust support") cc: stable@vger.kernel.org Signed-off-by: Conor Dooley conor.dooley@microchip.com
Acked-by: Nathan Chancellor nathan@kernel.org
It seems like this won't be forgotten about but if there is not already an issue open for this somewhere, it would be good to have one, since we obviously want this for both C and Rust code.
As a general meta comment not directed at anyone in particualr, I think these 'depends on !' should all have some sort of comment or description as to why they are disabled. I can infer from most of them but it would still be good to be explicit, especially since someone might want to work on fixing the ones that are due to missing support and such.
Sending this one on its own, there's no explicit dep on this for the riscv enabling patch, v3 to continue the numbering from there. Nothing has changed since v2.
CC: Miguel Ojeda ojeda@kernel.org CC: Alex Gaynor alex.gaynor@gmail.com CC: Wedson Almeida Filho wedsonaf@gmail.com CC: linux-kernel@vger.kernel.org (open list) CC: rust-for-linux@vger.kernel.org CC: Sami Tolvanen samitolvanen@google.com CC: Kees Cook keescook@chromium.org CC: Nathan Chancellor nathan@kernel.org CC: llvm@lists.linux.dev
init/Kconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/init/Kconfig b/init/Kconfig index aa02aec6aa7d..ad9a2da27dc9 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -1899,6 +1899,7 @@ config RUST bool "Rust support" depends on HAVE_RUST depends on RUST_IS_AVAILABLE
- depends on !CFI_CLANG depends on !MODVERSIONS depends on !GCC_PLUGINS depends on !RANDSTRUCT
-- 2.43.0
On Thu, Apr 4, 2024 at 5:33 PM Nathan Chancellor nathan@kernel.org wrote:
Acked-by: Nathan Chancellor nathan@kernel.org
Thanks!
It seems like this won't be forgotten about but if there is not already an issue open for this somewhere, it would be good to have one, since we obviously want this for both C and Rust code.
We track the unstable feature(s) at https://github.com/Rust-for-Linux/linux/issues/2 (I just moved this one there since it is close to ready, but it was in #355 previously, and cleaned things up a bit).
On the Rust side, I think the main one is https://github.com/rust-lang/rust/issues/89653.
It includes KCFI, but it is missing Matthew's PRs for KCFI etc. I added a link to get those issues/PRs (or hopefully most of them): https://github.com/rust-lang/rust/pulls?q=is%3Apr+cfi+label%3APG-exploit-mit...
We could create an explicit/concrete issue for removing the `depends on !` when Matthew sends the patches, though.
As a general meta comment not directed at anyone in particualr, I think these 'depends on !' should all have some sort of comment or description as to why they are disabled. I can infer from most of them but it would still be good to be explicit, especially since someone might want to work on fixing the ones that are due to missing support and such.
That is definitely a good idea. An alternative could be linking an issue instead (perhaps concrete ones for that, i.e. your other idea above) so that we can update the status etc. easily.
Cheers, Miguel
We track the unstable feature(s) at https://github.com/Rust-for-Linux/linux/issues/2 (I just moved this one there since it is close to ready, but it was in #355 previously, and cleaned things up a bit).
Sorry about that, I should've done this already. In addition to the link you added above, here's a tracking issue for KCFI support for Rust: https://github.com/rust-lang/rust/issues/123479
There are still a few unresolved questions that I'd like to hear from you as you experiment with it, which the tl,dr. is: KCFI support for Rust shares most of its implementation with the CFI support, with some key differences:
KCFI performs type tests differently and are implemented as different LLVM passes than CFI to not require LTO. KCFI has the limitation that a function or method may have one type id assigned only.
Because of limitation listed above (2), the current KCFI implementation (not CFI) does reifying of types (i.e., adds shims/trampolines for indirect calls in these cases) for:
Supporting casting between function items, closures, and Fn trait objects. Supporting methods being cast as function pointers.
There may be possible costs of these added levels of indirections for KCFI for cache coherence/locality and performance, possible introduction of gadgets or KCFI bypasses, and increased artifact/binary sizes, which haven't been looked at yet, and I'm honestly not super happy about it (but it's better than the original proposal of adding shims/trampolines to every virtual call in the Rust compiler), so I'd be interested in any performance results or regressions you may have while experimenting with it, more specifically:
Linux Kernel without KCFI vs Linux Kernel with KCFI vs Linux Kernel with Rust and KCFI, possibly isolated to the code of a driver that has an implementation in C vs an alternative implementation in Rust.
On Thu, Apr 4, 2024 at 4:17 PM Conor Dooley conor@kernel.org wrote:
From: Conor Dooley conor.dooley@microchip.com
On RISC-V and arm64, and presumably x86, if CFI_CLANG is enabled, loading a rust module will trigger a kernel panic. Support for sanitisers, including kcfi (CFI_CLANG), is in the works, but for now they're nightly-only options in rustc. Make RUST depend on !CFI_CLANG to prevent configuring a kernel without symmetrical support for kfi.
Fixes: 2f7ab1267dc9 ("Kbuild: add Rust support") cc: stable@vger.kernel.org Signed-off-by: Conor Dooley conor.dooley@microchip.com
[ Matthew Maurer writes [1]:
This patch is fine by me - the last patch needed for KCFI to be functional in Rust just landed upstream last night, so we should revisit this (in the form of enabling it) once we move to `rustc-1.79.0` or later.
Ramon de C Valle also gave feedback [2] on the status of KCFI for Rust and created a tracking issue [3] in upstream Rust. - Miguel ]
[ Added feedback from the list, links, and used Cc for the tag. ]
Applied to `rust-fixes` -- thanks everyone! Please feel free to send more tags for this one.
Cheers, Miguel
linux-stable-mirror@lists.linaro.org
-
Conor Dooley -
Matthew Maurer -
Miguel Ojeda -
Nathan Chancellor -
Ramon de C Valle