Re: [PATCH] x86/alternatives: Make FineIBT mode Kconfig selectable

From: Nathan Chancellor
Date: Wed May 01 2024 - 12:33:53 EST


On Tue, Apr 30, 2024 at 05:02:22PM -0700, Kees Cook wrote:
> Since FineIBT performs checking at the destination, it is weaker against
> attacks that can construct arbitrary executable memory contents. As such,
> some system builders want to run with FineIBT disabled by default. Allow
> the "cfi=kcfi" boot param mode to be selectable through Kconfig via the
> newly introduced CONFIG_CFI_AUTO_DEFAULT.
>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>

I verified that flipping the configuration does indeed change the
default and that 'cfi=' could still be used to override whatever choice
was made at compile time. This patch was a perfect excuse to put my new
CET enabled test machine to work.

Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx>
Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx>

CFI_DEFAULT_AUTO reads a little bit better to me personally but I am not
looking to get into painting today :)

> ---
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: x86@xxxxxxxxxx
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: Alexei Starovoitov <ast@xxxxxxxxxx>
> Cc: Sami Tolvanen <samitolvanen@xxxxxxxxxx>
> Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
> Cc: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> ---
> arch/x86/Kconfig | 9 +++++++++
> arch/x86/include/asm/cfi.h | 2 +-
> arch/x86/kernel/alternative.c | 8 ++++----
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 4fff6ed46e90..d5cf52d2f6a8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2424,6 +2424,15 @@ config STRICT_SIGALTSTACK_SIZE
>
> Say 'N' unless you want to really enforce this check.
>
> +config CFI_AUTO_DEFAULT
> + bool "Attempt to use FineIBT by default at boot time"
> + depends on FINEIBT
> + default y
> + help
> + Attempt to use FineIBT by default at boot time. If enabled,
> + this is the same as booting with "cfi=auto". If disabled,
> + this is the same as booting with "cfi=kcfi".
> +
> source "kernel/livepatch/Kconfig"
>
> endmenu
> diff --git a/arch/x86/include/asm/cfi.h b/arch/x86/include/asm/cfi.h
> index 7cd752557905..31d19c815f99 100644
> --- a/arch/x86/include/asm/cfi.h
> +++ b/arch/x86/include/asm/cfi.h
> @@ -93,7 +93,7 @@
> *
> */
> enum cfi_mode {
> - CFI_DEFAULT, /* FineIBT if hardware has IBT, otherwise kCFI */
> + CFI_AUTO, /* FineIBT if hardware has IBT, otherwise kCFI */
> CFI_OFF, /* Taditional / IBT depending on .config */
> CFI_KCFI, /* Optionally CALL_PADDING, IBT, RETPOLINE */
> CFI_FINEIBT, /* see arch/x86/kernel/alternative.c */
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 45a280f2161c..e8d0892d89cf 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -902,8 +902,8 @@ void __init_or_module apply_seal_endbr(s32 *start, s32 *end) { }
>
> #endif /* CONFIG_X86_KERNEL_IBT */
>
> -#ifdef CONFIG_FINEIBT
> -#define __CFI_DEFAULT CFI_DEFAULT
> +#ifdef CONFIG_CFI_AUTO_DEFAULT
> +#define __CFI_DEFAULT CFI_AUTO
> #elif defined(CONFIG_CFI_CLANG)
> #define __CFI_DEFAULT CFI_KCFI
> #else
> @@ -1011,7 +1011,7 @@ static __init int cfi_parse_cmdline(char *str)
> }
>
> if (!strcmp(str, "auto")) {
> - cfi_mode = CFI_DEFAULT;
> + cfi_mode = CFI_AUTO;
> } else if (!strcmp(str, "off")) {
> cfi_mode = CFI_OFF;
> cfi_rand = false;
> @@ -1271,7 +1271,7 @@ static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline,
> "FineIBT preamble wrong size: %ld", fineibt_preamble_size))
> return;
>
> - if (cfi_mode == CFI_DEFAULT) {
> + if (cfi_mode == CFI_AUTO) {
> cfi_mode = CFI_KCFI;
> if (HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT))
> cfi_mode = CFI_FINEIBT;
> --
> 2.34.1
>