| Message ID | 20230417195317.898696-3-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/hugetlb: More fixes around uffd-wp vs fork() / RO pins | expand |
On Mon, 17 Apr 2023 15:53:13 -0400 Peter Xu <peterx@redhat.com> wrote: > When we try to unshare a pinned page for a private hugetlb, uffd-wp bit can > get lost during unsharing. Fix it by carrying it over. > > This should be very rare, only if an unsharing happened on a private > hugetlb page with uffd-wp protected (e.g. in a child which shares the same > page with parent with UFFD_FEATURE_EVENT_FORK enabled). What are the user-visible consequences of the bug? > Cc: linux-stable <stable@vger.kernel.org> When proposing a backport, it's better to present the patch as a standalone thing, against current -linus. I'll then queue it in mm-hotfixes and shall send it upstream during this -rc cycle. As presented, this patch won't go upstream until after 6.3 is released, and as it comes later in time, more backporting effort might be needed. I can rework things if this fix is reasonably urgent (the "user-visible consequences" info is the guide). If not urgent, we can leave things as they are.
Hi, Andrew, On Mon, Apr 17, 2023 at 04:48:22PM -0700, Andrew Morton wrote: > On Mon, 17 Apr 2023 15:53:13 -0400 Peter Xu <peterx@redhat.com> wrote: > > > When we try to unshare a pinned page for a private hugetlb, uffd-wp bit can > > get lost during unsharing. Fix it by carrying it over. > > > > This should be very rare, only if an unsharing happened on a private > > hugetlb page with uffd-wp protected (e.g. in a child which shares the same > > page with parent with UFFD_FEATURE_EVENT_FORK enabled). > > What are the user-visible consequences of the bug? When above condition met, one can lose uffd-wp bit on the privately mapped hugetlb page. It allows the page to be writable even if it should still be wr-protected. I assume it can mean data loss. However it's very hard to trigger. When I wrote the reproducer (provided in the last patch) I needed to use the newest gup_test cmd introduced by David to trigger it because I don't even know another way to do a proper RO longerm pin. Besides that, it needs a bunch of other conditions all met: (1) hugetlb being mapped privately, (2) userfaultfd registered with WP and EVENT_FORK, (3) the user app fork()s, then, (4) RO longterm pin onto a wr-protected anonymous page. If it's not impossible to hit in production I'd say extremely rare. > > > Cc: linux-stable <stable@vger.kernel.org> > > When proposing a backport, it's better to present the patch as a > standalone thing, against current -linus. I'll then queue it in > mm-hotfixes and shall send it upstream during this -rc cycle. > > As presented, this patch won't go upstream until after 6.3 is released, > and as it comes later in time, more backporting effort might be needed. > > I can rework things if this fix is reasonably urgent (the "user-visible > consequences" info is the guide). If not urgent, we can leave things > as they are. IMHO it's not urgent so suitable for mm-unstable (current base of this set; sorry if I forgot to mention it explicitly). I'll post (and remember to post) patches on top of mm-stable if they're urgent, or e.g. bugs introduced in current release. I copied stable for the pure logic of fixing a bug in old kernels. The consequence of hitting the bug is very bad but chance to hit is very low. Thanks,
diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 0213efaf31be..cd3a9d8f4b70 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -5637,13 +5637,16 @@ static vm_fault_t hugetlb_wp(struct mm_struct *mm, struct vm_area_struct *vma, spin_lock(ptl); ptep = hugetlb_walk(vma, haddr, huge_page_size(h)); if (likely(ptep && pte_same(huge_ptep_get(ptep), pte))) { + pte_t newpte = make_huge_pte(vma, &new_folio->page, !unshare); + /* Break COW or unshare */ huge_ptep_clear_flush(vma, haddr, ptep); mmu_notifier_invalidate_range(mm, range.start, range.end); page_remove_rmap(old_page, vma, true); hugepage_add_new_anon_rmap(new_folio, vma, haddr); - set_huge_pte_at(mm, haddr, ptep, - make_huge_pte(vma, &new_folio->page, !unshare)); + if (huge_pte_uffd_wp(pte)) + newpte = huge_pte_mkuffd_wp(newpte); + set_huge_pte_at(mm, haddr, ptep, newpte); folio_set_hugetlb_migratable(new_folio); /* Make the old page be freed below */ new_folio = page_folio(old_page);
When we try to unshare a pinned page for a private hugetlb, uffd-wp bit can get lost during unsharing. Fix it by carrying it over. This should be very rare, only if an unsharing happened on a private hugetlb page with uffd-wp protected (e.g. in a child which shares the same page with parent with UFFD_FEATURE_EVENT_FORK enabled). Cc: linux-stable <stable@vger.kernel.org> Fixes: 166f3ecc0daf ("mm/hugetlb: hook page faults for uffd write protection") Reported-by: Mike Kravetz <mike.kravetz@oracle.com> Reviewed-by: David Hildenbrand <david@redhat.com> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com> Signed-off-by: Peter Xu <peterx@redhat.com> --- mm/hugetlb.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)