From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 54F9A46D2D3; Thu, 15 Oct 2026 08:34:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1792053248; cv=none; b=rVbyenrdGd1ndvmsTAlSGkdL8tKrwacNf5eWblCMDgH5irpGrQYJ73Req86WHhW4oiSKMcI8lk5CsXmPK53LB/VeAxQE/+tkrvC/Cg4mdIinvVqYxw3HlgpZsMrNJw72CkrqTt/zOkuaBLwoBjc4nC4Dtb68phFctByOlVU/XY4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1792053248; c=relaxed/simple; bh=1lje05dHeTbxWp3eklRDl2JzKlfVPd+4rryEkd4m0DI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Hfxk1mQ/OR/yA5w0/Sk47CF7IefX0PKFslMeDiUSHz4LnyyjbQG5/3u/aWIzXWIDG94tx0FoWzLWS6M0OsypxPoZs0pX+XdsNI24rkcvTzrSD+pPLH53LTJeEqGg5WUtTxsDWaslfYVhgn57DQDrnQ8UAm37Uju+tkI6VkMa31M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=aaZgNvMA; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="aaZgNvMA" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6D8351F000C4; Thu, 15 Oct 2026 08:34:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1792053246; bh=1y2O0PSZS5Bzt/8EZ2Y7b6A1XzbAJe0MR4Va/N0YUuU=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=aaZgNvMANpmUBSrs1hVX5dOBdZ5ubITsPMpzYC6ohqDfmcGNBK0HFP5IIu9enoq6P M90gmP2DGfFq7fJBhjZXldmj7fCOobTX9AHeFQzYBu4OsHOqAPJJq89m5d2JBMLE6d IX1ZzHm67oSWuDWCK9DW06xW9l7a/FAvXx9Q0KKpdbq+YUXOZRCJUzJGIofXanWSnG iZqjvsZlDyceUmFP40JWDI0N5xbgR0TDu1XDbh4oGorzXB8pv2C4VsUYL7MtPYDwtv 9k/hkPAULu7KSk8UF8gOha7el2Ie8z8fXbCfvm6aqqNh3aAxRqU6ts/NigbqVxw8SM fCAorPY+owbwQ== Date: Thu, 15 Oct 2026 11:33:58 +0300 From: Mike Rapoport To: "David Hildenbrand (Arm)" Cc: Muchun Song , Oscar Salvador , Andrew Morton , Peter Xu , Miaohe Lin , Lokesh Gidra , "Liam R. Howlett" , Kyle Zeng , Lorenzo Stoakes , linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] mm/userfaultfd: only allocate anon_vma in CoW mappings Message-ID: References: <20261014-copy_hugetlb_page_range-v1-1-5b68e03fe7fe@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20261014-copy_hugetlb_page_range-v1-1-5b68e03fe7fe@kernel.org> On Wed, Oct 14, 2026 at 01:53:11PM +0200, David Hildenbrand (Arm) wrote: > find_vma_and_prepare_anon() can end up allocating an anon_vma in VMAs that > are not CoW mappings, which is wrong. > > If we have a MAP_SHARED R/O file mapped, we lack VM_SHARED (but do have > VM_MAYSHARE) and lack VM_MAYWRITE. > > While uffd later rejects such mappings (uffd registration requires > VM_MAYWRITE, see comment in validate_dst_vma) find_vma_and_prepare_anon() > still wrongly allocates an anon vma before it validates the vma. > > This in turn later confuses copy_page_range()->vma_needs_copy() such > that we end up calling copy_hugetlb_page_range() with ranges that > actually do not contain *any* per-pagetable information that would > require copying, which is the basis for PMD-table sharing in the first > place. > > copy_hugetlb_page_range() should not have to worry about shared > PMD tables at all: it doesn't make any sense to copy page tables that are > shared, as they can be fully recreated later. Ending up in the function > with shareable page tables is the real issue. > > If a destination PMD table is shared in copy_hugetlb_page_range(), it > might get concurrently unshared as we are not holding relevant > locks, resulting in us using a page table that is no longer mapped and > possibly freed after calling: > > dst_pte = huge_pte_alloc(dst, dst_vma, addr, sz); > > To fix it, do not allocate an anon_vma in non-CoW mappings and add a > safety guard to copy_hugetlb_page_range(), making sure that we never > end up messing with shared PMD tables in there. > > Before commit 867a43a34ff8a ("userfaultfd: use per-vma locks in userfaultfd > operations") we would always validate the VMA before allocating an > anon_vma, and consequently never wrongly allocate an anon_vma. > > As uffd_lock_vma() is part of the same lookup machinery and > also contains a misleading VM_SHARED check, change that one as well > directly for consistency. > > A follow-up cleanup will stop taking the hugetlb_vma_lock_read(src_vma) > lock in copy_hugetlb_page_range() entirely. Further, the usage of > VM_SHARED in userfaultfd code should be cleaned up later to focus on > CoW vs. non-CoW mappings. > > Reported-by: Kyle Zeng > Fixes: 867a43a34ff8a ("userfaultfd: use per-vma locks in userfaultfd operations") > Cc: stable@vger.kernel.org > Signed-off-by: David Hildenbrand (Arm) Acked-by: Mike Rapoport (Microsoft) > --- > mm/hugetlb.c | 24 ++++++++++++------------ > mm/userfaultfd.c | 5 ++--- > 2 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/mm/hugetlb.c b/mm/hugetlb.c > index 63dc836061c99..60f9b209c8db1 100644 > --- a/mm/hugetlb.c > +++ b/mm/hugetlb.c > @@ -4984,10 +4984,11 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, > raw_write_seqcount_begin(&src->write_protect_seq); > } else { > /* > - * For shared mappings the vma lock must be held before > - * calling hugetlb_walk() in the src vma. Otherwise, the > - * returned ptep could go away if part of a shared pmd and > - * another thread calls huge_pmd_unshare. > + * Shared mappings only need copying when we could have > + * information stored in page tables (e.g., userfaultfd PTE > + * bits or markers), which is incompatible with PMD sharing. > + * Still take the hugetlb VMA lock for now to satisfy > + * hugetlb_walk()'s locking requirements. > */ > hugetlb_vma_lock_read(src_vma); > } > @@ -4995,6 +4996,13 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, > last_addr_mask = hugetlb_mask_last_page(h); > for (addr = src_vma->vm_start; addr < src_vma->vm_end; addr += sz) { > spinlock_t *src_ptl, *dst_ptl; > + > + if (WARN_ON_ONCE(want_pmd_share(src_vma, addr) || > + want_pmd_share(dst_vma, addr))) { > + ret = -EINVAL; > + break; > + } > + > src_pte = hugetlb_walk(src_vma, addr, sz); > if (!src_pte) { > addr |= last_addr_mask; > @@ -5006,14 +5014,6 @@ int copy_hugetlb_page_range(struct mm_struct *dst, struct mm_struct *src, > break; > } > > -#ifdef CONFIG_HUGETLB_PMD_PAGE_TABLE_SHARING > - /* If the pagetables are shared, there is nothing to do */ > - if (ptdesc_pmd_is_shared(virt_to_ptdesc(dst_pte))) { > - addr |= last_addr_mask; > - continue; > - } > -#endif > - > dst_ptl = huge_pte_lock(h, dst, dst_pte); > src_ptl = huge_pte_lockptr(h, src, src_pte); > spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING); > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c > index 0e0fe9bfef239..82b56a58fc40d 100644 > --- a/mm/userfaultfd.c > +++ b/mm/userfaultfd.c > @@ -115,8 +115,7 @@ struct vm_area_struct *find_vma_and_prepare_anon(struct mm_struct *mm, > vma = vma_lookup(mm, addr); > if (!vma) > vma = ERR_PTR(-ENOENT); > - else if (!(vma->vm_flags & VM_SHARED) && > - unlikely(anon_vma_prepare(vma))) > + else if (vma_is_cow_mapping(vma) && unlikely(anon_vma_prepare(vma))) > vma = ERR_PTR(-ENOMEM); > > return vma; > @@ -145,7 +144,7 @@ static struct vm_area_struct *uffd_lock_vma(struct mm_struct *mm, > * We know we're going to need an anon rmap, so check that > * early. > */ > - if (!(vma->vm_flags & VM_SHARED) && unlikely(!vma_has_anon_rmap(vma))) > + if (vma_is_cow_mapping(vma) && unlikely(!vma_has_anon_rmap(vma))) > vma_end_read(vma); > else > return vma; > > --- > > base-commit: 1d0b04a60516de48250b04227dcf483c6c09aab1 > > change-id: 20261014-copy_hugetlb_page_range-743d71a2980e > > -- > > Cheers, > > David > -- Sincerely yours, Mike.