On Thu, Sep 29, 2022 at 03:29:04PM -0700, Rick Edgecombe wrote: > [...] > -#ifdef CONFIG_X86_KERNEL_IBT > +#if defined(CONFIG_X86_KERNEL_IBT) || defined(CONFIG_X86_SHADOW_STACK) This pattern is repeated several times. Perhaps there needs to be a CONFIG_X86_CET to make this more readable? Really just a style question. diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b68eb75887b8..6cb52616e0cf 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1836,6 +1836,11 @@ config CC_HAS_IBT (CC_IS_CLANG && CLANG_VERSION >= 140000)) && \ $(as-instr,endbr64) +config X86_CET + def_bool n + help + CET features are enabled (IBT and/or Shadow Stack) + config X86_KERNEL_IBT prompt "Indirect Branch Tracking" bool @@ -1843,6 +1848,7 @@ config X86_KERNEL_IBT # https://github.com/llvm/llvm-project/commit/9d7001eba9c4cb311e03cd8cdc231f9e579f2d0f depends on !LD_IS_LLD || LLD_VERSION >= 140000 select OBJTOOL + select X86_CET help Build the kernel with support for Indirect Branch Tracking, a hardware support course-grain forward-edge Control Flow Integrity @@ -1945,6 +1951,7 @@ config X86_SHADOW_STACK def_bool n depends on ARCH_HAS_SHADOW_STACK select ARCH_USES_HIGH_VMA_FLAGS + select X86_CET help Shadow Stack protection is a hardware feature that detects function return address corruption. Today the kernel's support is limited to > [...] > +#if defined(CONFIG_X86_KERNEL_IBT) || defined(CONFIG_X86_SHADOW_STACK) > +DEFINE_IDTENTRY_ERRORCODE(exc_control_protection) > +{ > + if (!cpu_feature_enabled(X86_FEATURE_IBT) && > + !cpu_feature_enabled(X86_FEATURE_SHSTK)) { > + pr_err("Unexpected #CP\n"); > + BUG(); > + } I second Kirill's question here. This seems an entirely survivable (but highly unexpected) state. I think this whole "if" could just be replaced with: WARN_ON_ONCE(!cpu_feature_enabled(X86_FEATURE_IBT) && !cpu_feature_enabled(X86_FEATURE_SHSTK), "Unexpected #CP\n"); Otherwise this looks good to me. Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> -- Kees Cook
