[PATCH] cfi: fix conditions in HAVE_CFI_ICALL_NORMALIZE_INTEGERS

[PATCH] cfi: fix conditions in HAVE_CFI_ICALL_NORMALIZE_INTEGERS

[Alice Ryhl]

The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with KASAN
because LLVM will emit some constructors when using KASAN that are
assigned incorrect CFI tags. These constructors are emitted due to use
of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are
respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled.
However, the KASAN_HW_TAGS option relies on hardware support for MTE
instead and does not pass either flag. (Note also that KASAN_HW_TAGS
does not `select CONSTRUCTORS`.)

Additionally, the option is configured to have a prompt and gated behind
EXPERT. The previous method for a user override of the option did not
actually work. This is expected to be temporary, as I intend to add a
precise detection check for 6.13 - I did not included that here to avoid
adding a RUSTC_LLVM_VERSION config in a fix.

Fixes: 4c66f8307ac0 ("cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig")
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 arch/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8af374ea1adc..2632de28c05a 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -852,8 +852,9 @@ 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
+	bool "Are normalized CFI tags for integers available?"
+	default !GCOV_KERNEL && !KASAN_GENERIC && !KASAN_SW_TAGS
+	depends on EXPERT || (!GCOV_KERNEL && !KASAN_GENERIC && !KASAN_SW_TAGS)
 	depends on $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers)
 	help
 	  Is CFI_ICALL_NORMALIZE_INTEGERS supported with the set of compilers

---
base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
change-id: 20241008-cfi-fix-llvm-gate-115e48d6acc9

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

Re: [PATCH] cfi: fix conditions in HAVE_CFI_ICALL_NORMALIZE_INTEGERS

[Matthew Maurer]

This makes sense, as some folks have a Rust compiler they know has the
fix, but build system detection for it isn't there yet. This lets them
override availability if needed.

That said, we should definitely be sure to get this back to a
non-configurable toggle once the LLVM version detection is in.

Reviewed-By: Matthew Maurer <mmaurer@google.com>

On Tue, Oct 8, 2024 at 10:42 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with KASAN
> because LLVM will emit some constructors when using KASAN that are
> assigned incorrect CFI tags. These constructors are emitted due to use
> of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are
> respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled.
> However, the KASAN_HW_TAGS option relies on hardware support for MTE
> instead and does not pass either flag. (Note also that KASAN_HW_TAGS
> does not `select CONSTRUCTORS`.)
>
> Additionally, the option is configured to have a prompt and gated behind
> EXPERT. The previous method for a user override of the option did not
> actually work. This is expected to be temporary, as I intend to add a
> precise detection check for 6.13 - I did not included that here to avoid
> adding a RUSTC_LLVM_VERSION config in a fix.
>
> Fixes: 4c66f8307ac0 ("cfi: encode cfi normalized integers + kasan/gcov bug in Kconfig")
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  arch/Kconfig | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8af374ea1adc..2632de28c05a 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -852,8 +852,9 @@ 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
> +       bool "Are normalized CFI tags for integers available?"
> +       default !GCOV_KERNEL && !KASAN_GENERIC && !KASAN_SW_TAGS
> +       depends on EXPERT || (!GCOV_KERNEL && !KASAN_GENERIC && !KASAN_SW_TAGS)
>         depends on $(cc-option,-fsanitize=kcfi -fsanitize-cfi-icall-experimental-normalize-integers)
>         help
>           Is CFI_ICALL_NORMALIZE_INTEGERS supported with the set of compilers
>
> ---
> base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
> change-id: 20241008-cfi-fix-llvm-gate-115e48d6acc9
>
> Best regards,
> --
> Alice Ryhl <aliceryhl@google.com>
>

Re: [PATCH] cfi: fix conditions in HAVE_CFI_ICALL_NORMALIZE_INTEGERS

[Sami Tolvanen]

Hi Alice,

On Tue, Oct 8, 2024 at 10:46 AM Matthew Maurer <mmaurer@google.com> wrote:
>
> This makes sense, as some folks have a Rust compiler they know has the
> fix, but build system detection for it isn't there yet. This lets them
> override availability if needed.
>
> That said, we should definitely be sure to get this back to a
> non-configurable toggle once the LLVM version detection is in.
>
> Reviewed-By: Matthew Maurer <mmaurer@google.com>
>
> On Tue, Oct 8, 2024 at 10:42 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > The CFI_ICALL_NORMALIZE_INTEGERS option is incompatible with KASAN
> > because LLVM will emit some constructors when using KASAN that are
> > assigned incorrect CFI tags. These constructors are emitted due to use
> > of -fsanitize=kernel-address or -fsanitize=kernel-hwaddress that are
> > respectively passed when KASAN_GENERIC or KASAN_SW_TAGS are enabled.
> > However, the KASAN_HW_TAGS option relies on hardware support for MTE
> > instead and does not pass either flag. (Note also that KASAN_HW_TAGS
> > does not `select CONSTRUCTORS`.)
> >
> > Additionally, the option is configured to have a prompt and gated behind
> > EXPERT. The previous method for a user override of the option did not
> > actually work. This is expected to be temporary, as I intend to add a
> > precise detection check for 6.13 - I did not included that here to avoid
> > adding a RUSTC_LLVM_VERSION config in a fix.

This sounds reasonable to me.

Reviewed-by: Sami Tolvanen <samitolvanen@google.com>

Sami