From: Conor Dooley <[email protected]>
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: [email protected]
Signed-off-by: Conor Dooley <[email protected]>
---
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 <[email protected]>
CC: Alex Gaynor <[email protected]>
CC: Wedson Almeida Filho <[email protected]>
CC: [email protected] (open list)
CC: [email protected]
CC: Sami Tolvanen <[email protected]>
CC: Kees Cook <[email protected]>
CC: Nathan Chancellor <[email protected]>
CC: [email protected]
---
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 4:17 PM Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> 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: [email protected]
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> 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 <[email protected]>
> CC: Alex Gaynor <[email protected]>
> CC: Wedson Almeida Filho <[email protected]>
> CC: [email protected] (open list)
> CC: [email protected]
> CC: Sami Tolvanen <[email protected]>
> CC: Kees Cook <[email protected]>
> CC: Nathan Chancellor <[email protected]>
> CC: [email protected]
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 03:17:02PM +0100, Conor Dooley wrote:
> From: Conor Dooley <[email protected]>
>
> 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: [email protected]
> Signed-off-by: Conor Dooley <[email protected]>
Acked-by: Nathan Chancellor <[email protected]>
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 <[email protected]>
> CC: Alex Gaynor <[email protected]>
> CC: Wedson Almeida Filho <[email protected]>
> CC: [email protected] (open list)
> CC: [email protected]
> CC: Sami Tolvanen <[email protected]>
> CC: Kees Cook <[email protected]>
> CC: Nathan Chancellor <[email protected]>
> CC: [email protected]
> ---
> 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 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 <[email protected]> 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`.
>
> [1]: https://github.com/maurer/rust/tree/1.73.0%2Bcfi
> [2]: https://android-review.git.corp.google.com/c/kernel/common/+/2930616
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 4, 2024 at 5:33 PM Nathan Chancellor <[email protected]> wrote:
>
> Acked-by: Nathan Chancellor <[email protected]>
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-mitigations+-label%3Arollup
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 Fri, Apr 5, 2024 at 12:34 AM Ramon de C Valle <[email protected]> wrote:
>
> 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
No worries! Thanks for taking the time creating the new tracking issue
-- linked now from our issue #2.
I also linked your update here and noted the benchmarking needs /
regressions report request too.
Cheers,
Miguel
On Thu, Apr 4, 2024 at 4:17 PM Conor Dooley <[email protected]> wrote:
>
> From: Conor Dooley <[email protected]>
>
> 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: [email protected]
> Signed-off-by: Conor Dooley <[email protected]>
[ 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