linux-kernel - Re: [PATCH v2 1/8] mm/rmap: improve anon_vma_clone(), unlink_anon

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: <3b56ecb1-377e-4b50-815a-19ebb850f108@lucifer.local>
Date: Wed, 14 Jan 2026 18:48:55 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Suren Baghdasaryan <surenb@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Shakeel Butt <shakeel.butt@...ux.dev>,
        David Hildenbrand <david@...nel.org>, Rik van Riel <riel@...riel.com>,
        Harry Yoo <harry.yoo@...cle.com>, Jann Horn <jannh@...gle.com>,
        Mike Rapoport <rppt@...nel.org>, Michal Hocko <mhocko@...e.com>,
        Pedro Falcato <pfalcato@...e.de>, Chris Li <chriscli@...gle.com>,
        Barry Song <v-songbaohua@...o.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/8] mm/rmap: improve anon_vma_clone(),
 unlink_anon_vmas() comments, add asserts

On Wed, Jan 14, 2026 at 06:33:39PM +0000, Lorenzo Stoakes wrote:
> On Wed, Jan 14, 2026 at 08:14:23AM -0800, Suren Baghdasaryan wrote:
> > > + *
> > > + * So this function needs only traverse the anon_vma_chain and free each
> > > + * allocated anon_vma_chain.
> > > + */
> > > +static void cleanup_partial_anon_vmas(struct vm_area_struct *vma)
> > > +{
> > > +       struct anon_vma_chain *avc, *next;
> > > +       bool locked = false;
> > > +
> > > +       /*
> > > +        * We exclude everybody else from being able to modify anon_vma's
> > > +        * underneath us.
> > > +        */
> > > +       mmap_assert_locked(vma->vm_mm);
> > > +
> > > +       list_for_each_entry_safe(avc, next, &vma->anon_vma_chain, same_vma) {
> > > +               struct anon_vma *anon_vma = avc->anon_vma;
> > > +
> > > +               /* All anon_vma's share the same root. */
> > > +               if (!locked) {
> > > +                       anon_vma_lock_write(anon_vma);
> > > +                       locked = true;
> > > +               }
> > > +
> > > +               anon_vma_interval_tree_remove(avc, &anon_vma->rb_root);
> > > +               list_del(&avc->same_vma);
> > > +               anon_vma_chain_free(avc);
> > > +       }
> >
> > Are you missing "if (locked) anon_vma_unlock_write()" here?
>
> Ugh god yes. This is an annoying irrelevance as we eliminate it anyway later but
> I'll do a fix-patch to avoid bisection hazard.
>
> > You could also avoid using "locked" variable by setting anon_vma =
> > NULL initially and using "if (anon_vma)" as an equivalent of "if
> > (locked)"
>
> I know, I did this on purpose to be explicit. The compiler will optimise this away.
>
> Anyway it's moot as this gets removed later so I say let's keep it as it is.

Ugh yeah it's just broken, as we obviously don't save anon_vma. I just hated
that way of doing this but I guess I have no choice.

And find solace in this being an intermediate patch that gets yoinked in a later
one :)