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

[Vlastimil Babka]

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@xxxxxxxxxx>
> Fixes: 879bca0a2c4f ("mm/vma: fix incorrectly disallowed anonymous VMA merges")
> Reported-by: syzbot+b165fc2e11771c66d8ba@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/all/694a2745.050a0220.19928e.0017.GAE@xxxxxxxxxx/
> Cc: stable@xxxxxxxxxx

Acked-by: Vlastimil Babka <vbabka@xxxxxxx>

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: */
>  
>  	/*