Re: [PATCH v2 07/39] x86/cet: Add user control-protection fault handler

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 29/09/2022 23:29, Rick Edgecombe wrote:
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index d62b2cb85cea..b7dde8730236 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -229,16 +223,74 @@ enum cp_error_code {
>  	CP_ENCL	     = 1 << 15,
>  };
>  
> -DEFINE_IDTENTRY_ERRORCODE(exc_control_protection)
> +#ifdef CONFIG_X86_SHADOW_STACK
> +static const char * const control_protection_err[] = {
> +	"unknown",
> +	"near-ret",
> +	"far-ret/iret",
> +	"endbranch",
> +	"rstorssp",
> +	"setssbsy",
> +};

These are a mix of SHSTK and IBT errors.  They should be inside
CONFIG_X86_CET using Kees' suggestion.

Also, if you express this as

static const char errors[][10] = {
    [0] = "unknown",
    [1] = "near ret",
    [2] = "far/iret",
    [3] = "endbranch",
    [4] = "rstorssp",
    [5] = "setssbsy",
};

then you can encode all the strings in roughly the space it takes to lay
out the pointers above.

> +
> +static DEFINE_RATELIMIT_STATE(cpf_rate, DEFAULT_RATELIMIT_INTERVAL,
> +			      DEFAULT_RATELIMIT_BURST);
> +
> +static void do_user_control_protection_fault(struct pt_regs *regs,
> +					     unsigned long error_code)
>  {
> -	if (!cpu_feature_enabled(X86_FEATURE_IBT)) {
> -		pr_err("Unexpected #CP\n");
> -		BUG();
> +	struct task_struct *tsk;
> +	unsigned long ssp;
> +
> +	/* Read SSP before enabling interrupts. */
> +	rdmsrl(MSR_IA32_PL3_SSP, ssp);
> +
> +	cond_local_irq_enable(regs);
> +
> +	if (!cpu_feature_enabled(X86_FEATURE_SHSTK))
> +		WARN_ONCE(1, "User-mode control protection fault with shadow support disabled\n");

So it's ok to get an unexpected #CP on CET-capable hardware, but not on
CET-incapable hardware?

The conditions for this WARN() (and others) probably want adjusting to
what the kernel has enabled, not what hardware is capable of.

> @@ -283,9 +335,29 @@ static int __init ibt_setup(char *str)
>  }
>  
>  __setup("ibt=", ibt_setup);
> -
> +#else
> +static void do_kernel_control_protection_fault(struct pt_regs *regs)
> +{
> +	WARN_ONCE(1, "Kernel-mode control protection fault with IBT disabled\n");
> +}
>  #endif /* CONFIG_X86_KERNEL_IBT */
>  
> +#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");

Do some future poor sole a favour and render the numeric error code
too.  Without it, the error is ambiguous between SHSTK and IBT when %rip
points at a call/ret instruction.

~Andrew