| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 20 Feb 2023 13:57:08 +0100
From: David Hildenbrand <david@...hat.com>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>, x86@...nel.org,
"H . Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, linux-mm@...ck.org,
linux-arch@...r.kernel.org, linux-api@...r.kernel.org,
Arnd Bergmann <arnd@...db.de>,
Andy Lutomirski <luto@...nel.org>,
Balbir Singh <bsingharora@...il.com>,
Borislav Petkov <bp@...en8.de>,
Cyrill Gorcunov <gorcunov@...il.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Eugene Syromiatnikov <esyr@...hat.com>,
Florian Weimer <fweimer@...hat.com>,
"H . J . Lu" <hjl.tools@...il.com>, Jann Horn <jannh@...gle.com>,
Jonathan Corbet <corbet@....net>,
Kees Cook <keescook@...omium.org>,
Mike Kravetz <mike.kravetz@...cle.com>,
Nadav Amit <nadav.amit@...il.com>,
Oleg Nesterov <oleg@...hat.com>, Pavel Machek <pavel@....cz>,
Peter Zijlstra <peterz@...radead.org>,
Randy Dunlap <rdunlap@...radead.org>,
Weijiang Yang <weijiang.yang@...el.com>,
"Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
John Allen <john.allen@....com>, kcc@...gle.com,
eranian@...gle.com, rppt@...nel.org, jamorris@...ux.microsoft.com,
dethoma@...rosoft.com, akpm@...ux-foundation.org,
Andrew.Cooper3@...rix.com, christina.schimpe@...el.com,
debug@...osinc.com
Cc: Yu-cheng Yu <yu-cheng.yu@...el.com>
Subject: Re: [PATCH v6 19/41] x86/mm: Check shadow stack page fault errors
On 18.02.23 22:14, Rick Edgecombe wrote:
> From: Yu-cheng Yu <yu-cheng.yu@...el.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@...el.com>
> Tested-by: John Allen <john.allen@....com>
> Reviewed-by: Kees Cook <keescook@...omium.org>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@...el.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.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.
--
Thanks,
David / dhildenb
Powered by blists - more mailing lists