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