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@oracle.com
Hopefully we didn't forget about yet another case :D
Acked-by: David Hildenbrand david@redhat.com