The patch titled
Subject: mm/khugepaged: fix collapse_pte_mapped_thp() versus uffd
has been added to the -mm mm-stable branch. Its filename is
mm-khugepaged-fix-collapse_pte_mapped_thp-versus-uffd.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-khugepaged-fix-collapse_pte_mapped_thp-versus-uffd.patch
This patch will later appear in the mm-stable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Hugh Dickins <hughd@xxxxxxxxxx>
Subject: mm/khugepaged: fix collapse_pte_mapped_thp() versus uffd
Date: Mon, 21 Aug 2023 12:51:20 -0700 (PDT)
Jann Horn demonstrated how userfaultfd ioctl UFFDIO_COPY into a private
shmem mapping can add valid PTEs to page table collapse_pte_mapped_thp()
thought it had emptied: page lock on the huge page is enough to protect
against WP faults (which find the PTE has been cleared), but not enough to
protect against userfaultfd. "BUG: Bad rss-counter state" followed.
retract_page_tables() protects against this by checking !vma->anon_vma;
but we know that MADV_COLLAPSE needs to be able to work on private shmem
mappings, even those with an anon_vma prepared for another part of the
mapping; and we know that MADV_COLLAPSE needs to work on shared shmem
mappings which are userfaultfd_armed(). Whether it needs to work on
private shmem mappings which are userfaultfd_armed(), I'm not so sure: but
assume that it does.
Just for this case, take the pmd_lock() two steps earlier: not because it
gives any protection against this case itself, but because ptlock nests
inside it, and it's the dropping of ptlock which let the bug in. In other
cases, continue to minimize the pmd_lock() hold time.
Link: https://lkml.kernel.org/r/4d31abf5-56c0-9f3d-d12f-c9317936691@xxxxxxxxxx
Fixes: 1043173eb5eb ("mm/khugepaged: collapse_pte_mapped_thp() with mmap_read_lock()")
Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx>
Reported-by: Jann Horn <jannh@xxxxxxxxxx>
Closes: https://lore.kernel.org/linux-mm/CAG48ez0FxiRC4d3VTu_a9h=rg5FW-kYD5Rg5xo_RDBM0LTTqZQ@xxxxxxxxxxxxxx/
Acked-by: Peter Xu <peterx@xxxxxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
mm/khugepaged.c | 38 +++++++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 9 deletions(-)
--- a/mm/khugepaged.c~mm-khugepaged-fix-collapse_pte_mapped_thp-versus-uffd
+++ a/mm/khugepaged.c
@@ -1476,7 +1476,7 @@ int collapse_pte_mapped_thp(struct mm_st
struct page *hpage;
pte_t *start_pte, *pte;
pmd_t *pmd, pgt_pmd;
- spinlock_t *pml, *ptl;
+ spinlock_t *pml = NULL, *ptl;
int nr_ptes = 0, result = SCAN_FAIL;
int i;
@@ -1572,9 +1572,25 @@ int collapse_pte_mapped_thp(struct mm_st
haddr, haddr + HPAGE_PMD_SIZE);
mmu_notifier_invalidate_range_start(&range);
notified = true;
- start_pte = pte_offset_map_lock(mm, pmd, haddr, &ptl);
+
+ /*
+ * pmd_lock covers a wider range than ptl, and (if split from mm's
+ * page_table_lock) ptl nests inside pml. The less time we hold pml,
+ * the better; but userfaultfd's mfill_atomic_pte() on a private VMA
+ * inserts a valid as-if-COWed PTE without even looking up page cache.
+ * So page lock of hpage does not protect from it, so we must not drop
+ * ptl before pgt_pmd is removed, so uffd private needs pml taken now.
+ */
+ if (userfaultfd_armed(vma) && !(vma->vm_flags & VM_SHARED))
+ pml = pmd_lock(mm, pmd);
+
+ start_pte = pte_offset_map_nolock(mm, pmd, haddr, &ptl);
if (!start_pte) /* mmap_lock + page lock should prevent this */
goto abort;
+ if (!pml)
+ spin_lock(ptl);
+ else if (ptl != pml)
+ spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
/* step 2: clear page table and adjust rmap */
for (i = 0, addr = haddr, pte = start_pte;
@@ -1608,7 +1624,9 @@ int collapse_pte_mapped_thp(struct mm_st
nr_ptes++;
}
- pte_unmap_unlock(start_pte, ptl);
+ pte_unmap(start_pte);
+ if (!pml)
+ spin_unlock(ptl);
/* step 3: set proper refcount and mm_counters. */
if (nr_ptes) {
@@ -1616,12 +1634,12 @@ int collapse_pte_mapped_thp(struct mm_st
add_mm_counter(mm, mm_counter_file(hpage), -nr_ptes);
}
- /* step 4: remove page table */
-
- /* Huge page lock is still held, so page table must remain empty */
- pml = pmd_lock(mm, pmd);
- if (ptl != pml)
- spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
+ /* step 4: remove empty page table */
+ if (!pml) {
+ pml = pmd_lock(mm, pmd);
+ if (ptl != pml)
+ spin_lock_nested(ptl, SINGLE_DEPTH_NESTING);
+ }
pgt_pmd = pmdp_collapse_flush(vma, haddr, pmd);
pmdp_get_lockless_sync();
if (ptl != pml)
@@ -1648,6 +1666,8 @@ abort:
}
if (start_pte)
pte_unmap_unlock(start_pte, ptl);
+ if (pml && pml != ptl)
+ spin_unlock(pml);
if (notified)
mmu_notifier_invalidate_range_end(&range);
drop_hpage:
_
Patches currently in -mm which might be from hughd@xxxxxxxxxx are
shmem-fix-smaps-bug-sleeping-while-atomic.patch
mm-khugepaged-fix-collapse_pte_mapped_thp-versus-uffd.patch
