Re: [PATCH] mm/mremap: reset unfaulted VMA page offset for MREMAP_DONTUNMAP

[Lorenzo Stoakes (ARM)]

On Thu, Aug 27, 2026 at 10:32:15AM +0200, Vlastimil Babka (SUSE) wrote:
> On 8/25/26 9:55 AM, Lorenzo Stoakes (ARM) wrote:
> > Uniquely an mremap() invocation using the MREMAP_DONTUNMAP flag can reset
> > a faulted VMA into an unfaulted one.
> >
> > It does so after the page tables have been moved to the copied VMA with
> > MREMAP_DONTUNMAP leaving the old VMA in place which is naturally unfaulted
> > as the page tables it had are no longer present.
> >
> > However, in doing so, it violates the invariant that the anonymous page
> > offset of an unfaulted VMA is vma->vm_start >> PAGE_SHIFT.
> >
> > This is because a VMA may have been faulted in, mremap()'d (causing a delta
> > between its page offset and vma->vm_start >> PAGE_SHIFT), and then
> > mremap()'d again with MREMAP_DONTUNMAP resulting in the unfaulting.
> >
> > This condition is a violation of a fundamental assumption in mm, but now
> > also triggers an assert in assert_sane_pgoff() which explicitly checks for
> > this condition.
>
> Oof. So what's the worst thing that could happen before the assert was
> added? We'd use the "unfaulted" state to allow a merge, but the wrong
> pgoff could mess up the result of the merge somehow?

Yep you'd just get merging not working. It's a bit of a unique set of
circumstances so it's not a huge impact, but it's an edge case that'd break
scalable CoW assumptions that I want to use to avoid having to track remaps
so it's a good one to find :)

Definitely incorrect however even if low impact in the past.

>
> > Correct it by resetting the VMA's page offset at the point of completing
> > the MREMAP_DONTUNMAP operation.
> >
> > Reported-by: syzbot+f12658786a4153df5113@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://lore.kernel.org/all/6a87853b.ae6ddae5.3da009.0023.GAE@xxxxxxxxxx/
> > Fixes: 1583aa278f5f ("mm: mremap: unlink anon_vmas when mremap with MREMAP_DONTUNMAP success")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
>
> Acked-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>

Thanks!

>
> > ---
> >  mm/mremap.c | 22 +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/mm/mremap.c b/mm/mremap.c
> > index e8df5cdb0ac9..2b4b523a86b8 100644
> > --- a/mm/mremap.c
> > +++ b/mm/mremap.c
> > @@ -1331,18 +1331,30 @@ static void dontunmap_complete(struct vma_remap_struct *vrm,
> >  {
> >  	unsigned long start = vrm->addr;
> >  	unsigned long end = vrm->addr + vrm->old_len;
> > -	unsigned long old_start = vrm->vma->vm_start;
> > -	unsigned long old_end = vrm->vma->vm_end;
> > +	struct vm_area_struct *vma = vrm->vma;
> > +	unsigned long old_start = vma->vm_start;
> > +	unsigned long old_end = vma->vm_end;
> >
> >  	/* We always clear VMA_LOCKED[ONFAULT]_BIT on the old VMA. */
> > -	vma_clear_flags_mask(vrm->vma, VMA_LOCKED_MASK);
> > +	vma_clear_flags_mask(vma, VMA_LOCKED_MASK);
> >
> >  	/*
> >  	 * anon_vma links of the old vma is no longer needed after its page
> >  	 * table has been moved.
> >  	 */
> > -	if (new_vma != vrm->vma && start == old_start && end == old_end)
> > -		unlink_anon_vmas(vrm->vma);
> > +	if (new_vma != vma && start == old_start && end == old_end) {
> > +		const pgoff_t pgoff_unfaulted = vma->vm_start >> PAGE_SHIFT;
> > +
> > +		unlink_anon_vmas(vma);
> > +		/*
> > +		 * The VMA is now unfaulted and it is an invariant that
> > +		 * unfaulted anonymous VMAs have page offset equal to
> > +		 * vma->vm_start >> PAGE_SHIFT.
> > +		 */
> > +		vma_set_anon_pgoff(vma, pgoff_unfaulted);
> > +		if (vma_is_anonymous(vma) && !vma->vm_file)
> > +			vma_set_pgoff(vma, pgoff_unfaulted);
> > +	}
> >
> >  	/* Because we won't unmap we don't need to touch locked_vm. */
> >  }
> >
> > ---
> > base-commit: efecab401cb15fd3bb9bc05990609acb6b267ff2
> > change-id: 20260824-fix-mremap-dontunmap-pgoff-a687134e995e
> >
> > Best regards,
>

--
Cheers, Lorenzo