[PATCH] mm/hugetlb: Fix race condition of uffd missing handling

[Peter Xu]

Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
---
 mm/hugetlb.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index dd29cba46e9e..5015d8aa5da4 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -5557,9 +5557,39 @@ static vm_fault_t hugetlb_no_page(struct mm_struct *mm,
 	if (!page) {
 		/* Check for page in userfault range */
 		if (userfaultfd_missing(vma)) {
-			ret = hugetlb_handle_userfault(vma, mapping, idx,
-						       flags, haddr, address,
-						       VM_UFFD_MISSING);
+			bool same;
+
+			/*
+			 * Since hugetlb_no_page() was examining pte
+			 * without pgtable lock, we need to re-test under
+			 * lock because the pte may not be stable and could
+			 * have changed from under us.  Try to detect
+			 * either changed or during-changing ptes and bail
+			 * out properly.
+			 *
+			 * One example of changing pte is in-progress CoW
+			 * of private mapping, which will clear+flush pte
+			 * then reinstall the new one.
+			 *
+			 * Note that userfaultfd is actually fine with
+			 * false positives (e.g. caused by pte changed),
+			 * but not wrong logical events (e.g. caused by
+			 * reading a pte during changing).  The latter can
+			 * confuse the userspace, so the strictness is very
+			 * much preferred.  E.g., MISSING event should
+			 * never happen on the page after UFFDIO_COPY has
+			 * correctly installed the page and returned.
+			 */
+			ptl = huge_pte_lock(h, mm, ptep);
+			same = pte_same(huge_ptep_get(ptep), old_pte);
+			spin_unlock(ptl);
+			if (same)
+				ret = hugetlb_handle_userfault(vma, mapping, idx,
+							       flags, haddr, address,
+							       VM_UFFD_MISSING);
+			else
+				/* PTE changed or unstable, retry */
+				ret = 0;
 			goto out;
 		}
 
-- 
2.37.3


--/2ttB/dmRH7GVaAc--