[RFC PATCH v2 2/2] binder: add install_mutex to serialize page install and shrinker zap

[Bo Zhang]

The previous patch converted alloc->mutex to a spinlock for the hot
path (buffer alloc/free). However, this leaves page installation and
shrinker's zap_vma_range() unserialized, which can cause use-after-free
as identified by Alice Ryhl.

Add a separate install_mutex to serialize page installation against
the shrinker's page reclaim (pages[index]=NULL + zap_vma_range). This
mutex is only contended on the cold path when pages need to be installed
or reclaimed, not on the hot path.

Key changes:

- binder_install_single_page() holds install_mutex across the entire
  install sequence, eliminating the need for binder_page_lookup() (GUP)
  since concurrent installers are now serialized.

- The shrinker holds install_mutex across pages[index]=NULL and
  zap_vma_range(), making them atomic to the install side.

- binder_lru_freelist_del() returns -EAGAIN when list_lru_del() fails
  (shrinker already isolated the page), with rollback of any pages
  already removed from the LRU. The caller waits on install_mutex for
  the shrinker to finish, then retries.

- EBUSY from vm_insert_page() is handled defensively with -EAGAIN,
  though it should not occur under install_mutex serialization.

Performance with memory pressure (binderThroughputTest, SM8850,
2 workers, 10 runs, concurrent drop_caches) shows no regression
from the install_mutex:

                    mutex (baseline)    spinlock + install_mutex
  throughput:        27k-59k iter/s       81k-94k iter/s
  average:           0.031-0.068ms        0.020-0.023ms
  P99:               0.088-0.148ms        0.047-0.061ms

Signed-off-by: Bo Zhang <zhangbo56@xxxxxxxxxx>
---
 drivers/android/binder_alloc.c | 75 ++++++++++++++++++++++++++--------
 drivers/android/binder_alloc.h |  3 ++
 2 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 9775df3616aa..f2cc822df5f7 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -325,24 +325,28 @@ static int binder_install_single_page(struct binder_alloc *alloc,
 		goto out;
 	}
 
+	mutex_lock(&alloc->install_mutex);
+
+	/* Check again under install_mutex */
+	if (binder_get_installed_page(alloc, index)) {
+		mutex_unlock(&alloc->install_mutex);
+		binder_free_page(page);
+		ret = 0;
+		goto out;
+	}
+
 	ret = binder_page_insert(alloc, addr, page);
 	switch (ret) {
 	case -EBUSY:
 		/*
-		 * EBUSY is ok. Someone installed the pte first but the
-		 * alloc->pages[index] has not been updated yet. Discard
-		 * our page and look up the one already installed.
+		 * This should not happen since install_mutex serializes
+		 * all page installations and shrinker zaps. Handle it
+		 * defensively by retrying.
 		 */
-		ret = 0;
 		binder_free_page(page);
-		page = binder_page_lookup(alloc, addr);
-		if (!page) {
-			pr_err("%d: failed to find page at offset %lx\n",
-			       alloc->pid, addr - alloc->vm_start);
-			ret = -ESRCH;
-			break;
-		}
-		fallthrough;
+		mutex_unlock(&alloc->install_mutex);
+		ret = -EAGAIN;
+		goto out;
 	case 0:
 		/* Mark page installation complete and safe to use */
 		binder_set_installed_page(alloc, index, page);
@@ -353,6 +357,8 @@ static int binder_install_single_page(struct binder_alloc *alloc,
 		       alloc->pid, __func__, addr - alloc->vm_start, ret);
 		break;
 	}
+
+	mutex_unlock(&alloc->install_mutex);
 out:
 	mmput_async(alloc->mm);
 	return ret;
@@ -377,8 +383,10 @@ static int binder_install_buffer_pages(struct binder_alloc *alloc,
 			continue;
 
 		trace_binder_alloc_page_start(alloc, index);
-
+retry:
 		ret = binder_install_single_page(alloc, index, page_addr);
+		if (ret == -EAGAIN)
+			goto retry;
 		if (ret)
 			return ret;
 
@@ -389,7 +397,7 @@ static int binder_install_buffer_pages(struct binder_alloc *alloc,
 }
 
 /* The range of pages should exclude those shared with other buffers */
-static void binder_lru_freelist_del(struct binder_alloc *alloc,
+static int binder_lru_freelist_del(struct binder_alloc *alloc,
 				    unsigned long start, unsigned long end)
 {
 	unsigned long page_addr;
@@ -411,7 +419,16 @@ static void binder_lru_freelist_del(struct binder_alloc *alloc,
 					      page_to_lru(page),
 					      page_to_nid(page),
 					      NULL);
-			WARN_ON(!on_lru);
+			/*
+			 * If !on_lru, the shrinker has already isolated this
+			 * page and will reclaim it. Abort so the caller can
+			 * retry after the shrinker finishes.
+			 */
+			if (!on_lru) {
+				/* Rollback pages already removed from LRU */
+				binder_lru_freelist_add(alloc, start, page_addr);
+				return -EAGAIN;
+			}
 
 			trace_binder_alloc_lru_end(alloc, index);
 			continue;
@@ -420,6 +437,8 @@ static void binder_lru_freelist_del(struct binder_alloc *alloc,
 		if (index + 1 > alloc->pages_high)
 			alloc->pages_high = index + 1;
 	}
+
+	return 0;
 }
 
 static void debug_no_space_locked(struct binder_alloc *alloc)
@@ -583,8 +602,12 @@ static struct binder_buffer *binder_alloc_new_buf_locked(
 	 */
 	next_used_page = (buffer->user_data + buffer_size) & PAGE_MASK;
 	curr_last_page = PAGE_ALIGN(buffer->user_data + size);
-	binder_lru_freelist_del(alloc, PAGE_ALIGN(buffer->user_data),
-				min(next_used_page, curr_last_page));
+	if (binder_lru_freelist_del(alloc, PAGE_ALIGN(buffer->user_data),
+				min(next_used_page, curr_last_page))) {
+		/* Shrinker is reclaiming a page we need. Undo and retry. */
+		buffer = ERR_PTR(-EAGAIN);
+		goto out;
+	}
 
 	rb_erase(&buffer->rb_node, &alloc->free_buffers);
 	buffer->free = 0;
@@ -676,10 +699,21 @@ struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
 	if (!next)
 		return ERR_PTR(-ENOMEM);
 
+retry:
 	spin_lock(&alloc->lock);
 	buffer = binder_alloc_new_buf_locked(alloc, next, size, is_async);
 	if (IS_ERR(buffer)) {
 		spin_unlock(&alloc->lock);
+		if (PTR_ERR(buffer) == -EAGAIN) {
+			/*
+			 * Shrinker is reclaiming a page we need.
+			 * Wait for it to finish by briefly acquiring
+			 * install_mutex, then retry.
+			 */
+			mutex_lock(&alloc->install_mutex);
+			mutex_unlock(&alloc->install_mutex);
+			goto retry;
+		}
 		goto out;
 	}
 
@@ -1175,7 +1209,6 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 	trace_binder_unmap_kernel_start(alloc, index);
 
 	page_to_free = alloc->pages[index];
-	binder_set_installed_page(alloc, index, NULL);
 
 	trace_binder_unmap_kernel_end(alloc, index);
 
@@ -1183,6 +1216,9 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 	spin_unlock(&alloc->lock);
 	spin_unlock(&lru->lock);
 
+	mutex_lock(&alloc->install_mutex);
+	binder_set_installed_page(alloc, index, NULL);
+
 	if (vma) {
 		trace_binder_unmap_user_start(alloc, index);
 
@@ -1191,6 +1227,8 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 		trace_binder_unmap_user_end(alloc, index);
 	}
 
+	mutex_unlock(&alloc->install_mutex);
+
 	if (mm_locked)
 		mmap_read_unlock(mm);
 	else
@@ -1236,6 +1274,7 @@ VISIBLE_IF_KUNIT void __binder_alloc_init(struct binder_alloc *alloc,
 	alloc->mm = current->mm;
 	mmgrab(alloc->mm);
 	spin_lock_init(&alloc->lock);
+	mutex_init(&alloc->install_mutex);
 	INIT_LIST_HEAD(&alloc->buffers);
 	alloc->freelist = freelist;
 }
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index bea5a77bb6da..85817efdbef6 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -9,6 +9,7 @@
 #include <linux/rbtree.h>
 #include <linux/list.h>
 #include <linux/mm.h>
+#include <linux/mutex.h>
 #include <linux/spinlock.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
@@ -81,6 +82,7 @@ static inline struct list_head *page_to_lru(struct page *p)
 /**
  * struct binder_alloc - per-binder proc state for binder allocator
  * @lock:               protects binder_alloc fields
+ * @install_mutex:      serializes page installation and shrinker zap
  * @mm:                 copy of task->mm (invariant after open)
  * @vm_start:           base of per-proc address space mapped via mmap
  * @buffers:            list of all buffers for this proc
@@ -106,6 +108,7 @@ static inline struct list_head *page_to_lru(struct page *p)
  */
 struct binder_alloc {
 	spinlock_t lock;
+	struct mutex install_mutex;
 	struct mm_struct *mm;
 	unsigned long vm_start;
 	struct list_head buffers;
-- 
2.34.1