Skip to content

Commit

Permalink
cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig
Browse files Browse the repository at this point in the history
There is a bug in the LLVM implementation of KASAN and GCOV that makes
these options incompatible with the CFI_ICALL_NORMALIZE_INTEGERS option.

The bug has already been fixed in llvm/clang [1] and rustc [2]. However,
Kconfig currently has no way to gate features on the LLVM version inside
rustc, so we cannot write down a precise `depends on` clause in this
case. Instead, a `def_bool` option is defined for whether
CFI_ICALL_NORMALIZE_INTEGERS is available, and its default value is set
to false when GCOV or KASAN are turned on. End users using a patched
clang/rustc can turn on the HAVE_CFI_ICALL_NORMALIZE_INTEGERS option
directly to override this.

An alternative solution is to inspect a binary created by clang or rustc
to see whether the faulty CFI tags are in the binary. This would be a
precise check, but it would involve hard-coding the *hashed* version of
the CFI tag. This is because there's no way to get clang or rustc to
output the unhased version of the CFI tag. Relying on the precise
hashing algorithm using by CFI seems too fragile, so I have not pursued
this option. Besides, this kind of hack is exactly what lead to the LLVM
bug in the first place.

If the CFI_ICALL_NORMALIZE_INTEGERS option is used without CONFIG_RUST,
then we actually can perform a precise check today: just compare the
clang version number. This works since clang and llvm are always updated
in lockstep. However, encoding this in Kconfig would give the
HAVE_CFI_ICALL_NORMALIZE_INTEGERS option a dependency on CONFIG_RUST,
which is not possible as the reverse dependency already exists.

HAVE_CFI_ICALL_NORMALIZE_INTEGERS is defined to be a `def_bool` instead
of `bool` to avoid asking end users whether they want to turn on the
option. Turning it on explicitly is something only experts should do, so
making it hard to do so is not an issue.

I added a `depends on CFI_CLANG` clause to the new Kconfig option. I'm
not sure whether that makes sense or not, but it doesn't seem to make a
big difference.

In a future kernel release, I would like to add a Kconfig option similar
to CLANG_VERSION/RUSTC_VERSION for inspecting the version of the LLVM
inside rustc. Once that feature lands, this logic will be replaced with
a precise version check. This check is not being introduced here to
avoid introducing a new _VERSION constant in a fix.

Link: llvm/llvm-project#104826 [1]
Link: rust-lang/rust#129373 [2]
Fixes: ce4a262 ("cfi: add CONFIG_CFI_ICALL_NORMALIZE_INTEGERS")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202409231044.4f064459-oliver.sang@intel.com
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Link: https://lore.kernel.org/r/20240925-cfi-norm-kasan-fix-v1-1-0328985cdf33@google.com
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
  • Loading branch information
Darksonn authored and ojeda committed Sep 26, 2024
1 parent 93e34a0 commit 4c66f83
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
18 changes: 17 additions & 1 deletion arch/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,7 @@ config CFI_CLANG
config CFI_ICALL_NORMALIZE_INTEGERS
bool "Normalize CFI tags for integers"
depends on CFI_CLANG
depends on $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers)
depends on HAVE_CFI_ICALL_NORMALIZE_INTEGERS
help
This option normalizes the CFI tags for integer types so that all
integer types of the same size and signedness receive the same CFI
Expand All @@ -851,6 +851,22 @@ config CFI_ICALL_NORMALIZE_INTEGERS

This option is necessary for using CFI with Rust. If unsure, say N.

config HAVE_CFI_ICALL_NORMALIZE_INTEGERS
def_bool !GCOV_KERNEL && !KASAN
depends on CFI_CLANG
depends on $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers)
help
Is CFI_ICALL_NORMALIZE_INTEGERS supported with the set of compilers
currently in use?

This option defaults to false if GCOV or KASAN is enabled, as there is
an LLVM bug that makes normalized integers tags incompatible with
KASAN and GCOV. Kconfig currently does not have the infrastructure to
detect whether your rustc compiler contains the fix for this bug, so
it is assumed that it doesn't. If your compiler has the fix, you can
explicitly enable this option in your config file. The Kconfig logic
needed to detect this will be added in a future kernel release.

config CFI_PERMISSIVE
bool "Use CFI in permissive mode"
depends on CFI_CLANG
Expand Down
2 changes: 1 addition & 1 deletion init/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -1946,7 +1946,7 @@ config RUST
depends on !GCC_PLUGIN_RANDSTRUCT
depends on !RANDSTRUCT
depends on !DEBUG_INFO_BTF || PAHOLE_HAS_LANG_EXCLUDE
depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers)
depends on !CFI_CLANG || RUSTC_VERSION >= 107900 && HAVE_CFI_ICALL_NORMALIZE_INTEGERS
select CFI_ICALL_NORMALIZE_INTEGERS if CFI_CLANG
depends on !CALL_PADDING || RUSTC_VERSION >= 108100
depends on !KASAN_SW_TAGS
Expand Down

0 comments on commit 4c66f83

Please sign in to comment.