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

[Liam R. Howlett]

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@xxxxxxxxxx> 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@xxxxxxxxxx
> > 
> > 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).

Thanks,
Liam