| Message ID | 20230218211433.26859-20-rick.p.edgecombe@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Shadow stacks for userspace | expand |
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. "
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. + */ + 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)