diff mbox series

[v3,15/37] x86/mm: Check Shadow Stack page fault errors

Message ID 20221104223604.29615-16-rick.p.edgecombe@intel.com (mailing list archive)
State New
Headers show
Series Shadow stacks for userspace | expand

Commit Message

Edgecombe, Rick P Nov. 4, 2022, 10:35 p.m. UTC 
From: Yu-cheng Yu <yu-cheng.yu@intel.com>

The CPU performs "shadow stack accesses" when it expects to encounter
shadow stack mappings. These accesses can be implicit (via CALL/RET
instructions) or explicit (instructions like WRSS).

Shadow stacks accesses to shadow-stack mappings 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. The kernel needs to use faults to implement those features.

The architecture has concepts of both shadow stack reads and shadow stack
writes. Any shadow stack access to non-shadow stack memory will generate
a fault with the shadow stack error code bit set.

This means that, unlike normal write protection, the fault handler needs
to create a type of memory that can be written to (with instructions that
generate shadow stack writes), even to fulfill a read access. So in the
case of COW memory, the COW needs to take place even with a shadow stack
read. Otherwise the page will be left (shadow stack) writable in
userspace. So to trigger the appropriate behavior, set FAULT_FLAG_WRITE
for shadow stack accesses, even if the access was a shadow stack read.

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. Also, generate the errors for invalid shadow stack accesses.

Tested-by: Pengfei Xu <pengfei.xu@intel.com>
Tested-by: John Allen <john.allen@amd.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>

---

v3:
 - Improve comment talking about using FAULT_FLAG_WRITE (Peterz)

v2:
 - Update commit log with verbiage/feedback from Dave Hansen
 - Clarify reasoning for FAULT_FLAG_WRITE for all shadow stack accesses
 - Update comments with some verbiage from Dave Hansen

Yu-cheng v30:
 - Update Subject line and add a verb

 arch/x86/include/asm/trap_pf.h |  2 ++
 arch/x86/mm/fault.c            | 26 ++++++++++++++++++++++++++
 2 files changed, 28 insertions(+)

Comments

Peter Zijlstra Nov. 15, 2022, 11:47 a.m. UTC | #1 
On Fri, Nov 04, 2022 at 03:35:42PM -0700, Rick Edgecombe wrote:
> @@ -1331,6 +1345,18 @@ void do_user_addr_fault(struct pt_regs *regs,
>  
>  	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
>  
> +	/*
> +	 * To service shadow stack read faults, unlike normal read faults, the
> +	 * fault handler needs to create a type of memory that will also be
> +	 * writable (with instructions that generate shadow stack writes).
> +	 * In the case of COW memory, the COW needs to take place even with
> +	 * a shadow stack read. Otherwise the shared page will be left (shadow
> +	 * stack) writable in userspace. So to trigger the appropriate behavior
> +	 * by setting FAULT_FLAG_WRITE for shadow stack accesses, even if the
> +	 * access was a shadow stack read.
> +	 */

Clear as mud... So SS pages are 'Write=0,Dirty=1', which, per
construction, lack a RW bit. And these pages are writable (WRUSS).

pte_wrprotect() seems to do: _PAGE_DIRTY->_PAGE_COW (which is really
weird in this situation), resulting in: 'Write=0,Dirty=0,Cow=1'.

That's regular RO memory and won't raise read-faults.

But I'm thinking RET will trip #PF here when it tries to read the SS
because the SSP is not a proper shadow stack page?

And in that case you want to tickle pte_mkwrite() to undo the
pte_wrprotect() above?

So while the #PF is a 'read' fault due to RET not actually writing to
the shadow stack, you want to force a write fault so it will re-instate
the SS page.

Did I get that right?

> +	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.17.1
>
Edgecombe, Rick P Nov. 15, 2022, 8:03 p.m. UTC | #2 
On Tue, 2022-11-15 at 12:47 +0100, Peter Zijlstra wrote:
> On Fri, Nov 04, 2022 at 03:35:42PM -0700, Rick Edgecombe wrote:
> > @@ -1331,6 +1345,18 @@ void do_user_addr_fault(struct pt_regs
> > *regs,
> >   
> >        perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
> >   
> > +     /*
> > +      * To service shadow stack read faults, unlike normal read
> > faults, the
> > +      * fault handler needs to create a type of memory that will
> > also be
> > +      * writable (with instructions that generate shadow stack
> > writes).
> > +      * In the case of COW memory, the COW needs to take place
> > even with
> > +      * a shadow stack read. Otherwise the shared page will be
> > left (shadow
> > +      * stack) writable in userspace. So to trigger the
> > appropriate behavior
> > +      * by setting FAULT_FLAG_WRITE for shadow stack accesses,
> > even if the
> > +      * access was a shadow stack read.
> > +      */
> 
> Clear as mud... So SS pages are 'Write=0,Dirty=1', which, per
> construction, lack a RW bit. And these pages are writable (WRUSS).
> 
> pte_wrprotect() seems to do: _PAGE_DIRTY->_PAGE_COW (which is really
> weird in this situation), resulting in: 'Write=0,Dirty=0,Cow=1'.
> 
> That's regular RO memory and won't raise read-faults.
> 
> But I'm thinking RET will trip #PF here when it tries to read the SS
> because the SSP is not a proper shadow stack page?
> 
> And in that case you want to tickle pte_mkwrite() to undo the
> pte_wrprotect() above?
> 
> So while the #PF is a 'read' fault due to RET not actually writing to
> the shadow stack, you want to force a write fault so it will re-
> instate
> the SS page.
> 
> Did I get that right?

That's right. I think the assumption that needs to be broken in the
readers head is that you can satisfy a read fault with read-only PTE.
This is kind of baked in all over the place with the zero-pfn, COW,
etc. Maybe I should try to start with that.
Peter Zijlstra Nov. 15, 2022, 9:07 p.m. UTC | #3 
On Tue, Nov 15, 2022 at 08:03:06PM +0000, Edgecombe, Rick P wrote:

> That's right. I think the assumption that needs to be broken in the
> readers head is that you can satisfy a read fault with read-only PTE.
> This is kind of baked in all over the place with the zero-pfn, COW,
> etc. Maybe I should try to start with that.

Maybe something like:

CoW -- pte_wrprotect() -- changes a SS page 'Write=0,Dirty=1' to
'Write=0,Dirty=0,CoW=1' which is a 'regular' RO page. A SS read from RET
will #PF because it expects a SS page. Make sure to break the CoW so it
can be restored to an SS page, as such force the write path and tickle
pte_mkwrite().
Edgecombe, Rick P Nov. 15, 2022, 11:13 p.m. UTC | #4 
On Tue, 2022-11-15 at 22:07 +0100, Peter Zijlstra wrote:
> On Tue, Nov 15, 2022 at 08:03:06PM +0000, Edgecombe, Rick P wrote:
> 
> > That's right. I think the assumption that needs to be broken in the
> > readers head is that you can satisfy a read fault with read-only
> > PTE.
> > This is kind of baked in all over the place with the zero-pfn, COW,
> > etc. Maybe I should try to start with that.
> 
> Maybe something like:
> 
> CoW -- pte_wrprotect() -- changes a SS page 'Write=0,Dirty=1' to
> 'Write=0,Dirty=0,CoW=1' which is a 'regular' RO page. A SS read from
> RET
> will #PF because it expects a SS page. Make sure to break the CoW so
> it
> can be restored to an SS page, as such force the write path and
> tickle
> pte_mkwrite().

Hmm, TBH I'm not sure it's more clear. I tried to take this and fill it
out more. Does it sound better?


When a page becomes COW it changes from a shadow stack permissioned
page (Write=0,Dirty=1) to (Write=0,Dirty=0,CoW=1), which is simply
read-only to the CPU. When shadow stack is enabled, a RET would
normally pop the shadow stack by reading it with a "shadow stack read"
access. However, in the COW case the shadow stack memory does not have
shadow stack permissions, it is read-only. So it will generate a fault.

For conventionally writable pages, a read can be serviced with a read
only PTE, and COW would not have to happen. But for shadow stack, there
isn't the concept of read-only shadow stack memory. If it is shadow
stack permissioned, it can be modified via CALL and RET instructions.
So COW needs to happen before any memory can be mapped with shadow
stack permissions.

Shadow stack accesses (read or write) need to be serviced with shadow
stack permissioned memory, so in the case of a shadow stack read
access, treat it as a WRITE fault so both COW will happen and the write
fault path will tickle maybe_mkwrite() and map the memory shadow stack.
Peter Zijlstra Nov. 16, 2022, 10:09 a.m. UTC | #5 
On Tue, Nov 15, 2022 at 11:13:34PM +0000, Edgecombe, Rick P wrote:

> When a page becomes COW it changes from a shadow stack permissioned
> page (Write=0,Dirty=1) to (Write=0,Dirty=0,CoW=1), which is simply
> read-only to the CPU. When shadow stack is enabled, a RET would
> normally pop the shadow stack by reading it with a "shadow stack read"
> access. However, in the COW case the shadow stack memory does not have
> shadow stack permissions, it is read-only. So it will generate a fault.
> 
> For conventionally writable pages, a read can be serviced with a read
> only PTE, and COW would not have to happen. But for shadow stack, there
> isn't the concept of read-only shadow stack memory. If it is shadow
> stack permissioned, it can be modified via CALL and RET instructions.
> So COW needs to happen before any memory can be mapped with shadow
> stack permissions.
> 
> Shadow stack accesses (read or write) need to be serviced with shadow
> stack permissioned memory, so in the case of a shadow stack read
> access, treat it as a WRITE fault so both COW will happen and the write
> fault path will tickle maybe_mkwrite() and map the memory shadow stack.

ACK.
diff mbox series

Patch

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 7b0d4ab894c8..0af3d7f52c2e 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1138,8 +1138,22 @@  access_error(unsigned long error_code, struct vm_area_struct *vma)
 				       (error_code & X86_PF_INSTR), foreign))
 		return 1;
 
+	/*
+	 * Shadow stack accesses (PF_SHSTK=1) are only permitted to
+	 * shadow stack VMAs. All other accesses result in an error.
+	 */
+	if (error_code & X86_PF_SHSTK) {
+		if (unlikely(!(vma->vm_flags & VM_SHADOW_STACK)))
+			return 1;
+		if (unlikely(!(vma->vm_flags & VM_WRITE)))
+			return 1;
+		return 0;
+	}
+
 	if (error_code & X86_PF_WRITE) {
 		/* write, present and write, not present: */
+		if (unlikely(vma->vm_flags & VM_SHADOW_STACK))
+			return 1;
 		if (unlikely(!(vma->vm_flags & VM_WRITE)))
 			return 1;
 		return 0;
@@ -1331,6 +1345,18 @@  void do_user_addr_fault(struct pt_regs *regs,
 
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
 
+	/*
+	 * To service shadow stack read faults, unlike normal read faults, the
+	 * fault handler needs to create a type of memory that will also be
+	 * writable (with instructions that generate shadow stack writes).
+	 * In the case of COW memory, the COW needs to take place even with
+	 * a shadow stack read. Otherwise the shared page will be left (shadow
+	 * stack) writable in userspace. So to trigger the appropriate behavior
+	 * by setting FAULT_FLAG_WRITE for shadow stack accesses, even if the
+	 * access was a shadow stack read.
+	 */
+	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)