[v6,19/41] x86/mm: Check shadow stack page fault errors

  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 stack accesses to shadow-stack mappings can result in 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.

For the purpose of making this clearer, consider the following example.
If a process has a shadow stack, and forks, the shadow stack PTEs will
become read-only due to COW. If the CPU in one process performs a shadow
stack read access to the shadow stack, for example executing a RET and
causing the CPU to read the shadow stack copy of the return address, then
in order for the fault to be resolved the PTE will need to be set with
shadow stack permissions. But then the memory would be changeable from
userspace (from CALL, RET, WRSS, etc). So this scenario needs to trigger
COW, otherwise the shared page would be changeable from both processes.

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>

---
v6:
 - Update comment due to rename of Cow bit to SavedDirty

v5:
 - Add description of COW example (Boris)
 - Replace "permissioned" (Boris)
 - Remove capitalization of shadow stack (Boris)

v4:
 - Further improve comment talking about FAULT_FLAG_WRITE (Peterz)

v3:
 - Improve comment talking about using FAULT_FLAG_WRITE (Peterz)
---
 arch/x86/include/asm/trap_pf.h |  2 ++
 arch/x86/mm/fault.c            | 38 ++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)
  

On 18.02.23 22:14, Rick Edgecombe wrote:
> 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 stack accesses to shadow-stack mappings can result in 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.
> 
> For the purpose of making this clearer, consider the following example.
> If a process has a shadow stack, and forks, the shadow stack PTEs will
> become read-only due to COW. If the CPU in one process performs a shadow
> stack read access to the shadow stack, for example executing a RET and
> causing the CPU to read the shadow stack copy of the return address, then
> in order for the fault to be resolved the PTE will need to be set with
> shadow stack permissions. But then the memory would be changeable from
> userspace (from CALL, RET, WRSS, etc). So this scenario needs to trigger
> COW, otherwise the shared page would be changeable from both processes.
> 
> 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>
> 
> ---
> v6:
>   - Update comment due to rename of Cow bit to SavedDirty
> 
> v5:
>   - Add description of COW example (Boris)
>   - Replace "permissioned" (Boris)
>   - Remove capitalization of shadow stack (Boris)
> 
> v4:
>   - Further improve comment talking about FAULT_FLAG_WRITE (Peterz)
> 
> v3:
>   - Improve comment talking about using FAULT_FLAG_WRITE (Peterz)
> ---
>   arch/x86/include/asm/trap_pf.h |  2 ++
>   arch/x86/mm/fault.c            | 38 ++++++++++++++++++++++++++++++++++
>   2 files changed, 40 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 7b0d4ab894c8..42885d8e2036 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,30 @@ void do_user_addr_fault(struct pt_regs *regs,
>   
>   	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
>   
> +	/*
> +	 * When a page becomes COW it changes from a shadow stack permission
> +	 * page (Write=0,Dirty=1) to (Write=0,Dirty=0,SavedDirty=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 permission, 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 permission 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.
> +	 */

Again, I suggest dropping all details about COW from this comment and 
from the patch description. It's just one such case that can happen.
  

On Mon, 2023-02-20 at 13:57 +0100, David Hildenbrand wrote:
> >    
> > +     /*
> > +      * When a page becomes COW it changes from a shadow stack
> > permission
> > +      * page (Write=0,Dirty=1) to (Write=0,Dirty=0,SavedDirty=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 permission, 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 permission 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.
> > +      */
> 
> Again, I suggest dropping all details about COW from this comment
> and 
> from the patch description. It's just one such case that can happen.

Hi David,

I was just trying to edit this one to drop COW details, but I think in
this case, one of the major reasons for the code *is* actually COW. We
are not working around the whole inadvertent shadow stack memory piece
here, but something else: Making sure shadow stack memory is faulted in
and doing COW if required to make this possible. I came up with this,
does it seem better?


/*
 * For conventionally writable pages, a read can be serviced with a
 *
read only PTE. But for shadow stack, there isn't a concept of
 * read-
only shadow stack memory. If it a PTE has the shadow stack
 *
permission, it can be modified via CALL and RET instructions. So
 * core
MM needs to fault in a writable PTE and do things it already
 * does for
write faults.
 *
 * Shadow stack accesses (read or write) need to be
serviced with
 * shadow stack permission memory, so in the case of a
shadow stack
 * read access, treat it as a WRITE fault so both any
required COW will
 * happen and the write fault path will tickle
maybe_mkwrite() and map
 * the memory shadow stack.
 */



Thanks,
Rick
  

On 23.02.23 00:07, Edgecombe, Rick P wrote:
> On Mon, 2023-02-20 at 13:57 +0100, David Hildenbrand wrote:
>>>     
>>> +     /*
>>> +      * When a page becomes COW it changes from a shadow stack
>>> permission
>>> +      * page (Write=0,Dirty=1) to (Write=0,Dirty=0,SavedDirty=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 permission, 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 permission 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.
>>> +      */
>>
>> Again, I suggest dropping all details about COW from this comment
>> and
>> from the patch description. It's just one such case that can happen.
> 
> Hi David,

Hi Rick,

> 
> I was just trying to edit this one to drop COW details, but I think in
> this case, one of the major reasons for the code *is* actually COW. We
> are not working around the whole inadvertent shadow stack memory piece
> here, but something else: Making sure shadow stack memory is faulted in
> and doing COW if required to make this possible. I came up with this,
> does it seem better?

Regarding the fault handling I completely agree. We have to treat a read 
like a write event. And as read-only shadow stack PTEs don't exist, we 
have to tell the MM to create a writable one for us.

> 
> 
> /*
>   * For conventionally writable pages, a read can be serviced with a
>   *
> read only PTE. But for shadow stack, there isn't a concept of
>   * read-
> only shadow stack memory. If it a PTE has the shadow stack
>   *
> permission, it can be modified via CALL and RET instructions. So
>   * core
> MM needs to fault in a writable PTE and do things it already
>   * does for
> write faults.
>   *
>   * Shadow stack accesses (read or write) need to be
> serviced with
>   * shadow stack permission memory, so in the case of a
> shadow stack
>   * read access, treat it as a WRITE fault so both any
> required COW will
>   * happen and the write fault path will tickle
> maybe_mkwrite() and map
>   * the memory shadow stack.
>   */

That sounds good! I'd rewrite the last part slightly.

"
Shadow stack accesses (read or write) need to be serviced with
shadow stack permission memory, which always include write permissions. 
So in the case of a shadow stack read access, treat it as a WRITE fault. 
This will make sure that MM will prepare everything (e.g., break COW) 
such that maybe_mkwrite() can create a proper shadow stack PTE.
"