Re: [PATCH v6 0/5] mm: Unconditional per-VMA locks and cleanups

[Carlos Llamas <cmllamas@google.com>]

On Thu, Sep 03, 2026 at 04:48:31PM -0400, Liam R. Howlett wrote:
> On 26/08/31 11:13AM, Alice Ryhl wrote:
> > On Sat, Aug 29, 2026 at 06:56:25PM -0700, Andrew Morton wrote:
> > > On Thu, 13 Aug 2026 12:34:28 -0700 Suren Baghdasaryan <surenb@google.com> wrote:
> > > 
> > > > v2 version of this patchset [1] was written by Dave Hansen and per his
> > > > request, I'm taking over this series.
> > > > 
> > > > tl;dr: Make per-VMA locks available in all configs. Simplify some
> > > > of the per-VMA lock users now that they can rely on them being
> > > > always available.
> > > 
> > > It's been 2+ weeks so perhaps a refresh-and-remind would be helpful.
> > > 
> > > But it applies well enough and is adequately reviewed so I put it in
> > > there for testing, thanks.
> > > 
> > > AI review might have found a couple of pre-existing binder bugs:
> > > 
> > > 	https://sashiko.dev/#/patchset/20260813193433.3318288-1-surenb@google.com
> > > 
> > > and a small rusty thing which you might wish to attend to.
> > 
> > The binder bug is not actually a bug. When using VM_MIXEDMAP and
> > vm_insert_page(), the vma takes a refcount on the page, so there is no
> > use-after-free even if free_page() is invoked without removing it from
> > the vma.
> > 
> > Adding an INVARIANT: comment to the Rust code SGTM.
> > 
> 
> I think you are correct about no UAF here, but the page isn't exactly
> pinned to the vma - which is what I thought you were saying when I first
> read your reply.  It's sort of misplaced in another vma by an mremap().
> 
> vm_insert_page() will increment the ref count, but if the vma is
> mremap()'ed with the same size vma (ie, not expanding), then move_vma()
> will relocate the pte and the old vma will be closed and set the
> binder's mapped = false without a change to alloc->vm_start.
> 
> Binder now thinks there is no mapping but the mapping has an address so
> it can't map anything new.  You could get around it by replacing the
> vma, but I don't think that leads to anything interesting.
> 
> So we still have a ref count that's okay, but now binder has an
> alloc->vm_start that's stale and a mapped = false which leaves binder in
> a bad state (one might say a bind).

Right, binder should really reject mremap(). And partial munmap() too.
The is no use case for them in binder and it only brings problems such
as the stale alloc->vm_start you mention.

I sent out fixes for these issues here:
https://lore.kernel.org/all/20260901205250.1638304-1-cmllamas@google.com/

I'll Cc you on the next round if needed. Thanks Liam.

--
Carlos Llamas