From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 5CBFB1F936 for ; Thu, 2 Apr 2026 03:58:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775102320; cv=none; b=kEc/tF2pDsiihqZQDuTDuH3KI8suJtrSX9+x0CY3U/QErkSxLrHEopF8UVe8QIpNvrD830dq3ongVvHOBqbNN6UEVm7bd7OwqQFjjfLHWFn2MfSD1mHAUgdQnnjwnlmNLnZE9g4qvu63V2NovWRd2Q9pd4X8+6XQuoNWy2SFIfQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775102320; c=relaxed/simple; bh=zmuodQKqPXnJljN5/Z/DtZcXm4ZNdE8k6tsXe4+G1cI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=X+jQLJ+WWS0Ll45o0zvb5XQCn0eOyOwZqSEWBh9LCKi/Gu4C9JpS4axZ7g/WbNKozavKcysLbd1AmYNERwsMjDtHKURYrf+iDxWDvKFrvHOZWaY+4lCjN65lzGBWCvUwNyRaKcjFVWgmLq+dDSDNa2HRXoGr9ugWOWXYkDCNfig= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=tRm1Dv2H; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="tRm1Dv2H" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 46B7BC2BCAF; Thu, 2 Apr 2026 03:58:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1775102320; bh=zmuodQKqPXnJljN5/Z/DtZcXm4ZNdE8k6tsXe4+G1cI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=tRm1Dv2HmLCxo2uNB6VvMEFQo+V7D47Hcx80cMrWftIsXT20xG41Y8n1kHimrKZVJ 4ql3gx6G8X7ICK5gpKMx7CQqxSesYY/QknDH5RP4F1sV+vvQ6T434en11R+GrqU5m1 edB3D+ZABFVSpv9+/1t1QN+edIGFzNjspP6Vy/k3shYFUbcF4/giu71eBBUW26uVcK X0Hc30lInPezEyDmcoxe+3/IznjWVJyP0IH3DwhXlv7SBDrxPVZJYW2kleKc0o1MP6 2u2qcww/wm8wvFLzlI7+Lths10620sqNiFyYd3KrALXHTjuSuyZM0CiwSLKAuDnPFG BgZ+wf7pRNQbQ== Date: Thu, 2 Apr 2026 06:58:33 +0300 From: Mike Rapoport To: David CARLIER Cc: Andrew Morton , Peter Xu , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Lorenzo Stoakes , "Liam R. Howlett" , Vlastimil Babka Subject: Re: [PATCH v4] mm/userfaultfd: detect VMA replacement after copy retry in mfill_copy_folio_retry() Message-ID: References: <20260331134158.622084-1-devnexen@gmail.com> <20260331200148.cc0c95deaf070579a68af041@linux-foundation.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: Hi David, It feels that you use an LLM for correspondence. Please tune it down to produce more laconic and to the point responses. On Wed, Apr 01, 2026 at 09:06:36AM +0100, David CARLIER wrote: > On Tue, Apr 01, 2026 at 08:49:00AM +0300, Mike Rapoport wrote: > > What does "folio allocated from the original VMA's backing store" exactly > > mean? Why is this a problem? > > Fair point, the commit message was vague here. What I meant is: > > mfill_atomic_pte_copy() captures ops = vma_uffd_ops(state->vma) and > passes it to __mfill_atomic_pte(). There, ops->alloc_folio() allocates > a folio for the original VMA's inode (e.g. a shmem folio for that > specific shmem inode). I wouldn't say ->alloc_folio() allocates a folio _for_ the inode, it allocates it with inode's memory policy. Worst can happen without any changes is that the allocated folio will end up in a wrong node. This is still a footgun, but I don't see it as a big deal. Let's revisit it after -rc1 and please make sure to cc "MEMORY MAPPING" folks for insights about how to better track VMA changes or their absence. > Then mfill_copy_folio_retry() drops all locks for > the copy_from_user retry. After mfill_get_vma() re-acquires them, > state->vma may now point to a replacement VMA, but ops is still the > stale pointer from before the drop. And this is a bug in my uffd refactoring, and it needs to be fixed ASAP with a simple comparison of old ops and new ops. > > Second, I have reservations about vma_snapshot implementation. What > > invariant does it exactly enforce? > > The invariant I was going for: "the folio we allocated is still > compatible with the VMA we're about to install it into." Since > alloc_folio() allocates from the VMA's backing file (inode), checking > that vm_file is still the same after re-acquiring locks ensures the > folio matches the inode. Again, it's not that folio matches the inode, but folio is allocated using the correct mempolicy. > The vm_flags comparison was a secondary guard against permission/type > changes during the window. Permissions should be fine, they are checked in userfaultfd_register. Some other flags that don't matter to uffd operation may change during the window, though and then a comparison of vm_flags will give a false positive. -- Sincerely yours, Mike.