linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Harry Yoo <harry.yoo@oracle.com>
To: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>,
	Vlastimil Babka <vbabka@suse.cz>, Jann Horn <jannh@google.com>,
	Pedro Falcato <pfalcato@suse.de>,
	Yeoreum Yun <yeoreum.yun@arm.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	David Hildenbrand <david@kernel.org>,
	Jeongjun Park <aha310510@gmail.com>,
	Rik van Riel <riel@surriel.com>
Subject: Re: [PATCH v2 1/4] mm/vma: fix anon_vma UAF on mremap() faulted, unfaulted merge
Date: Tue, 6 Jan 2026 12:15:53 +0900	[thread overview]
Message-ID: <aVx-aWZZBMvMMQy3@hyeyoo> (raw)
In-Reply-To: <b7930ad2b1503a657e29fe928eb33061d7eadf5b.1767638272.git.lorenzo.stoakes@oracle.com>

On Mon, Jan 05, 2026 at 08:11:47PM +0000, Lorenzo Stoakes wrote:
> Commit 879bca0a2c4f ("mm/vma: fix incorrectly disallowed anonymous VMA
> merges") introduced the ability to merge previously unavailable VMA merge
> scenarios.
> 
> The key piece of logic introduced was the ability to merge a faulted VMA
> immediately next to an unfaulted VMA, which relies upon dup_anon_vma() to
> correctly handle anon_vma state.
> 
> In the case of the merge of an existing VMA (that is changing properties
> of a VMA and then merging if those properties are shared by adjacent
> VMAs), dup_anon_vma() is invoked correctly.
> 
> However in the case of the merge of a new VMA, a corner case peculiar to
> mremap() was missed.
> 
> The issue is that vma_expand() only performs dup_anon_vma() if the target
> (the VMA that will ultimately become the merged VMA): is not the next VMA,
> i.e.  the one that appears after the range in which the new VMA is to be
> established.
> 
> A key insight here is that in all other cases other than mremap(), a new
> VMA merge either expands an existing VMA, meaning that the target VMA will
> be that VMA, or would have anon_vma be NULL.
> 
> Specifically:
> 
> * __mmap_region() - no anon_vma in place, initial mapping.
> * do_brk_flags() - expanding an existing VMA.
> * vma_merge_extend() - expanding an existing VMA.
> * relocate_vma_down() - no anon_vma in place, initial mapping.
> 
> In addition, we are in the unique situation of needing to duplicate
> anon_vma state from a VMA that is neither the previous or next VMA being
> merged with.
> 
> dup_anon_vma() deals exclusively with the target=unfaulted, src=faulted
> case. This leaves four possibilities, in each case where the copied VMA is
> faulted:
> 
> 1. Previous VMA unfaulted:
> 
>               copied -----|
>                           v
> 	|-----------|.............|
> 	| unfaulted |(faulted VMA)|
> 	|-----------|.............|
> 	     prev
> 
> target = prev, expand prev to cover.

Oops, I missed this case!

> 2. Next VMA unfaulted:
> 
>               copied -----|
>                           v
> 	            |.............|-----------|
> 	            |(faulted VMA)| unfaulted |
>                     |.............|-----------|
> 		                      next
> 
> target = next, expand next to cover.
> 
> 3. Both adjacent VMAs unfaulted:
> 
>               copied -----|
>                           v
> 	|-----------|.............|-----------|
> 	| unfaulted |(faulted VMA)| unfaulted |
> 	|-----------|.............|-----------|
> 	     prev                      next
> 
> target = prev, expand prev to cover.
> 
> 4. prev unfaulted, next faulted:
> 
>               copied -----|
>                           v
> 	|-----------|.............|-----------|
> 	| unfaulted |(faulted VMA)|  faulted  |
> 	|-----------|.............|-----------|
> 	     prev                      next
> 
> target = prev, expand prev to cover. Essentially equivalent to 3, but with
> additional requirement that next's anon_vma is the same as the copied
> VMA's. This is covered by the existing logic.
> 
> To account for this very explicitly, we introduce vma_merge_copied_range(),
> which sets a newly introduced vmg->copied_from field, then invokes
> vma_merge_new_range() which handles the rest of the logic.
> 
> We then update the key vma_expand() function to clean up the logic and make
> what's going on clearer, making the 'remove next' case less special, before
> invoking dup_anon_vma() unconditionally should we be copying from a VMA.
> 
> Note that in case 3, the if (remove_next) ... branch will be a no-op, as
> next=src in this instance and src is unfaulted.
> 
> In case 4, it won't be, but since in this instance next=src and it is
> faulted, this will have required tgt=faulted, src=faulted to be compatible,
> meaning that next->anon_vma == vmg->copied_from->anon_vma, and thus a
> single dup_anon_vma() of next suffices to copy anon_vma state for the
> copied-from VMA also.

Makes sense.

> If we are copying from a VMA in a successful merge we must _always_
> propagate anon_vma state.
> 
> This issue can be observed most directly by invoked mremap() to move
> around a VMA and cause this kind of merge with the MREMAP_DONTUNMAP flag
> specified.
> 
> This will result in unlink_anon_vmas() being called after failing to
> duplicate anon_vma state to the target VMA, which results in the anon_vma
> itself being freed with folios still possessing dangling pointers to the
> anon_vma and thus a use-after-free bug.
> 
> This bug was discovered via a syzbot report, which this patch resolves.
 
> We further make a change to update the mergeable anon_vma check to assert
> the copied-from anon_vma did not have CoW parents, as otherwise

I guess that part is in patch 3/4.

> dup_anon_vma() might incorrectly propagate CoW ancestors from the next VMA
> in case 4 despite the anon_vma's being identical for both VMAs.
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Fixes: 879bca0a2c4f ("mm/vma: fix incorrectly disallowed anonymous VMA merges")
> Reported-by: syzbot+b165fc2e11771c66d8ba@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/694a2745.050a0220.19928e.0017.GAE@google.com/
> Cc: stable@kernel.org
> ---

Looks good to me, so:
Reviewed-by: Harry Yoo <harry.yoo@oracle.com>

-- 
Cheers,
Harry / Hyeonggon


  reply	other threads:[~2026-01-06  3:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-05 20:11 [PATCH v2 0/4] " Lorenzo Stoakes
2026-01-05 20:11 ` [PATCH v2 1/4] " Lorenzo Stoakes
2026-01-06  3:15   ` Harry Yoo [this message]
2026-01-06 15:01   ` Jeongjun Park
2026-01-05 20:11 ` [PATCH v2 2/4] tools/testing/selftests: add tests for !tgt, src mremap() merges Lorenzo Stoakes
2026-01-05 20:11 ` [PATCH v2 3/4] mm/vma: enforce VMA fork limit on unfaulted,faulted mremap merge too Lorenzo Stoakes
2026-01-06  6:03   ` Harry Yoo
2026-01-06 15:23   ` Jeongjun Park
2026-01-05 20:11 ` [PATCH v2 4/4] tools/testing/selftests: add forked (un)/faulted VMA merge tests Lorenzo Stoakes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aVx-aWZZBMvMMQy3@hyeyoo \
    --to=harry.yoo@oracle.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=aha310510@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=pfalcato@suse.de \
    --cc=riel@surriel.com \
    --cc=vbabka@suse.cz \
    --cc=yeoreum.yun@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox