Re: [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction

[Dave Hansen]

On 07/12/2018 03:59 PM, Yu-cheng Yu wrote:
> On Tue, 2018-07-10 at 16:48 -0700, Dave Hansen wrote:
>>>
>>> +/*
>>> + * WRUSS is a kernel instrcution and but writes to user
>>> + * shadow stack memory.ÂÂWhen a fault occurs, both
>>> + * X86_PF_USER and X86_PF_SHSTK are set.
>>> + */
>>> +static int is_wruss(struct pt_regs *regs, unsigned long error_code)
>>> +{
>>> +	return (((error_code & (X86_PF_USER | X86_PF_SHSTK)) ==
>>> +		(X86_PF_USER | X86_PF_SHSTK)) && !user_mode(regs));
>>> +}
>> I thought X86_PF_USER was set based on the mode in which the fault
>> occurred.ÂÂDoes this mean that the architecture of this bit is different
>> now?
> 
> Yes.
> 
>> That seems like something we need to call out if so.ÂÂIt also means we
>> need to update the SDM because some of the text is wrong.
> 
> It needs to mention the WRUSS case.

Ugh.  The documentation for this is not pretty.  But, I guess this is
not fundamentally different from access to U=1 pages when SMAP is in
place and we've set EFLAGS.AC=1.

But, sheesh, we need to call this out really explicitly and make it
crystal clear what is going on.

We need to go through the page fault code very carefully and audit all
the X86_PF_USER spots and make sure there's no impact to those.  SMAP
should mean that we already dealt with these, but we still need an audit.

The docs[1] are clear as mud on this though: "Page entry has user
privilege (U=1) for a supervisor-level shadow-stack-load,
shadow-stack-store-intent or shadow-stack-store access except those that
originate from the WRUSS instruction."

Or, in short:

	"Page has U=1 ... except those that originate from the WRUSS 	
	instruction."

Which is backwards from what you said.  I really wish those docs had
reused the established SDM language instead of reinventing their own way
of saying things.

1.
https://software.intel.com/sites/default/files/managed/4d/2a/control-flow-enforcement-technology-preview.pdf