[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240501163314.GA2472577@dev-arch.thelio-3990X>
Date: Wed, 1 May 2024 09:33:14 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Kees Cook <keescook@...omium.org>
Cc: Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>,
Alexei Starovoitov <ast@...nel.org>,
Sami Tolvanen <samitolvanen@...gle.com>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Juergen Gross <jgross@...e.com>, Breno Leitao <leitao@...ian.org>,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] x86/alternatives: Make FineIBT mode Kconfig selectable
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@...omium.org>
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@...nel.org>
Tested-by: Nathan Chancellor <nathan@...nel.org>
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@...radead.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> Cc: x86@...nel.org
> Cc: "H. Peter Anvin" <hpa@...or.com>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Sami Tolvanen <samitolvanen@...gle.com>
> Cc: Nathan Chancellor <nathan@...nel.org>
> Cc: Josh Poimboeuf <jpoimboe@...nel.org>
> ---
> 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
>
Powered by blists - more mailing lists