On 27.03.23 20:34, Mike Kravetz wrote:
On 03/26/23 10:46, Peter Xu wrote:
On Fri, Mar 24, 2023 at 11:36:53PM +0100, David Hildenbrand wrote:
@@ -5487,6 +5487,17 @@ static vm_fault_t hugetlb_wp(struct mm_struct *mm, struct vm_area_struct *vma,
unsigned long haddr = address & huge_page_mask(h);
struct mmu_notifier_range range;
+ /*
+ * Never handle CoW for uffd-wp protected pages. It should be only
+ * handled when the uffd-wp protection is removed.
+ *
+ * Note that only the CoW optimization path (in hugetlb_no_page())
+ * can trigger this, because hugetlb_fault() will always resolve
+ * uffd-wp bit first.
+ */
+ if (!unshare && huge_pte_uffd_wp(pte))
+ return 0;
This looks correct. However, since the previous version looked correct I must
ask. Can we have unshare set and huge_pte_uffd_wp true? If so, then it seems
we would need to possibly propogate that uffd_wp to the new pte as in v2
Good point, thanks for spotting!
We can. A reproducer would share an anon hugetlb page because parent and
child. In the parent, we would uffd-wp that page. We could trigger unsharing
by R/O-pinning that page.
Right. This seems to be a separate bug.. It should be triggered in
totally different context and much harder due to rare use of RO pins,
meanwhile used with userfault-wp.
If both of you agree, I can prepare a separate patch for this bug, and I'll
better prepare a reproducer/selftest with it.
I am OK with separate patches, and agree that the R/O pinning case is less
likely to happen.
Yes, the combination should be rather rare and we can fix that
separately. Ideally, we'd try to mimic the same uffd code flow in
hugetlb cow/unshare handling that we use in memory.c
Since this patch addresses the issue found by Muhammad,
Reviewed-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
Hopefully we didn't forget about yet another case :D
Acked-by: David Hildenbrand <david@xxxxxxxxxx>
--
Thanks,
David / dhildenb
