Thread (19 messages) flat view 19 messages, 6 authors, 2d ago

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

From: Carlos Llamas <cmllamas@google.com>
Date: 2026-09-03 21:01:11
Also in: linux-mm, lkml

On Thu, Sep 03, 2026 at 04:48:31PM -0400, Liam R. Howlett wrote:
On 26/08/31 11:13AM, Alice Ryhl wrote:
quoted
On Sat, Aug 29, 2026 at 06:56:25PM -0700, Andrew Morton wrote:
quoted
On Thu, 13 Aug 2026 12:34:28 -0700 Suren Baghdasaryan [off-list ref] wrote:
quoted
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/ (local)

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

--
Carlos Llamas
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help