On Fri, Jun 12, 2026 at 09:54:58AM -0700, Dave Hansen wrote:

On 6/12/26 09:41, Suren Baghdasaryan wrote:

quoted

quoted

I think the key to distinguishing between:

        vma==NULL because there's no VMA
and
        vma==NULL because of a trylock failure

is binder_alloc_is_mapped(). It won't return false until vm_ops->close()
finishes. vm_ops->close() shouldn't be able to happen while
lock_vma_under_rcu() is held. So if you've got a non-NULL VMA, you've
also got a stable is binder_alloc_is_mapped().

By "stable binder_alloc_is_mapped()" do you mean it would always be
true?

By stable, I meant that it can't change.

	vma = lock_vma_under_rcu()
	mapped = binder_alloc_is_mapped();
	<window>
	vma_end_read(vma);

During <window> it can't go from true=>false or false=>true.

false=>true never happens from what I can tell. It's just plain
impossible given the current code.

true=>false is locked out because when lock_vma_under_rcu() is held.

quoted

Asking because in your patch you removed this condition:

-         if (vma && !binder_alloc_is_mapped(alloc))
-                  goto err_invalid_vma;

So, previously if we found the VMA but binder_alloc_is_mapped()==false
we would bail out and now we don't. Are you reasoning that this
combination is impossible?

It's not impossible, but I do think it is irrelevant. Or at least that
the *VMA* is irrelevant in this case. binder_alloc_is_mapped()==false
means that the binder VMA is gone. It's not in the maple tree, and it's
not coming back. If a VMA is found, it's an impostor.

That's why I did:

-        if (vma) {
+        if (mapped) {

The question isn't whether a VMA was found. The question is whether the
binder VMA is still mapped at page_addr. *That* is best inferred from
binder_alloc_is_mapped(), not the VMA lookup.

At least that's what I decided after staring at it for far too long.

Yes, I _think_ binder_alloc_is_mapped() can help distinguish between the
two scenarios (contention vs vma-close). However, I think it would be
simpler and safe to do an early exit:

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 88c3e1667d5b..9dd7d927249d 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c

@@ -1149,6 +1149,8 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 	 * for 'page_addr'.
 	 */
 	vma = lock_vma_under_rcu(mm, page_addr);
+	if (!vma && binder_alloc_is_mapped(alloc))
+		goto err_vma_lock_failed;
 
 	if (!mutex_trylock(&alloc->mutex))
 		goto err_get_alloc_mutex_failed;