Re: [PATCH RFC V3 8/9] x86/fault: Report the PKRS state on fault

From: Dave Hansen
Date: Tue Oct 13 2020 - 14:56:57 EST


> @@ -548,6 +549,11 @@ show_fault_oops(struct pt_regs *regs, unsigned long error_code, unsigned long ad
> (error_code & X86_PF_PK) ? "protection keys violation" :
> "permissions violation");
>
> +#ifdef CONFIG_ARCH_HAS_SUPERVISOR_PKEYS
> + if (irq_state && (error_code & X86_PF_PK))
> + pr_alert("PKRS: 0x%x\n", irq_state->pkrs);
> +#endif

This means everyone will see 'PKRS: 0x0', even if they're on non-PKS
hardware. I think I'd rather have this only show PKRS when we're on
cpu_feature_enabled(PKS) hardware.

...
> @@ -1148,14 +1156,15 @@ static int fault_in_kernel_space(unsigned long address)
> */
> static void
> do_kern_addr_fault(struct pt_regs *regs, unsigned long hw_error_code,
> - unsigned long address)
> + unsigned long address, irqentry_state_t *irq_state)
> {
> /*
> - * Protection keys exceptions only happen on user pages. We
> - * have no user pages in the kernel portion of the address
> - * space, so do not expect them here.
> + * If protection keys are not enabled for kernel space
> + * do not expect Pkey errors here.
> */

Let's fix the double-negative:

/*
* PF_PK is only expected on kernel addresses whenn
* supervisor pkeys are enabled:
*/

> - WARN_ON_ONCE(hw_error_code & X86_PF_PK);
> + if (!IS_ENABLED(CONFIG_ARCH_HAS_SUPERVISOR_PKEYS) ||
> + !cpu_feature_enabled(X86_FEATURE_PKS))
> + WARN_ON_ONCE(hw_error_code & X86_PF_PK);

Yeah, please stick X86_FEATURE_PKS in disabled-features so you can use
cpu_feature_enabled(X86_FEATURE_PKS) by itself here..