From nobody Mon Oct 6 22:49:48 2025 Received: from mail-pf1-f202.google.com (mail-pf1-f202.google.com [209.85.210.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5A27E1DE3C3 for ; Thu, 17 Jul 2025 01:10:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.202 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752714627; cv=none; b=O2Q42ZiRCpXv5EO7YJZ5KKpWqS4HhVZH8+O7IbT1f/eH3p4zdmLR/+NAtqisRmR3NAv0rzzc+CqAvxXYL/izUbnlUyfUzAUs1p6EutJHMkfzLiXuUf/Knxxd6roqHhtnsQ+Xb5ezkiSRtovvIkvq495b/7CwHh+5+1dptb4IHDo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752714627; c=relaxed/simple; bh=K2awaZpAJuOtWqai3S5U+wUt64RXm5fZbbdjwuVbQ4Q=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=kp49KgzTPf1daU84+5Ec/+RDl6gdzRj/Re7Oh7N3Cq95qVtgiMn2vQw+vMO8we9T2BlnFBTBmdYIZX/HOpwiLa4l5ErBDrGdPgwgMCz2yl965Hc6OP2e0v70YzXKUilZYJKGOF113wam3epAnK1kv/TRACyw7QmrMhoF5OpDT+g= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--ynaffit.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=aYxkft2B; arc=none smtp.client-ip=209.85.210.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--ynaffit.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="aYxkft2B" Received: by mail-pf1-f202.google.com with SMTP id d2e1a72fcca58-740774348f6so440684b3a.1 for ; Wed, 16 Jul 2025 18:10:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1752714624; x=1753319424; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=X9F22aOeClVcZUalYp7Mw24VMJBartALFFChTIR9NkM=; b=aYxkft2BKX3DWg+q/OO1pL/4nKopwjpS7SpKuWVc/H6TNkqkBZxBdAITtRzQDdfqKM qXSqDOnP1WpPkj9pfRmi4weqbYOQUsvsMY2PhTk4VYuQtiZURWFqGyoUNW2shA38mCZ3 anrv38F3JQq2b9otxUqH/M0+GCZo+Vhap/6P2WFo8EDGQndOqHAry0lustxIIyZgIWbC wOO0hZjnpknTTnArGgfL0JHJ5njekckRd4OHlayYXwoRAVZjvPUr/fJYJZQl1f2O8Cm7 iaIbpTi2CkebHQrKGukSZdKBUtkU0WBdbawg/zsNM5RXXvObcF0CnCdm98ELY82/fhJG zpUg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752714624; x=1753319424; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=X9F22aOeClVcZUalYp7Mw24VMJBartALFFChTIR9NkM=; b=WJ+ILhtM/vqj8qCZNNEI2Rf4/Bhe1mh2ZBxQMuO5piyoMuwfhPxEvOvIYd4YeXhD8y HxmUuBifPDVipMh2xhqgAYxSeAbqpM4w4buLhpuUOLaeBd7bL6+7RqZJhFbaBRu+Fm9+ jmzW3++EfRsvLj8aviKxMIgi99gXlKzNv01BoVduvAHbrNCHKMXHyK9HmEkkj0iw86YF UaM2lM1Loy135vJAgS4AQgfJQILSMnAcw51d6N/OvFO871q4NL97HfBbEHZXbCfWxMfj LqulGtPvbEqAPtfyvwbXW9wiKgKevYNFvw/SHYxIKBA1pBGkdJ1MfeK+fzGtDxVgkSWj uvLg== X-Gm-Message-State: AOJu0YyWLgIIr23o0A9HOHG57KZmG5kix/xQCQdg3tfmHUrwXxbXXGDg koftLI8z1Qj25VAkjorq/17/tZ/up3WGPW3aAd1qPoM84lLZNwaoAFLK1QAtcndncQPbt/+9GiS 0HwhMrA5rEby+Tv7GvPemyr+XqOQdvk89qSGrymHUf9oZlVxvU0uSx+PKO05NHBwoTvsbIZCj/k zwmDWY6p/YLpgYAKp84aNblxhCQwCf4tr4dMtEHFnCuNG1GFJ5kQ== X-Google-Smtp-Source: AGHT+IHeyufygucPmjyKG7ZisqfH2+Ux0zXu28nZqMN6HeA12IFuzeWkyqSXm1zxXedyKEXss0ljJSp+0mor X-Received: from pfbhw13.prod.google.com ([2002:a05:6a00:890d:b0:748:e22c:600c]) (user=ynaffit job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a00:10ca:b0:740:6f69:8d94 with SMTP id d2e1a72fcca58-756e546e78cmr7290794b3a.0.1752714624390; Wed, 16 Jul 2025 18:10:24 -0700 (PDT) Date: Wed, 16 Jul 2025 18:10:05 -0700 In-Reply-To: <20250717011011.3365074-1-ynaffit@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250717011011.3365074-1-ynaffit@google.com> X-Mailer: git-send-email 2.50.0.727.gbf7dc18ff4-goog Message-ID: <20250717011011.3365074-3-ynaffit@google.com> Subject: [PATCH v4 2/6] binder: Store lru freelist in binder_alloc From: Tiffany Yang To: linux-kernel@vger.kernel.org Cc: keescook@google.com, kernel-team@android.com, Greg Kroah-Hartman , "=?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?=" , Todd Kjos , Martijn Coenen , Joel Fernandes , Christian Brauner , Carlos Llamas , Suren Baghdasaryan , Brendan Higgins , David Gow , Rae Moar , linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Store a pointer to the free pages list that the binder allocator should use for a process inside of struct binder_alloc. This change allows binder allocator code to be tested and debugged deterministically while a system is using binder; i.e., without interfering with other binder processes and independently of the shrinker. This is necessary to convert the current binder_alloc_selftest into a kunit test that does not rely on hijacking an existing binder_proc to run. A binder process's binder_alloc->freelist should not be changed after it is initialized. A sole exception is the process that runs the existing binder_alloc selftest. Its freelist can be temporarily replaced for the duration of the test because it runs as a single thread before any pages can be added to the global binder freelist, and the test frees every page it allocates before dropping the binder_selftest_lock. This exception allows the existing selftest to be used to check for regressions, but it will be dropped when the binder_alloc tests are converted to kunit in a subsequent patch in this series. Acked-by: Carlos Llamas Signed-off-by: Tiffany Yang Reviewed-by: Kees Cook --- v4: * Collected tag --- drivers/android/binder_alloc.c | 25 +++++++---- drivers/android/binder_alloc.h | 3 +- drivers/android/binder_alloc_selftest.c | 59 ++++++++++++++++++++----- 3 files changed, 67 insertions(+), 20 deletions(-) diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c index fcfaf1b899c8..2e89f9127883 100644 --- a/drivers/android/binder_alloc.c +++ b/drivers/android/binder_alloc.c @@ -26,7 +26,7 @@ #include "binder_alloc.h" #include "binder_trace.h" =20 -struct list_lru binder_freelist; +static struct list_lru binder_freelist; =20 static DEFINE_MUTEX(binder_alloc_mmap_lock); =20 @@ -210,7 +210,7 @@ static void binder_lru_freelist_add(struct binder_alloc= *alloc, =20 trace_binder_free_lru_start(alloc, index); =20 - ret =3D list_lru_add(&binder_freelist, + ret =3D list_lru_add(alloc->freelist, page_to_lru(page), page_to_nid(page), NULL); @@ -409,7 +409,7 @@ static void binder_lru_freelist_del(struct binder_alloc= *alloc, if (page) { trace_binder_alloc_lru_start(alloc, index); =20 - on_lru =3D list_lru_del(&binder_freelist, + on_lru =3D list_lru_del(alloc->freelist, page_to_lru(page), page_to_nid(page), NULL); @@ -1007,7 +1007,7 @@ void binder_alloc_deferred_release(struct binder_allo= c *alloc) if (!page) continue; =20 - on_lru =3D list_lru_del(&binder_freelist, + on_lru =3D list_lru_del(alloc->freelist, page_to_lru(page), page_to_nid(page), NULL); @@ -1229,6 +1229,17 @@ binder_shrink_scan(struct shrinker *shrink, struct s= hrink_control *sc) =20 static struct shrinker *binder_shrinker; =20 +static void __binder_alloc_init(struct binder_alloc *alloc, + struct list_lru *freelist) +{ + alloc->pid =3D current->group_leader->pid; + alloc->mm =3D current->mm; + mmgrab(alloc->mm); + mutex_init(&alloc->mutex); + INIT_LIST_HEAD(&alloc->buffers); + alloc->freelist =3D freelist; +} + /** * binder_alloc_init() - called by binder_open() for per-proc initializati= on * @alloc: binder_alloc for this proc @@ -1238,11 +1249,7 @@ static struct shrinker *binder_shrinker; */ void binder_alloc_init(struct binder_alloc *alloc) { - alloc->pid =3D current->group_leader->pid; - alloc->mm =3D current->mm; - mmgrab(alloc->mm); - mutex_init(&alloc->mutex); - INIT_LIST_HEAD(&alloc->buffers); + __binder_alloc_init(alloc, &binder_freelist); } =20 int binder_alloc_shrinker_init(void) diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h index feecd7414241..aa05a9df1360 100644 --- a/drivers/android/binder_alloc.h +++ b/drivers/android/binder_alloc.h @@ -15,7 +15,6 @@ #include #include =20 -extern struct list_lru binder_freelist; struct binder_transaction; =20 /** @@ -91,6 +90,7 @@ static inline struct list_head *page_to_lru(struct page *= p) * @free_async_space: VA space available for async buffers. This is * initialized at mmap time to 1/2 the full VA space * @pages: array of struct page * + * @freelist: lru list to use for free pages (invariant after in= it) * @buffer_size: size of address space specified via mmap * @pid: pid for associated binder_proc (invariant after in= it) * @pages_high: high watermark of offset in @pages @@ -113,6 +113,7 @@ struct binder_alloc { struct rb_root allocated_buffers; size_t free_async_space; struct page **pages; + struct list_lru *freelist; size_t buffer_size; int pid; size_t pages_high; diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/bind= er_alloc_selftest.c index de5bd848d042..8b18b22aa3de 100644 --- a/drivers/android/binder_alloc_selftest.c +++ b/drivers/android/binder_alloc_selftest.c @@ -8,8 +8,9 @@ =20 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt =20 -#include #include +#include +#include #include "binder_alloc.h" =20 #define BUFFER_NUM 5 @@ -18,6 +19,7 @@ static bool binder_selftest_run =3D true; static int binder_selftest_failures; static DEFINE_MUTEX(binder_selftest_lock); +static struct list_lru binder_selftest_freelist; =20 /** * enum buf_end_align_type - Page alignment of a buffer @@ -143,11 +145,6 @@ static void binder_selftest_free_buf(struct binder_all= oc *alloc, binder_alloc_free_buf(alloc, buffers[seq[i]]); =20 for (i =3D 0; i <=3D (end - 1) / PAGE_SIZE; i++) { - /** - * Error message on a free page can be false positive - * if binder shrinker ran during binder_alloc_free_buf - * calls above. - */ if (list_empty(page_to_lru(alloc->pages[i]))) { pr_err_size_seq(sizes, seq); pr_err("expect lru but is %s at page index %d\n", @@ -162,8 +159,8 @@ static void binder_selftest_free_page(struct binder_all= oc *alloc) int i; unsigned long count; =20 - while ((count =3D list_lru_count(&binder_freelist))) { - list_lru_walk(&binder_freelist, binder_alloc_free_page, + while ((count =3D list_lru_count(&binder_selftest_freelist))) { + list_lru_walk(&binder_selftest_freelist, binder_alloc_free_page, NULL, count); } =20 @@ -187,7 +184,7 @@ static void binder_selftest_alloc_free(struct binder_al= loc *alloc, =20 /* Allocate from lru. */ binder_selftest_alloc_buf(alloc, buffers, sizes, seq); - if (list_lru_count(&binder_freelist)) + if (list_lru_count(&binder_selftest_freelist)) pr_err("lru list should be empty but is not\n"); =20 binder_selftest_free_buf(alloc, buffers, sizes, seq, end); @@ -275,6 +272,20 @@ static void binder_selftest_alloc_offset(struct binder= _alloc *alloc, } } =20 +int binder_selftest_alloc_get_page_count(struct binder_alloc *alloc) +{ + struct page *page; + int allocated =3D 0; + int i; + + for (i =3D 0; i < alloc->buffer_size / PAGE_SIZE; i++) { + page =3D alloc->pages[i]; + if (page) + allocated++; + } + return allocated; +} + /** * binder_selftest_alloc() - Test alloc and free of buffer pages. * @alloc: Pointer to alloc struct. @@ -286,6 +297,7 @@ static void binder_selftest_alloc_offset(struct binder_= alloc *alloc, */ void binder_selftest_alloc(struct binder_alloc *alloc) { + struct list_lru *prev_freelist; size_t end_offset[BUFFER_NUM]; =20 if (!binder_selftest_run) @@ -293,14 +305,41 @@ void binder_selftest_alloc(struct binder_alloc *alloc) mutex_lock(&binder_selftest_lock); if (!binder_selftest_run || !alloc->mapped) goto done; + + prev_freelist =3D alloc->freelist; + + /* + * It is not safe to modify this process's alloc->freelist if it has any + * pages on a freelist. Since the test runs before any binder ioctls can + * be dealt with, none of its pages should be allocated yet. + */ + if (binder_selftest_alloc_get_page_count(alloc)) { + pr_err("process has existing alloc state\n"); + goto cleanup; + } + + if (list_lru_init(&binder_selftest_freelist)) { + pr_err("failed to init test freelist\n"); + goto cleanup; + } + + alloc->freelist =3D &binder_selftest_freelist; + pr_info("STARTED\n"); binder_selftest_alloc_offset(alloc, end_offset, 0); - binder_selftest_run =3D false; if (binder_selftest_failures > 0) pr_info("%d tests FAILED\n", binder_selftest_failures); else pr_info("PASSED\n"); =20 + if (list_lru_count(&binder_selftest_freelist)) + pr_err("expect test freelist to be empty\n"); + +cleanup: + /* Even if we didn't run the test, it's no longer thread-safe. */ + binder_selftest_run =3D false; + alloc->freelist =3D prev_freelist; + list_lru_destroy(&binder_selftest_freelist); done: mutex_unlock(&binder_selftest_lock); } --=20 2.50.0.727.gbf7dc18ff4-goog