[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0bb8539e-4952-49e6-bcfb-262b8fc474bc@lucifer.local>
Date: Fri, 2 Jan 2026 11:31:25 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Harry Yoo <harry.yoo@...cle.com>
Cc: "David Hildenbrand (Red Hat)" <david@...nel.org>,
Jeongjun Park <aha310510@...il.com>, Liam.Howlett@...cle.com,
akpm@...ux-foundation.org, jannh@...gle.com,
linux-kernel@...r.kernel.org, linux-mm@...ck.org, riel@...riel.com,
syzbot+b165fc2e11771c66d8ba@...kaller.appspotmail.com,
syzkaller-bugs@...glegroups.com, vbabka@...e.cz
Subject: Re: [syzbot] [mm?] WARNING in folio_remove_rmap_ptes
Thanks Harry, appreciate the analysis :)
I will take a look through but first I am going through the now reliable repro
(apologies Jeongjing - indeed it can be reliable I did misunderstand you! :)
And once I've figured it out I will come back with an explanation and a fix.
Cheers, Lorenzo
On Fri, Jan 02, 2026 at 05:14:09PM +0900, Harry Yoo wrote:
> On Thu, Jan 01, 2026 at 09:28:46PM +0000, Lorenzo Stoakes wrote:
> > On Thu, Jan 01, 2026 at 06:06:23PM +0100, David Hildenbrand (Red Hat) wrote:
> > > On 1/1/26 17:32, Lorenzo Stoakes wrote:
> > > > On Thu, Jan 01, 2026 at 11:30:52PM +0900, Jeongjun Park wrote:
> > > > >
> > > > > Based on my testing, I found that the WARNING starts from commit
> > > > > d23cb648e365 ("mm/mremap: permit mremap() move of multiple VMAs"),
> > > > > which is right after commit 2cf442d74216 ("mm/mremap: clean up mlock
> > > > > populate behavior") in Lorenzo's mremap-related patch series.
> > > >
> > > > OK let me take a look.
> > >
> > > Trying to make sense of the reproducer and how bpf comes into play ... I
> > > assume BPF is only used to install a uprobe.
> > >
> > > We seem to create a file0 and register a uprobe on it.
> > >
> > > We then mmap() that file with PROT_NONE. We should end up in uprobe_mmap()
> > > and trigger a COW fault -> allocate an anon_vma.
> > >
> > > So likely the bpf magic is only there to allocate an anon_vma for a
> > > PROT_NONE region.
> > >
> > > But it's all a bit confusing ... :)
> > >
> > > --
> > > Cheers
> > >
> > > David
> >
> > OK I had a huge reply going through all of Jeongjun's stuff (thanks for
> > reporting!) but then got stuck into theories and highways and byways... all the
> > while I couldn't repro.
> >
> > Well now I can repro reliably, finally!
> >
>
> Great! still not sure why I can't still repro :P
>
> The most viable theory from me is:
>
> When we call mremap() and move VMA A into new range that fits into
> the gap between two VMAs:
>
> [ prev ][ new range ][ next ]
>
> Let's say prev and next don't have anon_vma, then
> we're supposed to link prev VMA to VMA A's anon_vma.
>
> But looking at vma_merge_new_range():
> > 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;
> > 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);
> >
> > 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;
> > 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);
>
> For 3-way merge, here we're passing target (prev) and next...
>
> > if (ret)
> > return ret;
> > }
>
> In dup_anon_vma():
> > /*
> > * dup_anon_vma() - Helper function to duplicate anon_vma on VMA merge in the
> > * instance that the destination VMA has no anon_vma but the source does.
> > *
> > * @dst: The destination VMA
> > * @src: The source VMA
> > * @dup: Pointer to the destination VMA when successful.
> > *
> > * Returns: 0 on success.
> > */
> > static int dup_anon_vma(struct vm_area_struct *dst,
> > struct vm_area_struct *src, struct vm_area_struct **dup)
> > {
> > /*
> > * There are three cases to consider for correctly propagating
> > * anon_vma's on merge.
> > *
> > * The first is trivial - neither VMA has anon_vma, we need not do
> > * anything.
> > *
> > * The second where both have anon_vma is also a no-op, as they must
> > * then be the same, so there is simply nothing to copy.
> > *
> > * Here we cover the third - if the destination VMA has no anon_vma,
> > * that is it is unfaulted, we need to ensure that the newly merged
> > * range is referenced by the anon_vma's of the source.
> > */
> > if (src->anon_vma && !dst->anon_vma) {
> > int ret;
>
> I think the "src" is supposed to be VMA A that has anon_vma,
> but we passed "next" that is unfaulted, so we don't link "src" vma to
> the anon_vma because both "src" and "dst" don't have anon_vma.
>
> BUT we reuse the anon_vma anyway, and by the time we call
> dontunmap_complete(), the anon_vma gets freed because its
> rbtree is empty (which isn't supposed to be empty because
> we should have linked prev to the anon_vma).
>
> Does this theory make sense, or am I confused again and my brain is
> misfunctioning :)
>
> >
> > vma_assert_write_locked(dst);
> > dst->anon_vma = src->anon_vma;
> > ret = anon_vma_clone(dst, src);
> > if (ret)
> > return ret;
> >
> > *dup = dst;
> > }
> >
> > return 0;
> > }
>
> --
> Cheers,
> Harry / Hyeonggon
Powered by blists - more mailing lists