| Message ID | 20180710222639.8241-13-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/10/2018 03:26 PM, Yu-cheng Yu wrote: > +++ b/arch/x86/include/asm/traps.h > @@ -157,6 +157,7 @@ enum { > * bit 3 == 1: use of reserved bit detected > * bit 4 == 1: fault was an instruction fetch > * bit 5 == 1: protection keys block access > + * bit 6 == 1: shadow stack access fault > */ Could we document this bit better? Is this a fault where the *processor* thought it should be a shadow stack fault? Or is it also set on faults to valid shadow stack PTEs that just happen to fault for other reasons, say protection keys?
On 07/10/2018 03:26 PM, Yu-cheng Yu wrote: > + /* > + * Verify X86_PF_SHSTK is within a shadow stack VMA. > + * It is always an error if there is a shadow stack > + * fault outside a shadow stack VMA. > + */ > + if (error_code & X86_PF_SHSTK) { > + if (!(vma->vm_flags & VM_SHSTK)) > + return 1; > + return 0; > + } It turns out that a X86_PF_SHSTK just means that the processor faulted while doing access to something it thinks should be a shadow-stack virtual address. But, we *can* have faults on shadow stack accesses for non-shadow-stack reasons. I think you need to remove the 'return 0' and let it fall through to the other access checks that we might be failing. If it's a shadow stack access, it has to be a shadow stack VMA. But, a shadow-stack access fault to a shadow stack VMA isn't _necessarily_ OK.
On Tue, 2018-07-10 at 15:52 -0700, Dave Hansen wrote: > On 07/10/2018 03:26 PM, Yu-cheng Yu wrote: > > > > +++ b/arch/x86/include/asm/traps.h > > @@ -157,6 +157,7 @@ enum { > > * bit 3 == 1: use of reserved > > bit detected > > * bit 4 == 1: fault was an > > instruction fetch > > * bit 5 == 1: protection keys > > block access > > + * bit 6 == 1: shadow stack > > access fault > > */ > Could we document this bit better? > > Is this a fault where the *processor* thought it should be a shadow > stack fault? Or is it also set on faults to valid shadow stack PTEs > that just happen to fault for other reasons, say protection keys? Thanks Vedvyas for explaining this to me. I will add this to comments: This flag is 1 if (1) CR4.CET = 1; and (2) the access causing the page- fault exception was a shadow-stack data access. So this bit does not report the reason for the fault. It reports the type of access; i.e. it was a shadow-stack-load or a shadow-stack-store that took the page fault. The fault could have been caused by any variety of reasons including protection keys.
diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h index 5196050ff3d5..58ea2f5722e9 100644 --- a/arch/x86/include/asm/traps.h +++ b/arch/x86/include/asm/traps.h @@ -157,6 +157,7 @@ enum { * bit 3 == 1: use of reserved bit detected * bit 4 == 1: fault was an instruction fetch * bit 5 == 1: protection keys block access + * bit 6 == 1: shadow stack access fault */ enum x86_pf_error_code { X86_PF_PROT = 1 << 0, @@ -165,5 +166,6 @@ enum x86_pf_error_code { X86_PF_RSVD = 1 << 3, X86_PF_INSTR = 1 << 4, X86_PF_PK = 1 << 5, + X86_PF_SHSTK = 1 << 6, }; #endif /* _ASM_X86_TRAPS_H */ diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 2aafa6ab6103..fcd5739151f9 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -1163,6 +1163,17 @@ access_error(unsigned long error_code, struct vm_area_struct *vma) (error_code & X86_PF_INSTR), foreign)) return 1; + /* + * Verify X86_PF_SHSTK is within a shadow stack VMA. + * It is always an error if there is a shadow stack + * fault outside a shadow stack VMA. + */ + if (error_code & X86_PF_SHSTK) { + if (!(vma->vm_flags & VM_SHSTK)) + return 1; + return 0; + } + if (error_code & X86_PF_WRITE) { /* write, present and write, not present: */ if (unlikely(!(vma->vm_flags & VM_WRITE)))
If a page fault is triggered by a shadow stack access (e.g. call/ret) or shadow stack management instructions (e.g. wrussq), then bit[6] of the page fault error code is set. In access_error(), we check if a shadow stack page fault is within a shadow stack memory area. Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> --- arch/x86/include/asm/traps.h | 2 ++ arch/x86/mm/fault.c | 11 +++++++++++ 2 files changed, 13 insertions(+)