linux-kernel - Re: [PATCH v13 2/8] mm: rust: add vm_area_struct methods that require read access

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: <CAH5fLgh2Y0rG5ntpdiK-ycn9eBvrSXekxPsjDaYWuHBDUYNppw@mail.gmail.com>
Date: Wed, 5 Feb 2025 13:10:16 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: "Liam R. Howlett" <Liam.Howlett@...cle.com>, Alice Ryhl <aliceryhl@...gle.com>, 
	Miguel Ojeda <ojeda@...nel.org>, Matthew Wilcox <willy@...radead.org>, 
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	John Hubbard <jhubbard@...dia.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Arnd Bergmann <arnd@...db.de>, Jann Horn <jannh@...gle.com>, 
	Suren Baghdasaryan <surenb@...gle.com>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, linux-kernel@...r.kernel.org, linux-mm@...ck.org, 
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v13 2/8] mm: rust: add vm_area_struct methods that require
 read access

On Tue, Feb 4, 2025 at 4:46 PM Liam R. Howlett <Liam.Howlett@...cle.com> wrote:
> > > > > > +        let vma = unsafe { bindings::vma_lookup(self.mm.as_raw(), vma_addr) };
> > > > > > +
> > > > > > +        if vma.is_null() {
> > > > > > +            None
> > > > > > +        } else {
> > > > > > +            // SAFETY: We just checked that a vma was found, so the pointer is valid. Furthermore,
> > > > > > +            // the returned area will borrow from this read lock guard, so it can only be used
> > > > > > +            // while the mmap read lock is still held.
> > > > >
> > > > > So We have complicated the locking of the vmas with rcu and per-vma
> > > > > locking recently.  We are now able to look up and use a vma under the
> > > > > rcu read lock.  Does this translate to rust model?
> > > > >
> > > > > I believe this is true in recent version of binder as well?
> > > >
> > > > Yes. The safety requirements of VmAreaRef is that you must hold the
> > > > mmap read lock *or* the vma read lock while you have a VmAreaRef
> > > > reference. This particular method achieves that requirement by holding
> > > > the mmap read lock. But there is also a Rust lock_vma_under_rcu(), see
> > > > patch 4 for that.
> > >
> > > Right, okay.  Thanks.  You can get the reference by only holding the rcu
> > > read lock, but you should hold the vma lock to ensure that the vma
> > > itself (and not just the pointer) is safe to use.
> >
> > Hmm... To modify the vma, you must hold the mmap *and* vma write lock,
> > so holding the mmap read lock prevents mutations?
>
> Sorry, I think I confused things with my answer.  Your code is fine.
> The phrasing of the "only be used while the mmap read lock is still
> held" made me wonder about further clarification on the locking here
> (because the locking is confusing).
>
> Yes, mmap read lock means there are no writers that can modify the vma.
> Essentially, you are using the lock to ensure the entire vma space isn't
> changed during your operation - which is heavier than just locking one
> vma.

I could extend the safety comment like this:

SAFETY: We just checked that a vma was found, so the pointer is valid.
Furthermore, the returned area will borrow from this read lock guard,
so it can only be used while the mmap read lock is still held. This
ensures that there are no writers because writers must hold both the
mmap and vma write lock.

Alice