[PATCH v2 1/4] mm/vma: fix anon_vma UAF on mremap() faulted, unfaulted merge

Lorenzo Stoakes posted 4 patches 1 month ago
[PATCH v2 1/4] mm/vma: fix anon_vma UAF on mremap() faulted, unfaulted merge
Posted by Lorenzo Stoakes 1 month ago
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.

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.

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
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
---
 mm/vma.c | 84 +++++++++++++++++++++++++++++++++++++++-----------------
 mm/vma.h |  3 ++
 2 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/mm/vma.c b/mm/vma.c
index 6377aa290a27..660f4732f8a5 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -829,6 +829,8 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
 	VM_WARN_ON_VMG(middle &&
 		       !(vma_iter_addr(vmg->vmi) >= middle->vm_start &&
 			 vma_iter_addr(vmg->vmi) < middle->vm_end), vmg);
+	/* An existing merge can never be used by the mremap() logic. */
+	VM_WARN_ON_VMG(vmg->copied_from, vmg);
 
 	vmg->state = VMA_MERGE_NOMERGE;
 
@@ -1098,6 +1100,33 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
 	return NULL;
 }
 
+/*
+ * vma_merge_copied_range - Attempt to merge a VMA that is being copied by
+ * mremap()
+ *
+ * @vmg: Describes the VMA we are adding, in the copied-to range @vmg->start to
+ *       @vmg->end (exclusive), which we try to merge with any adjacent VMAs if
+ *       possible.
+ *
+ * vmg->prev, next, start, end, pgoff should all be relative to the COPIED TO
+ * range, i.e. the target range for the VMA.
+ *
+ * Returns: In instances where no merge was possible, NULL. Otherwise, a pointer
+ *          to the VMA we expanded.
+ *
+ * ASSUMPTIONS: Same as vma_merge_new_range(), except vmg->middle must contain
+ *              the copied-from VMA.
+ */
+static struct vm_area_struct *vma_merge_copied_range(struct vma_merge_struct *vmg)
+{
+	/* We must have a copied-from VMA. */
+	VM_WARN_ON_VMG(!vmg->middle, vmg);
+
+	vmg->copied_from = vmg->middle;
+	vmg->middle = NULL;
+	return vma_merge_new_range(vmg);
+}
+
 /*
  * vma_expand - Expand an existing VMA
  *
@@ -1117,46 +1146,52 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
 int vma_expand(struct vma_merge_struct *vmg)
 {
 	struct vm_area_struct *anon_dup = NULL;
-	bool remove_next = false;
 	struct vm_area_struct *target = vmg->target;
 	struct vm_area_struct *next = vmg->next;
+	bool remove_next = false;
 	vm_flags_t sticky_flags;
-
-	sticky_flags = vmg->vm_flags & VM_STICKY;
-	sticky_flags |= target->vm_flags & VM_STICKY;
-
-	VM_WARN_ON_VMG(!target, vmg);
+	int ret = 0;
 
 	mmap_assert_write_locked(vmg->mm);
-
 	vma_start_write(target);
-	if (next && (target != next) && (vmg->end == next->vm_end)) {
-		int ret;
 
-		sticky_flags |= next->vm_flags & VM_STICKY;
+	if (next && target != next && vmg->end == next->vm_end)
 		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;
-	}
 
+	/* We must have a target. */
+	VM_WARN_ON_VMG(!target, vmg);
+	/* This should have already been checked by this point. */
+	VM_WARN_ON_VMG(remove_next && !can_merge_remove_vma(next), 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);
-	/* Only handles expanding */
+	/* Only handles expanding. */
 	VM_WARN_ON_VMG(target->vm_start < vmg->start ||
 		       target->vm_end > vmg->end, vmg);
 
+	sticky_flags = vmg->vm_flags & VM_STICKY;
+	sticky_flags |= target->vm_flags & VM_STICKY;
 	if (remove_next)
-		vmg->__remove_next = true;
+		sticky_flags |= next->vm_flags & VM_STICKY;
 
+	/*
+	 * If we are removing the next VMA or copying from a VMA
+	 * (e.g. mremap()'ing), we must propagate anon_vma state.
+	 *
+	 * Note that, by convention, callers ignore OOM for this case, so
+	 * we don't need to account for vmg->give_up_on_mm here.
+	 */
+	if (remove_next)
+		ret = dup_anon_vma(target, next, &anon_dup);
+	if (!ret && vmg->copied_from)
+		ret = dup_anon_vma(target, vmg->copied_from, &anon_dup);
+	if (ret)
+		return ret;
+
+	if (remove_next) {
+		vma_start_write(next);
+		vmg->__remove_next = true;
+	}
 	if (commit_merge(vmg))
 		goto nomem;
 
@@ -1828,10 +1863,9 @@ 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);
+	new_vma = vma_merge_copied_range(&vmg);
 
 	if (new_vma) {
 		/*
diff --git a/mm/vma.h b/mm/vma.h
index e4c7bd79de5f..d51efd9da113 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 copied from (i.e. mremap()'d) the VMA from which we are copying. */
+	struct vm_area_struct *copied_from;
+
 	/* Flags which callers can use to modify merge behaviour: */
 
 	/*
-- 
2.52.0
Re: [PATCH v2 1/4] mm/vma: fix anon_vma UAF on mremap() faulted, unfaulted merge
Posted by Vlastimil Babka 4 weeks ago
On 1/5/26 21:11, Lorenzo Stoakes wrote:
> 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
> 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

Acked-by: Vlastimil Babka <vbabka@suse.cz>

Nit below:

> @@ -1117,46 +1146,52 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
>  int vma_expand(struct vma_merge_struct *vmg)
>  {
>  	struct vm_area_struct *anon_dup = NULL;
> -	bool remove_next = false;
>  	struct vm_area_struct *target = vmg->target;
>  	struct vm_area_struct *next = vmg->next;
> +	bool remove_next = false;
>  	vm_flags_t sticky_flags;
> -
> -	sticky_flags = vmg->vm_flags & VM_STICKY;
> -	sticky_flags |= target->vm_flags & VM_STICKY;
> -
> -	VM_WARN_ON_VMG(!target, vmg);
> +	int ret = 0;
>  
>  	mmap_assert_write_locked(vmg->mm);
> -
>  	vma_start_write(target);
> -	if (next && (target != next) && (vmg->end == next->vm_end)) {
> -		int ret;
>  
> -		sticky_flags |= next->vm_flags & VM_STICKY;
> +	if (next && target != next && vmg->end == next->vm_end)
>  		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;
> -	}
>  
> +	/* We must have a target. */
> +	VM_WARN_ON_VMG(!target, vmg);
> +	/* This should have already been checked by this point. */
> +	VM_WARN_ON_VMG(remove_next && !can_merge_remove_vma(next), 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);
> -	/* Only handles expanding */
> +	/* Only handles expanding. */
>  	VM_WARN_ON_VMG(target->vm_start < vmg->start ||
>  		       target->vm_end > vmg->end, vmg);
>  
> +	sticky_flags = vmg->vm_flags & VM_STICKY;
> +	sticky_flags |= target->vm_flags & VM_STICKY;
>  	if (remove_next)
> -		vmg->__remove_next = true;
> +		sticky_flags |= next->vm_flags & VM_STICKY;
>  
> +	/*
> +	 * If we are removing the next VMA or copying from a VMA
> +	 * (e.g. mremap()'ing), we must propagate anon_vma state.
> +	 *
> +	 * Note that, by convention, callers ignore OOM for this case, so
> +	 * we don't need to account for vmg->give_up_on_mm here.

It's called "give_up_on_oom". It was already wrong so this comment move
would be a chance to fix it. I think Andrew can just edit locally?

> +	 */
> +	if (remove_next)
> +		ret = dup_anon_vma(target, next, &anon_dup);
> +	if (!ret && vmg->copied_from)
> +		ret = dup_anon_vma(target, vmg->copied_from, &anon_dup);
> +	if (ret)
> +		return ret;
> +
> +	if (remove_next) {
> +		vma_start_write(next);
> +		vmg->__remove_next = true;
> +	}
>  	if (commit_merge(vmg))
>  		goto nomem;
>  
> @@ -1828,10 +1863,9 @@ 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);
> +	new_vma = vma_merge_copied_range(&vmg);
>  
>  	if (new_vma) {
>  		/*
> diff --git a/mm/vma.h b/mm/vma.h
> index e4c7bd79de5f..d51efd9da113 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 copied from (i.e. mremap()'d) the VMA from which we are copying. */
> +	struct vm_area_struct *copied_from;
> +
>  	/* Flags which callers can use to modify merge behaviour: */
>  
>  	/*
Re: [PATCH v2 1/4] mm/vma: fix anon_vma UAF on mremap() faulted, unfaulted merge
Posted by Jeongjun Park 1 month ago
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> 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.
>
> 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.
>
> 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
> 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
> ---

Wow, I didn't know there would be this many problems. LGTM
Reviewed-by: Jeongjun Park <aha310510@gmail.com>

And this syzbot report seems to have the same root cause.
Reported-by: syzbot+5272541ccbbb14e2ec30@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/694e3dc6.050a0220.35954c.0066.GAE@google.com/

>  mm/vma.c | 84 +++++++++++++++++++++++++++++++++++++++-----------------
>  mm/vma.h |  3 ++
>  2 files changed, 62 insertions(+), 25 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index 6377aa290a27..660f4732f8a5 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -829,6 +829,8 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(
>         VM_WARN_ON_VMG(middle &&
>                        !(vma_iter_addr(vmg->vmi) >= middle->vm_start &&
>                          vma_iter_addr(vmg->vmi) < middle->vm_end), vmg);
> +       /* An existing merge can never be used by the mremap() logic. */
> +       VM_WARN_ON_VMG(vmg->copied_from, vmg);
>
>         vmg->state = VMA_MERGE_NOMERGE;
>
> @@ -1098,6 +1100,33 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
>         return NULL;
>  }
>
> +/*
> + * vma_merge_copied_range - Attempt to merge a VMA that is being copied by
> + * mremap()
> + *
> + * @vmg: Describes the VMA we are adding, in the copied-to range @vmg->start to
> + *       @vmg->end (exclusive), which we try to merge with any adjacent VMAs if
> + *       possible.
> + *
> + * vmg->prev, next, start, end, pgoff should all be relative to the COPIED TO
> + * range, i.e. the target range for the VMA.
> + *
> + * Returns: In instances where no merge was possible, NULL. Otherwise, a pointer
> + *          to the VMA we expanded.
> + *
> + * ASSUMPTIONS: Same as vma_merge_new_range(), except vmg->middle must contain
> + *              the copied-from VMA.
> + */
> +static struct vm_area_struct *vma_merge_copied_range(struct vma_merge_struct *vmg)
> +{
> +       /* We must have a copied-from VMA. */
> +       VM_WARN_ON_VMG(!vmg->middle, vmg);
> +
> +       vmg->copied_from = vmg->middle;
> +       vmg->middle = NULL;
> +       return vma_merge_new_range(vmg);
> +}
> +
>  /*
>   * vma_expand - Expand an existing VMA
>   *
> @@ -1117,46 +1146,52 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg)
>  int vma_expand(struct vma_merge_struct *vmg)
>  {
>         struct vm_area_struct *anon_dup = NULL;
> -       bool remove_next = false;
>         struct vm_area_struct *target = vmg->target;
>         struct vm_area_struct *next = vmg->next;
> +       bool remove_next = false;
>         vm_flags_t sticky_flags;
> -
> -       sticky_flags = vmg->vm_flags & VM_STICKY;
> -       sticky_flags |= target->vm_flags & VM_STICKY;
> -
> -       VM_WARN_ON_VMG(!target, vmg);
> +       int ret = 0;
>
>         mmap_assert_write_locked(vmg->mm);
> -
>         vma_start_write(target);
> -       if (next && (target != next) && (vmg->end == next->vm_end)) {
> -               int ret;
>
> -               sticky_flags |= next->vm_flags & VM_STICKY;
> +       if (next && target != next && vmg->end == next->vm_end)
>                 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;
> -       }
>
> +       /* We must have a target. */
> +       VM_WARN_ON_VMG(!target, vmg);
> +       /* This should have already been checked by this point. */
> +       VM_WARN_ON_VMG(remove_next && !can_merge_remove_vma(next), 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);
> -       /* Only handles expanding */
> +       /* Only handles expanding. */
>         VM_WARN_ON_VMG(target->vm_start < vmg->start ||
>                        target->vm_end > vmg->end, vmg);
>
> +       sticky_flags = vmg->vm_flags & VM_STICKY;
> +       sticky_flags |= target->vm_flags & VM_STICKY;
>         if (remove_next)
> -               vmg->__remove_next = true;
> +               sticky_flags |= next->vm_flags & VM_STICKY;
>
> +       /*
> +        * If we are removing the next VMA or copying from a VMA
> +        * (e.g. mremap()'ing), we must propagate anon_vma state.
> +        *
> +        * Note that, by convention, callers ignore OOM for this case, so
> +        * we don't need to account for vmg->give_up_on_mm here.
> +        */
> +       if (remove_next)
> +               ret = dup_anon_vma(target, next, &anon_dup);
> +       if (!ret && vmg->copied_from)
> +               ret = dup_anon_vma(target, vmg->copied_from, &anon_dup);
> +       if (ret)
> +               return ret;
> +
> +       if (remove_next) {
> +               vma_start_write(next);
> +               vmg->__remove_next = true;
> +       }
>         if (commit_merge(vmg))
>                 goto nomem;
>
> @@ -1828,10 +1863,9 @@ 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);
> +       new_vma = vma_merge_copied_range(&vmg);
>
>         if (new_vma) {
>                 /*
> diff --git a/mm/vma.h b/mm/vma.h
> index e4c7bd79de5f..d51efd9da113 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 copied from (i.e. mremap()'d) the VMA from which we are copying. */
> +       struct vm_area_struct *copied_from;
> +
>         /* Flags which callers can use to modify merge behaviour: */
>
>         /*
> --
> 2.52.0
>
Re: [PATCH v2 1/4] mm/vma: fix anon_vma UAF on mremap() faulted, unfaulted merge
Posted by Harry Yoo 1 month ago
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