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

[KunWu Chan]

On Wed, Aug 26, 2026 at 11:28 PM Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx> wrote:
>
> On Wed, Aug 26, 2026 at 11:15:18PM +0800, Kunwu Chan wrote:
> > On Tue, 25 Aug 2026 08:55:26 +0100 "Lorenzo Stoakes (ARM)" <ljs@xxxxxxxxxx> 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.
> > >
> > > 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>
> > > ---
> > >  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);
> >
> >
> > Hi Lorenzo,
> >
> > I think the fix makes sense. One thing I wanted to make sure
> > I understand correctly is the distinction between anon_pgoff
> > and vm_pgoff here.
> > Is the intention that anon_pgoff should be reset when the
> > VMA becomes unfaulted, while vm_pgoff should only be reset
> > for a truly anonymous VMA, since it may retain file-offset
> > semantics for VMAs with a vm_file?
>
> Yes.
>
> This is to account for both MAP_PRIVATE file-backed and pure anon. But to
> keep everything consistent (+ simple) we always update anon pgoff for
> everything.
>
Thanks, Lorenzo. That makes sense.

Reviewed-by: Kunwu Chan <kunwu.chan@xxxxxxxxx>

> >
> > Thanks,
> > KunWu
> >
> > > +   }
> > >
> > >     /* 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,
> > > --
> > > Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> > >
> > >
> >
>
> --
> Cheers, Lorenzo