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

Andrew Cooper
2022-10-05T01:21:10Z

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