Skip to content

Commit

Permalink
mm/gup: trigger FAULT_FLAG_UNSHARE when R/O-pinning a possibly shared…
Browse files Browse the repository at this point in the history
… anonymous page

Whenever GUP currently ends up taking a R/O pin on an anonymous page that
might be shared -- mapped R/O and !PageAnonExclusive() -- any write fault
on the page table entry will end up replacing the mapped anonymous page
due to COW, resulting in the GUP pin no longer being consistent with the
page actually mapped into the page table.

The possible ways to deal with this situation are:
 (1) Ignore and pin -- what we do right now.
 (2) Fail to pin -- which would be rather surprising to callers and
     could break user space.
 (3) Trigger unsharing and pin the now exclusive page -- reliable R/O
     pins.

Let's implement 3) because it provides the clearest semantics and allows
for checking in unpin_user_pages() and friends for possible BUGs: when
trying to unpin a page that's no longer exclusive, clearly something went
very wrong and might result in memory corruptions that might be hard to
debug.  So we better have a nice way to spot such issues.

This change implies that whenever user space *wrote* to a private mapping
(IOW, we have an anonymous page mapped), that GUP pins will always remain
consistent: reliable R/O GUP pins of anonymous pages.

As a side note, this commit fixes the COW security issue for hugetlb with
FOLL_PIN as documented in:
  https://lore.kernel.org/r/3ae33b08-d9ef-f846-56fb-645e3b9b4c66@redhat.com
The vmsplice reproducer still applies, because vmsplice uses FOLL_GET
instead of FOLL_PIN.

Note that follow_huge_pmd() doesn't apply because we cannot end up in
there with FOLL_PIN.

This commit is heavily based on prototype patches by Andrea.

Link: https://lkml.kernel.org/r/20220428083441.37290-17-david@redhat.com
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
Co-developed-by: Andrea Arcangeli <aarcange@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: David Rientjes <rientjes@google.com>
Cc: Don Dutile <ddutile@redhat.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jann Horn <jannh@google.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Cc: Khalid Aziz <khalid.aziz@oracle.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Liang Zhang <zhangliang5@huawei.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Mike Rapoport <rppt@linux.ibm.com>
Cc: Nadav Amit <namit@vmware.com>
Cc: Oded Gabbay <oded.gabbay@gmail.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Pedro Demarchi Gomes <pedrodemargomes@gmail.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Rik van Riel <riel@surriel.com>
Cc: Roman Gushchin <guro@fb.com>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: Yang Shi <shy828301@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
  • Loading branch information
davidhildenbrand authored and akpm00 committed May 10, 2022
1 parent c89357e commit a7f2266
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 6 deletions.
39 changes: 39 additions & 0 deletions include/linux/mm.h
Original file line number Diff line number Diff line change
Expand Up @@ -3002,6 +3002,45 @@ static inline int vm_fault_to_errno(vm_fault_t vm_fault, int foll_flags)
return 0;
}

/*
* Indicates for which pages that are write-protected in the page table,
* whether GUP has to trigger unsharing via FAULT_FLAG_UNSHARE such that the
* GUP pin will remain consistent with the pages mapped into the page tables
* of the MM.
*
* Temporary unmapping of PageAnonExclusive() pages or clearing of
* PageAnonExclusive() has to protect against concurrent GUP:
* * Ordinary GUP: Using the PT lock
* * GUP-fast and fork(): mm->write_protect_seq
* * GUP-fast and KSM or temporary unmapping (swap, migration):
* clear/invalidate+flush of the page table entry
*
* Must be called with the (sub)page that's actually referenced via the
* page table entry, which might not necessarily be the head page for a
* PTE-mapped THP.
*/
static inline bool gup_must_unshare(unsigned int flags, struct page *page)
{
/*
* FOLL_WRITE is implicitly handled correctly as the page table entry
* has to be writable -- and if it references (part of) an anonymous
* folio, that part is required to be marked exclusive.
*/
if ((flags & (FOLL_WRITE | FOLL_PIN)) != FOLL_PIN)
return false;
/*
* Note: PageAnon(page) is stable until the page is actually getting
* freed.
*/
if (!PageAnon(page))
return false;
/*
* Note that PageKsm() pages cannot be exclusive, and consequently,
* cannot get pinned.
*/
return !PageAnonExclusive(page);
}

typedef int (*pte_fn_t)(pte_t *pte, unsigned long addr, void *data);
extern int apply_to_page_range(struct mm_struct *mm, unsigned long address,
unsigned long size, pte_fn_t fn, void *data);
Expand Down
42 changes: 39 additions & 3 deletions mm/gup.c
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,10 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,
}
}

if (!pte_write(pte) && gup_must_unshare(flags, page)) {
page = ERR_PTR(-EMLINK);
goto out;
}
/* try_grab_page() does nothing unless FOLL_GET or FOLL_PIN is set. */
if (unlikely(!try_grab_page(page, flags))) {
page = ERR_PTR(-ENOMEM);
Expand Down Expand Up @@ -732,6 +736,11 @@ static struct page *follow_p4d_mask(struct vm_area_struct *vma,
* When getting pages from ZONE_DEVICE memory, the @ctx->pgmap caches
* the device's dev_pagemap metadata to avoid repeating expensive lookups.
*
* When getting an anonymous page and the caller has to trigger unsharing
* of a shared anonymous page first, -EMLINK is returned. The caller should
* trigger a fault with FAULT_FLAG_UNSHARE set. Note that unsharing is only
* relevant with FOLL_PIN and !FOLL_WRITE.
*
* On output, the @ctx->page_mask is set according to the size of the page.
*
* Return: the mapped (struct page *), %NULL if no mapping exists, or
Expand Down Expand Up @@ -855,7 +864,8 @@ static int get_gate_page(struct mm_struct *mm, unsigned long address,
* is, *@locked will be set to 0 and -EBUSY returned.
*/
static int faultin_page(struct vm_area_struct *vma,
unsigned long address, unsigned int *flags, int *locked)
unsigned long address, unsigned int *flags, bool unshare,
int *locked)
{
unsigned int fault_flags = 0;
vm_fault_t ret;
Expand All @@ -877,6 +887,11 @@ static int faultin_page(struct vm_area_struct *vma,
*/
fault_flags |= FAULT_FLAG_TRIED;
}
if (unshare) {
fault_flags |= FAULT_FLAG_UNSHARE;
/* FAULT_FLAG_WRITE and FAULT_FLAG_UNSHARE are incompatible */
VM_BUG_ON(fault_flags & FAULT_FLAG_WRITE);
}

ret = handle_mm_fault(vma, address, fault_flags, NULL);
if (ret & VM_FAULT_ERROR) {
Expand Down Expand Up @@ -1098,8 +1113,9 @@ static long __get_user_pages(struct mm_struct *mm,
cond_resched();

page = follow_page_mask(vma, start, foll_flags, &ctx);
if (!page) {
ret = faultin_page(vma, start, &foll_flags, locked);
if (!page || PTR_ERR(page) == -EMLINK) {
ret = faultin_page(vma, start, &foll_flags,
PTR_ERR(page) == -EMLINK, locked);
switch (ret) {
case 0:
goto retry;
Expand Down Expand Up @@ -2201,6 +2217,11 @@ static int gup_pte_range(pmd_t pmd, unsigned long addr, unsigned long end,
goto pte_unmap;
}

if (!pte_write(pte) && gup_must_unshare(flags, page)) {
gup_put_folio(folio, 1, flags);
goto pte_unmap;
}

/*
* We need to make the page accessible if and only if we are
* going to access its content (the FOLL_PIN case). Please
Expand Down Expand Up @@ -2381,6 +2402,11 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
return 0;
}

if (!pte_write(pte) && gup_must_unshare(flags, &folio->page)) {
gup_put_folio(folio, refs, flags);
return 0;
}

*nr += refs;
folio_set_referenced(folio);
return 1;
Expand Down Expand Up @@ -2442,6 +2468,11 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
return 0;
}

if (!pmd_write(orig) && gup_must_unshare(flags, &folio->page)) {
gup_put_folio(folio, refs, flags);
return 0;
}

*nr += refs;
folio_set_referenced(folio);
return 1;
Expand Down Expand Up @@ -2477,6 +2508,11 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
return 0;
}

if (!pud_write(orig) && gup_must_unshare(flags, &folio->page)) {
gup_put_folio(folio, refs, flags);
return 0;
}

*nr += refs;
folio_set_referenced(folio);
return 1;
Expand Down
3 changes: 3 additions & 0 deletions mm/huge_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -1389,6 +1389,9 @@ struct page *follow_trans_huge_pmd(struct vm_area_struct *vma,
page = pmd_page(*pmd);
VM_BUG_ON_PAGE(!PageHead(page) && !is_zone_device_page(page), page);

if (!pmd_write(*pmd) && gup_must_unshare(flags, page))
return ERR_PTR(-EMLINK);

if (!try_grab_page(page, flags))
return ERR_PTR(-ENOMEM);

Expand Down
27 changes: 24 additions & 3 deletions mm/hugetlb.c
Original file line number Diff line number Diff line change
Expand Up @@ -5982,6 +5982,25 @@ static void record_subpages_vmas(struct page *page, struct vm_area_struct *vma,
}
}

static inline bool __follow_hugetlb_must_fault(unsigned int flags, pte_t *pte,
bool *unshare)
{
pte_t pteval = huge_ptep_get(pte);

*unshare = false;
if (is_swap_pte(pteval))
return true;
if (huge_pte_write(pteval))
return false;
if (flags & FOLL_WRITE)
return true;
if (gup_must_unshare(flags, pte_page(pteval))) {
*unshare = true;
return true;
}
return false;
}

long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
struct page **pages, struct vm_area_struct **vmas,
unsigned long *position, unsigned long *nr_pages,
Expand All @@ -5996,6 +6015,7 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
while (vaddr < vma->vm_end && remainder) {
pte_t *pte;
spinlock_t *ptl = NULL;
bool unshare = false;
int absent;
struct page *page;

Expand Down Expand Up @@ -6046,16 +6066,17 @@ long follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma,
* both cases, and because we can't follow correct pages
* directly from any kind of swap entries.
*/
if (absent || is_swap_pte(huge_ptep_get(pte)) ||
((flags & FOLL_WRITE) &&
!huge_pte_write(huge_ptep_get(pte)))) {
if (absent ||
__follow_hugetlb_must_fault(flags, pte, &unshare)) {
vm_fault_t ret;
unsigned int fault_flags = 0;

if (pte)
spin_unlock(ptl);
if (flags & FOLL_WRITE)
fault_flags |= FAULT_FLAG_WRITE;
else if (unshare)
fault_flags |= FAULT_FLAG_UNSHARE;
if (locked)
fault_flags |= FAULT_FLAG_ALLOW_RETRY |
FAULT_FLAG_KILLABLE;
Expand Down

0 comments on commit a7f2266

Please sign in to comment.