| Message ID | 20210316151054.5405-15-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Control-flow Enforcement: Shadow Stack | expand |
On Tue, Mar 16, 2021 at 08:10:40AM -0700, Yu-cheng Yu wrote: > Shadow stack accesses are those that are performed by the CPU where it > expects to encounter a shadow stack mapping. These accesses are performed > implicitly by CALL/RET at the site of the shadow stack pointer. These > accesses are made explicitly by shadow stack management instructions like > WRUSSQ. > > Shadow stacks accesses to shadow-stack mapping can see faults in normal, > valid operation just like regular accesses to regular mappings. Shadow > stacks need some of the same features like delayed allocation, swap and > copy-on-write. > > Shadow stack accesses can also result in errors, such as when a shadow > stack overflows, or if a shadow stack access occurs to a non-shadow-stack > mapping. > > In handling a shadow stack page fault, verify it occurs within a shadow > stack mapping. It is always an error otherwise. For valid shadow stack > accesses, set FAULT_FLAG_WRITE to effect copy-on-write. Because clearing > _PAGE_DIRTY (vs. _PAGE_RW) is used to trigger the fault, shadow stack read > fault and shadow stack write fault are not differentiated and both are > handled as a write access. > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > --- > arch/x86/include/asm/trap_pf.h | 2 ++ > arch/x86/mm/fault.c | 19 +++++++++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/arch/x86/include/asm/trap_pf.h b/arch/x86/include/asm/trap_pf.h > index 10b1de500ab1..afa524325e55 100644 > --- a/arch/x86/include/asm/trap_pf.h > +++ b/arch/x86/include/asm/trap_pf.h > @@ -11,6 +11,7 @@ > * 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 > * bit 15 == 1: SGX MMU page-fault > */ > enum x86_pf_error_code { > @@ -20,6 +21,7 @@ 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, > X86_PF_SGX = 1 << 15, > }; > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > index a73347e2cdfc..4316732a18c6 100644 > --- a/arch/x86/mm/fault.c > +++ b/arch/x86/mm/fault.c > @@ -1100,6 +1100,17 @@ access_error(unsigned long error_code, struct vm_area_struct *vma) > (error_code & X86_PF_INSTR), foreign)) > return 1; > > + /* > + * Verify a shadow stack access is within a shadow stack VMA. > + * It is always an error otherwise. Normal data access to a > + * shadow stack area is checked in the case followed. > + */ > + if (error_code & X86_PF_SHSTK) { > + if (!(vma->vm_flags & VM_SHSTK)) > + return 1; > + return 0; Any reason to return 0 here? I would rather keep the single return 0 in the function, after all checks are done. > + } > + > if (error_code & X86_PF_WRITE) { > /* write, present and write, not present: */ > if (unlikely(!(vma->vm_flags & VM_WRITE))) > @@ -1293,6 +1304,14 @@ void do_user_addr_fault(struct pt_regs *regs, > > perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address); > > + /* > + * Clearing _PAGE_DIRTY is used to detect shadow stack access. > + * This method cannot distinguish shadow stack read vs. write. > + * For valid shadow stack accesses, set FAULT_FLAG_WRITE to effect > + * copy-on-write. > + */ > + if (error_code & X86_PF_SHSTK) > + flags |= FAULT_FLAG_WRITE; > if (error_code & X86_PF_WRITE) > flags |= FAULT_FLAG_WRITE; > if (error_code & X86_PF_INSTR) > -- > 2.21.0 >
On 3/22/2021 3:38 AM, Kirill A. Shutemov wrote: > On Tue, Mar 16, 2021 at 08:10:40AM -0700, Yu-cheng Yu wrote: [...] >> >> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c >> index a73347e2cdfc..4316732a18c6 100644 >> --- a/arch/x86/mm/fault.c >> +++ b/arch/x86/mm/fault.c >> @@ -1100,6 +1100,17 @@ access_error(unsigned long error_code, struct vm_area_struct *vma) >> (error_code & X86_PF_INSTR), foreign)) >> return 1; >> >> + /* >> + * Verify a shadow stack access is within a shadow stack VMA. >> + * It is always an error otherwise. Normal data access to a >> + * shadow stack area is checked in the case followed. >> + */ >> + if (error_code & X86_PF_SHSTK) { >> + if (!(vma->vm_flags & VM_SHSTK)) >> + return 1; >> + return 0; > > Any reason to return 0 here? I would rather keep the single return 0 in > the function, after all checks are done. > For shadow stack fault, X86_PF_SHSTK and X86_PF_WRITE both can be set. So for shadow stack fault, return from here and don't go into the normal write fault case. Thanks, Yu-cheng >> + } >> + >> if (error_code & X86_PF_WRITE) { >> /* write, present and write, not present: */ >> if (unlikely(!(vma->vm_flags & VM_WRITE))) >> @@ -1293,6 +1304,14 @@ void do_user_addr_fault(struct pt_regs *regs, >> >> perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address); >> >> + /* >> + * Clearing _PAGE_DIRTY is used to detect shadow stack access. >> + * This method cannot distinguish shadow stack read vs. write. >> + * For valid shadow stack accesses, set FAULT_FLAG_WRITE to effect >> + * copy-on-write. >> + */ >> + if (error_code & X86_PF_SHSTK) >> + flags |= FAULT_FLAG_WRITE; >> if (error_code & X86_PF_WRITE) >> flags |= FAULT_FLAG_WRITE; >> if (error_code & X86_PF_INSTR) >> -- >> 2.21.0 >> >
diff --git a/arch/x86/include/asm/trap_pf.h b/arch/x86/include/asm/trap_pf.h index 10b1de500ab1..afa524325e55 100644 --- a/arch/x86/include/asm/trap_pf.h +++ b/arch/x86/include/asm/trap_pf.h @@ -11,6 +11,7 @@ * 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 * bit 15 == 1: SGX MMU page-fault */ enum x86_pf_error_code { @@ -20,6 +21,7 @@ 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, X86_PF_SGX = 1 << 15, }; diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index a73347e2cdfc..4316732a18c6 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -1100,6 +1100,17 @@ access_error(unsigned long error_code, struct vm_area_struct *vma) (error_code & X86_PF_INSTR), foreign)) return 1; + /* + * Verify a shadow stack access is within a shadow stack VMA. + * It is always an error otherwise. Normal data access to a + * shadow stack area is checked in the case followed. + */ + 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))) @@ -1293,6 +1304,14 @@ void do_user_addr_fault(struct pt_regs *regs, perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address); + /* + * Clearing _PAGE_DIRTY is used to detect shadow stack access. + * This method cannot distinguish shadow stack read vs. write. + * For valid shadow stack accesses, set FAULT_FLAG_WRITE to effect + * copy-on-write. + */ + if (error_code & X86_PF_SHSTK) + flags |= FAULT_FLAG_WRITE; if (error_code & X86_PF_WRITE) flags |= FAULT_FLAG_WRITE; if (error_code & X86_PF_INSTR)