lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ee25bfdf-17b7-432e-adcd-42d104dcca86@lucifer.local>
Date: Fri, 2 Jan 2026 15:49:40 +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

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.

That is nearly it, actually well done! You are smart :)

I am going to give a full explanation in a little while because I've
discovered the root cause and have a fix.

It's a bit fiddly and want to be thorough.

>
> 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

Cheers, Lorenzo

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ