linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "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>, Harry Yoo <harry.yoo@oracle.com>
Subject: Re: [PATCH] mm/vma: fix anon_vma UAF on mremap() faulted, unfaulted merge
Date: Fri, 2 Jan 2026 21:00:52 +0000	[thread overview]
Message-ID: <7ae14310-662b-4487-a9a4-5a6e828d3453@lucifer.local> (raw)
In-Reply-To: <20260102205520.986725-1-lorenzo.stoakes@oracle.com>

Andrew - obviously pending review scrutiny, could we get this into an rc-
relatively soon? As this is quite a serious bug.

Also many thanks due to Jeongjun for his work in analysing this bug and ensuring
it got attention, and Harry + David for their insightful contributions, much
appreciated!

Cheers, Lorenzo

On Fri, Jan 02, 2026 at 08:55:20PM +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.
>
> To account for this, introduce a new field in struct vma_merge_struct
> specifically for the mremap() case, and update vma_expand() to explicitly
> check for this case and invoke dup_anon_vma() to ensure anon_vma state is
> correctly propagated.
>
> 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.
>
> The following program reproduces the issue (and is fixed by this patch):
>
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/mman.h>
>
> #define RESERVED_PGS	(100)
> #define VMA_A_PGS	(10)
> #define VMA_B_PGS	(10)
> #define NUM_ITERS	(1000)
>
> static void trigger_bug(void)
> {
> 	unsigned long page_size = sysconf(_SC_PAGE_SIZE);
> 	char *reserved, *ptr_a, *ptr_b;
>
> 	/*
> 	 * The goal here is to achieve:
> 	 *
> 	 * mremap() with MREMAP_DONTUNMAP such that A and B merge:
> 	 *
> 	 *      |-------------------------|
> 	 *      |                         |
> 	 *      |    |-----------|   |---------|
> 	 *      v    | unfaulted |   | faulted |
> 	 *           |-----------|   |---------|
> 	 *                 B              A
> 	 *
> 	 * Then unmap VMA A to trigger the bug.
> 	 */
>
> 	/* Reserve a region of memory to operate in. */
> 	reserved = mmap(NULL, RESERVED_PGS * page_size, PROT_NONE,
> 			MAP_PRIVATE | MAP_ANON, -1, 0);
> 	if (reserved == MAP_FAILED) {
> 		perror("mmap reserved");
> 		exit(EXIT_FAILURE);
> 	}
>
> 	/* Map VMA A into place. */
> 	ptr_a = mmap(&reserved[page_size], VMA_A_PGS * page_size,
> 		     PROT_READ | PROT_WRITE,
> 		     MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0);
> 	if (ptr_a == MAP_FAILED) {
> 		perror("mmap VMA A");
> 		exit(EXIT_FAILURE);
> 	}
> 	/* Fault it in. */
> 	ptr_a[0] = 'x';
>
> 	/*
> 	 * Now move it out of the way so we can place VMA B in position,
> 	 * unfaulted.
> 	 */
> 	ptr_a = mremap(ptr_a, VMA_A_PGS * page_size, VMA_A_PGS * page_size,
> 		       MREMAP_FIXED | MREMAP_MAYMOVE, &reserved[50 * page_size]);
> 	if (ptr_a == MAP_FAILED) {
> 		perror("mremap VMA A out of the way");
> 		exit(EXIT_FAILURE);
> 	}
>
> 	/* Map VMA B into place. */
> 	ptr_b = mmap(&reserved[page_size + VMA_A_PGS * page_size],
> 		     VMA_B_PGS * page_size, PROT_READ | PROT_WRITE,
> 		     MAP_PRIVATE | MAP_ANON | MAP_FIXED, -1, 0);
> 	if (ptr_b == MAP_FAILED) {
> 		perror("mmap VMA B");
> 		exit(EXIT_FAILURE);
> 	}
>
> 	/* Now move VMA A into position w/MREMAP_DONTUNMAP + free anon_vma. */
> 	ptr_a = mremap(ptr_a, VMA_A_PGS * page_size, VMA_A_PGS * page_size,
> 		       MREMAP_FIXED | MREMAP_MAYMOVE | MREMAP_DONTUNMAP,
> 		       &reserved[page_size]);
> 	if (ptr_a == MAP_FAILED) {
> 		perror("mremap VMA A with MREMAP_DONTUNMAP");
> 		exit(EXIT_FAILURE);
> 	}
>
> 	/* Finally, unmap VMA A which should trigger the bug. */
> 	munmap(ptr_a, VMA_A_PGS * page_size);
>
> 	/* Cleanup in case bug didn't trigger sufficiently visibly... */
> 	munmap(reserved, RESERVED_PGS * page_size);
> }
>
> int main(void)
> {
> 	int i;
>
> 	for (i = 0; i < NUM_ITERS; i++)
> 		trigger_bug();
>
> 	return EXIT_SUCCESS;
> }
>
> 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
> ---
>  mm/vma.c | 58 ++++++++++++++++++++++++++++++++++++++++++--------------
>  mm/vma.h |  3 +++
>  2 files changed, 47 insertions(+), 14 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index 6377aa290a27..2268f518a89b 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -1130,26 +1130,50 @@ int vma_expand(struct vma_merge_struct *vmg)
>  	mmap_assert_write_locked(vmg->mm);
>
>  	vma_start_write(target);
> -	if (next && (target != next) && (vmg->end == next->vm_end)) {
> +	if (next && vmg->end == next->vm_end) {
> +		struct vm_area_struct *copied_from = vmg->copied_from;
>  		int ret;
>
> -		sticky_flags |= next->vm_flags & VM_STICKY;
> -		remove_next = true;
> -		/* This should already have been checked by this point. */
> -		VM_WARN_ON_VMG(!can_merge_remove_vma(next), vmg);
> -		vma_start_write(next);
> -		/*
> -		 * In this case we don't report OOM, so vmg->give_up_on_mm is
> -		 * safe.
> -		 */
> -		ret = dup_anon_vma(target, next, &anon_dup);
> -		if (ret)
> -			return ret;
> +		if (target != next) {
> +			sticky_flags |= next->vm_flags & VM_STICKY;
> +			remove_next = true;
> +			/* This should already have been checked by this point. */
> +			VM_WARN_ON_VMG(!can_merge_remove_vma(next), vmg);
> +			vma_start_write(next);
> +			/*
> +			 * In this case we don't report OOM, so vmg->give_up_on_mm is
> +			 * safe.
> +			 */
> +			ret = dup_anon_vma(target, next, &anon_dup);
> +			if (ret)
> +				return ret;
> +		} else if (copied_from) {
> +			vma_start_write(next);
> +
> +			/*
> +			 * We are copying from a VMA (i.e. mremap()'ing) to
> +			 * next, and thus must ensure that either anon_vma's are
> +			 * already compatible (in which case this call is a nop)
> +			 * or all anon_vma state is propagated to next
> +			 */
> +			ret = dup_anon_vma(next, copied_from, &anon_dup);
> +			if (ret)
> +				return ret;
> +		} else {
> +			/* In no other case may the anon_vma differ. */
> +			VM_WARN_ON_VMG(target->anon_vma != next->anon_vma, vmg);
> +		}
>  	}
>
>  	/* Not merging but overwriting any part of next is not handled. */
>  	VM_WARN_ON_VMG(next && !remove_next &&
>  		       next != target && vmg->end > next->vm_start, vmg);
> +	/*
> +	 * We should only see a copy with next as the target on a new merge
> +	 * which sets the end to the next of next.
> +	 */
> +	VM_WARN_ON_VMG(target == next && vmg->copied_from &&
> +		       vmg->end != next->vm_end, vmg);
>  	/* Only handles expanding */
>  	VM_WARN_ON_VMG(target->vm_start < vmg->start ||
>  		       target->vm_end > vmg->end, vmg);
> @@ -1807,6 +1831,13 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
>  	VMA_ITERATOR(vmi, mm, addr);
>  	VMG_VMA_STATE(vmg, &vmi, NULL, vma, addr, addr + len);
>
> +	/*
> +	 * VMG_VMA_STATE() installs vma in middle, but this is a new VMA, inform
> +	 * merging logic correctly.
> +	 */
> +	vmg.copied_from = vma;
> +	vmg.middle = NULL;
> +
>  	/*
>  	 * If anonymous vma has not yet been faulted, update new pgoff
>  	 * to match new location, to increase its chance of merging.
> @@ -1828,7 +1859,6 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
>  	if (new_vma && new_vma->vm_start < addr + len)
>  		return NULL;	/* should never get here */
>
> -	vmg.middle = NULL; /* New VMA range. */
>  	vmg.pgoff = pgoff;
>  	vmg.next = vma_iter_next_rewind(&vmi, NULL);
>  	new_vma = vma_merge_new_range(&vmg);
> diff --git a/mm/vma.h b/mm/vma.h
> index e4c7bd79de5f..50f0bdb0eb79 100644
> --- a/mm/vma.h
> +++ b/mm/vma.h
> @@ -106,6 +106,9 @@ struct vma_merge_struct {
>  	struct anon_vma_name *anon_name;
>  	enum vma_merge_state state;
>
> +	/* If we are copying a VMA, which VMA are we copying from? */
> +	struct vm_area_struct *copied_from;
> +
>  	/* Flags which callers can use to modify merge behaviour: */
>
>  	/*
> --
> 2.52.0


      reply	other threads:[~2026-01-02 21:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-02 20:55 Lorenzo Stoakes
2026-01-02 21:00 ` Lorenzo Stoakes [this message]

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=7ae14310-662b-4487-a9a4-5a6e828d3453@lucifer.local \
    --to=lorenzo.stoakes@oracle.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=aha310510@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=harry.yoo@oracle.com \
    --cc=jannh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --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