Messages in this thread | | | Date | Tue, 21 Feb 2023 09:42:04 +0100 | Subject | Re: [PATCH v6 24/41] mm: Don't allow write GUPs to shadow stack memory | From | David Hildenbrand <> |
| |
On 18.02.23 22:14, Rick Edgecombe wrote: > The x86 Control-flow Enforcement Technology (CET) feature includes a new > type of memory called shadow stack. This shadow stack memory has some > unusual properties, which requires some core mm changes to function > properly. > > Shadow stack memory is writable only in very specific, controlled ways. > However, since it is writable, the kernel treats it as such. As a result > there remain many ways for userspace to trigger the kernel to write to > shadow stack's via get_user_pages(, FOLL_WRITE) operations. To make this a > little less exposed, block writable GUPs for shadow stack VMAs. > > Still allow FOLL_FORCE to write through shadow stack protections, as it > does for read-only protections. > > Reviewed-by: Kees Cook <keescook@chromium.org> > Tested-by: Pengfei Xu <pengfei.xu@intel.com> > Tested-by: John Allen <john.allen@amd.com> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com> > > --- > v3: > - Add comment in __pte_access_permitted() (Dave) > - Remove unneeded shadow stack specific check in > __pte_access_permitted() (Jann) > --- > arch/x86/include/asm/pgtable.h | 5 +++++ > mm/gup.c | 2 +- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h > index 6b7106457bfb..20d0df494269 100644 > --- a/arch/x86/include/asm/pgtable.h > +++ b/arch/x86/include/asm/pgtable.h > @@ -1641,6 +1641,11 @@ static inline bool __pte_access_permitted(unsigned long pteval, bool write) > { > unsigned long need_pte_bits = _PAGE_PRESENT|_PAGE_USER; > > + /* > + * Write=0,Dirty=1 PTEs are shadow stack, which the kernel > + * shouldn't generally allow access to, but since they > + * are already Write=0, the below logic covers both cases. > + */ > if (write) > need_pte_bits |= _PAGE_RW;
So, GUP fast will always fail when writing ...
> > diff --git a/mm/gup.c b/mm/gup.c > index f45a3a5be53a..bfd33d9edb89 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -982,7 +982,7 @@ static int check_vma_flags(struct vm_area_struct *vma, unsigned long gup_flags) > return -EFAULT; > > if (write) { > - if (!(vm_flags & VM_WRITE)) { > + if (!(vm_flags & VM_WRITE) || (vm_flags & VM_SHADOW_STACK)) { > if (!(gup_flags & FOLL_FORCE)) > return -EFAULT; > /* hugetlb does not support FOLL_FORCE|FOLL_WRITE. */
and ordinary GUP without FOLL_FORCE.
Acked-by: David Hildenbrand <david@redhat.com>
-- Thanks,
David / dhildenb
| |