linux-kernel - Re: [PATCH] mm: Always sanity check 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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAJuCfpH8OpNF3vpcRAoBy_k9G_bMicgoHXipe=93Ub4dLjGOug@mail.gmail.com>
Date: Sat, 13 Apr 2024 16:11:09 -0700
From: Suren Baghdasaryan <surenb@...gle.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: Peter Xu <peterx@...hat.com>, "Liam R. Howlett" <Liam.Howlett@...cle.com>, 
	LKML <linux-kernel@...r.kernel.org>, linux-mm@...ck.org, 
	Andrew Morton <akpm@...ux-foundation.org>, Lokesh Gidra <lokeshgidra@...gle.com>, 
	Alistair Popple <apopple@...dia.com>
Subject: Re: [PATCH] mm: Always sanity check anon_vma first for per-vma locks

On Sat, Apr 13, 2024 at 3:52 PM Matthew Wilcox <willy@...radead.org> wrote:
>
> On Sat, Apr 13, 2024 at 02:46:56PM -0700, Suren Baghdasaryan wrote:
> > On Fri, Apr 12, 2024 at 8:31 AM Matthew Wilcox <willy@...radeadorg> wrote:
> > >  - Rename lock_vma() to uffd_lock_vma() because it really is uffd
> > >    specific.
> >
> > I'm planning to expand the scope of lock_vma() and reuse it for
> > /proc/pid/maps reading under per-VMA locks. No objection to renaming
> > it for now but I'll likely rename it back later once it's used in more
> > places.
>
> That would seem like a mistake.  The uffd lock_vma() will create an
> anon_vma for VMAs that don't have one, and you wouldn't want that.
> It seems to me that lock_vma_under_rcu() does everything you want except
> the fallback to mmap_read_lock().  And I'm not sure there's a good way
> to package that up ... indeed, I don't see why you'd want the "take
> the mmap_lock, look up the VMA, drop the mmap read lock" part at all --
> once you've got the mmap_lock, just hold it until you're done.

Yeah, you are right about anon_vma creation. I definitely don't want
that part when reading maps files.

Not sure about holding mmap_lock until I'm done. The goal of that
patch is to minimize blocking of any modifications while we are
reading maps files, so locking smaller parts might still make sense.
But it would be hard to argue one way or another without any data.