linux-kernel - Re: [PATCH v2 2/5] mm: add atomic VMA flags, use VM_MAYBE

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4822cf8d-a871-4e8d-99cd-bb31463149fa@lucifer.local>
Date: Thu, 6 Nov 2025 14:03:52 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>, David Hildenbrand <david@...hat.com>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Mike Rapoport <rppt@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-mm@...ck.org,
        linux-trace-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        Andrei Vagin <avagin@...il.com>
Subject: Re: [PATCH v2 2/5] mm: add atomic VMA flags, use VM_MAYBE_GUARD as
 such

On Thu, Nov 06, 2025 at 12:31:29PM +0100, Vlastimil Babka wrote:
> On 11/6/25 11:46, Lorenzo Stoakes wrote:
> > This patch adds the ability to atomically set VMA flags with only the mmap
> > read/VMA read lock held.
> >
> > As this could be hugely problematic for VMA flags in general given that all
> > other accesses are non-atomic and serialised by the mmap/VMA locks, we
> > implement this with a strict allow-list - that is, only designated flags
> > are allowed to do this.
> >
> > We make VM_MAYBE_GUARD one of these flags, and then set it under the mmap
> > read flag upon guard region installation.
> >
> > The places where this flag is used currently and matter are:
> >
> > * VMA merge - performed under mmap/VMA write lock, therefore excluding
> >   racing writes.
> >
> > * /proc/$pid/smaps - can race the write, however this isn't meaningful as
> >   the flag write is performed at the point of the guard region being
> >   established, and thus an smaps reader can't reasonably expect to avoid
> >   races. Due to atomicity, a reader will observe either the flag being set
> >   or not. Therefore consistency will be maintained.
> >
> > In all other cases the flag being set is irrelevant and atomicity
> > guarantees other flags will be read correctly.
>
> Could we maybe also spell out that we rely on the read mmap/VMA lock to
> exclude with writers that have write lock and then use non-atomic updates to
> update completely different flags than VM_MAYBE_GUARD? Those non-atomic
> updates could cause RMW races when only our side uses an atomic update, but
> the trick is that the read lock excludes with the write lock.

I thought this was implicit, I guess I can spell that out.

>
> > We additionally update madvise_guard_install() to ensure that
> > anon_vma_prepare() is set for anonymous VMAs to maintain consistency with
> > the assumption that any anonymous VMA with page tables will have an
> > anon_vma set, and any with an anon_vma unset will not have page tables
> > established.
>
> Could we more obviously say that we did anon_vma_prepare() unconditionally
> before this patch to trigger the page table copying in fork, but it's not
> needed anymore because fork now checks also VM_MAYBE_GUARD that we're
> setting here. Maybe it would be even more obvious to move that
> vma_needs_copy() hunk from previous patch to this one, but doesn't matter
> that much.

I thought that was covered between the comment, the previous patch and this but
I can spell it out also.

>
> Also we could mention that this patch alone will prevent merging of VMAs in
> some situations, but that's addressed next. I don't think it's such a bisect
> hazard to need reordering or combining changes, just mention perhaps.

A little pedantic but sure :)

>
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
>
> Otherwise LGTM.
>
> Reviewed-by: Vlastimil Babka <vbabka@...e.cz>
>

Thanks!