[PATCH v3 0/6] binder: Set up KUnit tests for alloc

[PATCH v3 0/6] binder: Set up KUnit tests for alloc

[Tiffany Yang]

Hello,

binder_alloc_selftest provides a robust set of checks for the binder
allocator, but it rarely runs because it must hook into a running binder
process and block all other binder threads until it completes. The test
itself is a good candidate for conversion to KUnit, and it can be
further isolated from user processes by using a test-specific lru
freelist instead of the global one. This series converts the selftest
to KUnit to make it less burdensome to run and to set up a foundation
for unit testing future binder_alloc changes.

Thanks,
Tiffany

Tiffany Yang (6):
  binder: Fix selftest page indexing
  binder: Store lru freelist in binder_alloc
  kunit: test: Export kunit_attach_mm()
  binder: Scaffolding for binder_alloc KUnit tests
  binder: Convert binder_alloc selftests to KUnit
  binder: encapsulate individual alloc test cases

 drivers/android/Kconfig                    |  15 +-
 drivers/android/Makefile                   |   2 +-
 drivers/android/binder.c                   |  10 +-
 drivers/android/binder_alloc.c             |  39 +-
 drivers/android/binder_alloc.h             |  14 +-
 drivers/android/binder_alloc_selftest.c    | 306 -----------
 drivers/android/binder_internal.h          |   4 +
 drivers/android/tests/.kunitconfig         |   3 +
 drivers/android/tests/Makefile             |   3 +
 drivers/android/tests/binder_alloc_kunit.c | 573 +++++++++++++++++++++
 include/kunit/test.h                       |  12 +
 lib/kunit/user_alloc.c                     |   4 +-
 12 files changed, 645 insertions(+), 340 deletions(-)
 delete mode 100644 drivers/android/binder_alloc_selftest.c
 create mode 100644 drivers/android/tests/.kunitconfig
 create mode 100644 drivers/android/tests/Makefile
 create mode 100644 drivers/android/tests/binder_alloc_kunit.c

-- 
2.50.0.727.gbf7dc18ff4-goog

[PATCH v3 1/6] binder: Fix selftest page indexing

[Tiffany Yang]

The binder allocator selftest was only checking the last page of buffers
that ended on a page boundary. Correct the page indexing to account for
buffers that are not page-aligned.

Signed-off-by: Tiffany Yang <ynaffit@google.com>
---
 drivers/android/binder_alloc_selftest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/binder_alloc_selftest.c
index c88735c54848..486af3ec3c02 100644
--- a/drivers/android/binder_alloc_selftest.c
+++ b/drivers/android/binder_alloc_selftest.c
@@ -142,12 +142,12 @@ static void binder_selftest_free_buf(struct binder_alloc *alloc,
 	for (i = 0; i < BUFFER_NUM; i++)
 		binder_alloc_free_buf(alloc, buffers[seq[i]]);
 
-	for (i = 0; i < end / PAGE_SIZE; i++) {
 		/**
 		 * Error message on a free page can be false positive
 		 * if binder shrinker ran during binder_alloc_free_buf
 		 * calls above.
 		 */
+	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
 		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",
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v3 1/6] binder: Fix selftest page indexing

[Carlos Llamas]

On Mon, Jul 14, 2025 at 11:53:14AM -0700, Tiffany Yang wrote:
> The binder allocator selftest was only checking the last page of buffers
> that ended on a page boundary. Correct the page indexing to account for
> buffers that are not page-aligned.
>
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
>  drivers/android/binder_alloc_selftest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/binder_alloc_selftest.c
> index c88735c54848..486af3ec3c02 100644
> --- a/drivers/android/binder_alloc_selftest.c
> +++ b/drivers/android/binder_alloc_selftest.c
> @@ -142,12 +142,12 @@ static void binder_selftest_free_buf(struct binder_alloc *alloc,
>       for (i = 0; i < BUFFER_NUM; i++)
>               binder_alloc_free_buf(alloc, buffers[seq[i]]);
>
> -     for (i = 0; i < end / PAGE_SIZE; i++) {
>               /**
>                * Error message on a free page can be false positive
>                * if binder shrinker ran during binder_alloc_free_buf
>                * calls above.
>                */
> +     for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
>               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",
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

LGTM!

Acked-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH v3 1/6] binder: Fix selftest page indexing

[Kees Cook]

On Mon, Jul 14, 2025 at 11:53:14AM -0700, Tiffany Yang wrote:
> The binder allocator selftest was only checking the last page of buffers
> that ended on a page boundary. Correct the page indexing to account for
> buffers that are not page-aligned.
> 
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
>  drivers/android/binder_alloc_selftest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/binder_alloc_selftest.c
> index c88735c54848..486af3ec3c02 100644
> --- a/drivers/android/binder_alloc_selftest.c
> +++ b/drivers/android/binder_alloc_selftest.c
> @@ -142,12 +142,12 @@ static void binder_selftest_free_buf(struct binder_alloc *alloc,
>  	for (i = 0; i < BUFFER_NUM; i++)
>  		binder_alloc_free_buf(alloc, buffers[seq[i]]);
>  
> -	for (i = 0; i < end / PAGE_SIZE; i++) {
>  		/**
>  		 * Error message on a free page can be false positive
>  		 * if binder shrinker ran during binder_alloc_free_buf
>  		 * calls above.
>  		 */
> +	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {

Nit: this comment is now not aligned correctly. Probably the best would
be the leave the "for" line above the comment.

-Kees

>  		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",
> -- 
> 2.50.0.727.gbf7dc18ff4-goog
> 

-- 
Kees Cook

Re: [PATCH v3 1/6] binder: Fix selftest page indexing

[Tiffany Yang]

Kees Cook <kees@kernel.org> writes:

> On Mon, Jul 14, 2025 at 11:53:14AM -0700, Tiffany Yang wrote:
>> The binder allocator selftest was only checking the last page of buffers
>> that ended on a page boundary. Correct the page indexing to account for
>> buffers that are not page-aligned.

>> Signed-off-by: Tiffany Yang <ynaffit@google.com>
>> ---
>>   drivers/android/binder_alloc_selftest.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)

>> diff --git a/drivers/android/binder_alloc_selftest.c  
>> b/drivers/android/binder_alloc_selftest.c
>> index c88735c54848..486af3ec3c02 100644
>> --- a/drivers/android/binder_alloc_selftest.c
>> +++ b/drivers/android/binder_alloc_selftest.c
>> @@ -142,12 +142,12 @@ static void binder_selftest_free_buf(struct  
>> binder_alloc *alloc,
>>   	for (i = 0; i < BUFFER_NUM; i++)
>>   		binder_alloc_free_buf(alloc, buffers[seq[i]]);

>> -	for (i = 0; i < end / PAGE_SIZE; i++) {
>>   		/**
>>   		 * Error message on a free page can be false positive
>>   		 * if binder shrinker ran during binder_alloc_free_buf
>>   		 * calls above.
>>   		 */
>> +	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {

> Nit: this comment is now not aligned correctly. Probably the best would
> be the leave the "for" line above the comment.

> -Kees


Thank you for catching this! Will be fixed in v4.

>>   		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",
>> --
>> 2.50.0.727.gbf7dc18ff4-goog


-- 
Tiffany Y. Yang

[PATCH v3 2/6] binder: Store lru freelist in binder_alloc

[Tiffany Yang]

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.

Signed-off-by: Tiffany Yang <ynaffit@google.com>
---
 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"
 
-struct list_lru binder_freelist;
+static struct list_lru binder_freelist;
 
 static DEFINE_MUTEX(binder_alloc_mmap_lock);
 
@@ -210,7 +210,7 @@ static void binder_lru_freelist_add(struct binder_alloc *alloc,
 
 		trace_binder_free_lru_start(alloc, index);
 
-		ret = list_lru_add(&binder_freelist,
+		ret = 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);
 
-			on_lru = list_lru_del(&binder_freelist,
+			on_lru = 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_alloc *alloc)
 			if (!page)
 				continue;
 
-			on_lru = list_lru_del(&binder_freelist,
+			on_lru = 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 shrink_control *sc)
 
 static struct shrinker *binder_shrinker;
 
+static void __binder_alloc_init(struct binder_alloc *alloc,
+				struct list_lru *freelist)
+{
+	alloc->pid = current->group_leader->pid;
+	alloc->mm = current->mm;
+	mmgrab(alloc->mm);
+	mutex_init(&alloc->mutex);
+	INIT_LIST_HEAD(&alloc->buffers);
+	alloc->freelist = freelist;
+}
+
 /**
  * binder_alloc_init() - called by binder_open() for per-proc initialization
  * @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 = current->group_leader->pid;
-	alloc->mm = current->mm;
-	mmgrab(alloc->mm);
-	mutex_init(&alloc->mutex);
-	INIT_LIST_HEAD(&alloc->buffers);
+	__binder_alloc_init(alloc, &binder_freelist);
 }
 
 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 <linux/list_lru.h>
 #include <uapi/linux/android/binder.h>
 
-extern struct list_lru binder_freelist;
 struct binder_transaction;
 
 /**
@@ -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 init)
  * @buffer_size:        size of address space specified via mmap
  * @pid:                pid for associated binder_proc (invariant after init)
  * @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/binder_alloc_selftest.c
index 486af3ec3c02..8b18b22aa3de 100644
--- a/drivers/android/binder_alloc_selftest.c
+++ b/drivers/android/binder_alloc_selftest.c
@@ -8,8 +8,9 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/mm_types.h>
 #include <linux/err.h>
+#include <linux/list_lru.h>
+#include <linux/mm_types.h>
 #include "binder_alloc.h"
 
 #define BUFFER_NUM 5
@@ -18,6 +19,7 @@
 static bool binder_selftest_run = true;
 static int binder_selftest_failures;
 static DEFINE_MUTEX(binder_selftest_lock);
+static struct list_lru binder_selftest_freelist;
 
 /**
  * enum buf_end_align_type - Page alignment of a buffer
@@ -142,11 +144,6 @@ static void binder_selftest_free_buf(struct binder_alloc *alloc,
 	for (i = 0; i < BUFFER_NUM; i++)
 		binder_alloc_free_buf(alloc, buffers[seq[i]]);
 
-		/**
-		 * Error message on a free page can be false positive
-		 * if binder shrinker ran during binder_alloc_free_buf
-		 * calls above.
-		 */
 	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
 		if (list_empty(page_to_lru(alloc->pages[i]))) {
 			pr_err_size_seq(sizes, seq);
@@ -162,8 +159,8 @@ static void binder_selftest_free_page(struct binder_alloc *alloc)
 	int i;
 	unsigned long count;
 
-	while ((count = list_lru_count(&binder_freelist))) {
-		list_lru_walk(&binder_freelist, binder_alloc_free_page,
+	while ((count = list_lru_count(&binder_selftest_freelist))) {
+		list_lru_walk(&binder_selftest_freelist, binder_alloc_free_page,
 			      NULL, count);
 	}
 
@@ -187,7 +184,7 @@ static void binder_selftest_alloc_free(struct binder_alloc *alloc,
 
 	/* 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");
 
 	binder_selftest_free_buf(alloc, buffers, sizes, seq, end);
@@ -275,6 +272,20 @@ static void binder_selftest_alloc_offset(struct binder_alloc *alloc,
 	}
 }
 
+int binder_selftest_alloc_get_page_count(struct binder_alloc *alloc)
+{
+	struct page *page;
+	int allocated = 0;
+	int i;
+
+	for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) {
+		page = 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];
 
 	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 = 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 = &binder_selftest_freelist;
+
 	pr_info("STARTED\n");
 	binder_selftest_alloc_offset(alloc, end_offset, 0);
-	binder_selftest_run = false;
 	if (binder_selftest_failures > 0)
 		pr_info("%d tests FAILED\n", binder_selftest_failures);
 	else
 		pr_info("PASSED\n");
 
+	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 = false;
+	alloc->freelist = prev_freelist;
+	list_lru_destroy(&binder_selftest_freelist);
 done:
 	mutex_unlock(&binder_selftest_lock);
 }
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v3 2/6] binder: Store lru freelist in binder_alloc

[Carlos Llamas]

On Mon, Jul 14, 2025 at 11:53:15AM -0700, Tiffany Yang wrote:
> 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.
>
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---

LGTM!

Acked-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH v3 2/6] binder: Store lru freelist in binder_alloc

[Kees Cook]

On Mon, Jul 14, 2025 at 11:53:15AM -0700, Tiffany Yang wrote:
> 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.
> 
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
>  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"
>  
> -struct list_lru binder_freelist;
> +static struct list_lru binder_freelist;
>  
>  static DEFINE_MUTEX(binder_alloc_mmap_lock);
>  
> @@ -210,7 +210,7 @@ static void binder_lru_freelist_add(struct binder_alloc *alloc,
>  
>  		trace_binder_free_lru_start(alloc, index);
>  
> -		ret = list_lru_add(&binder_freelist,
> +		ret = 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);
>  
> -			on_lru = list_lru_del(&binder_freelist,
> +			on_lru = 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_alloc *alloc)
>  			if (!page)
>  				continue;
>  
> -			on_lru = list_lru_del(&binder_freelist,
> +			on_lru = 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 shrink_control *sc)
>  
>  static struct shrinker *binder_shrinker;
>  
> +static void __binder_alloc_init(struct binder_alloc *alloc,
> +				struct list_lru *freelist)
> +{
> +	alloc->pid = current->group_leader->pid;
> +	alloc->mm = current->mm;
> +	mmgrab(alloc->mm);
> +	mutex_init(&alloc->mutex);
> +	INIT_LIST_HEAD(&alloc->buffers);
> +	alloc->freelist = freelist;
> +}
> +
>  /**
>   * binder_alloc_init() - called by binder_open() for per-proc initialization
>   * @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 = current->group_leader->pid;
> -	alloc->mm = current->mm;
> -	mmgrab(alloc->mm);
> -	mutex_init(&alloc->mutex);
> -	INIT_LIST_HEAD(&alloc->buffers);
> +	__binder_alloc_init(alloc, &binder_freelist);
>  }
>  
>  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 <linux/list_lru.h>
>  #include <uapi/linux/android/binder.h>
>  
> -extern struct list_lru binder_freelist;
>  struct binder_transaction;
>  
>  /**
> @@ -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 init)
>   * @buffer_size:        size of address space specified via mmap
>   * @pid:                pid for associated binder_proc (invariant after init)
>   * @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/binder_alloc_selftest.c
> index 486af3ec3c02..8b18b22aa3de 100644
> --- a/drivers/android/binder_alloc_selftest.c
> +++ b/drivers/android/binder_alloc_selftest.c
> @@ -8,8 +8,9 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include <linux/mm_types.h>
>  #include <linux/err.h>
> +#include <linux/list_lru.h>
> +#include <linux/mm_types.h>
>  #include "binder_alloc.h"
>  
>  #define BUFFER_NUM 5
> @@ -18,6 +19,7 @@
>  static bool binder_selftest_run = true;
>  static int binder_selftest_failures;
>  static DEFINE_MUTEX(binder_selftest_lock);
> +static struct list_lru binder_selftest_freelist;
>  
>  /**
>   * enum buf_end_align_type - Page alignment of a buffer
> @@ -142,11 +144,6 @@ static void binder_selftest_free_buf(struct binder_alloc *alloc,
>  	for (i = 0; i < BUFFER_NUM; i++)
>  		binder_alloc_free_buf(alloc, buffers[seq[i]]);
>  
> -		/**
> -		 * Error message on a free page can be false positive
> -		 * if binder shrinker ran during binder_alloc_free_buf
> -		 * calls above.
> -		 */
>  	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
>  		if (list_empty(page_to_lru(alloc->pages[i]))) {
>  			pr_err_size_seq(sizes, seq);

Ah, I see the comment is removed entirely here. Better to move this
hunk to patch 1, then.

> @@ -162,8 +159,8 @@ static void binder_selftest_free_page(struct binder_alloc *alloc)
>  	int i;
>  	unsigned long count;
>  
> -	while ((count = list_lru_count(&binder_freelist))) {
> -		list_lru_walk(&binder_freelist, binder_alloc_free_page,
> +	while ((count = list_lru_count(&binder_selftest_freelist))) {
> +		list_lru_walk(&binder_selftest_freelist, binder_alloc_free_page,
>  			      NULL, count);
>  	}
>  
> @@ -187,7 +184,7 @@ static void binder_selftest_alloc_free(struct binder_alloc *alloc,
>  
>  	/* 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");
>  
>  	binder_selftest_free_buf(alloc, buffers, sizes, seq, end);
> @@ -275,6 +272,20 @@ static void binder_selftest_alloc_offset(struct binder_alloc *alloc,
>  	}
>  }
>  
> +int binder_selftest_alloc_get_page_count(struct binder_alloc *alloc)
> +{
> +	struct page *page;
> +	int allocated = 0;
> +	int i;
> +
> +	for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) {
> +		page = 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];
>  
>  	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 = 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 = &binder_selftest_freelist;
> +
>  	pr_info("STARTED\n");
>  	binder_selftest_alloc_offset(alloc, end_offset, 0);
> -	binder_selftest_run = false;
>  	if (binder_selftest_failures > 0)
>  		pr_info("%d tests FAILED\n", binder_selftest_failures);
>  	else
>  		pr_info("PASSED\n");
>  
> +	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 = false;
> +	alloc->freelist = prev_freelist;
> +	list_lru_destroy(&binder_selftest_freelist);
>  done:
>  	mutex_unlock(&binder_selftest_lock);
>  }
> -- 
> 2.50.0.727.gbf7dc18ff4-goog

Otherwise looks good.

-- 
Kees Cook

Re: [PATCH v3 2/6] binder: Store lru freelist in binder_alloc

[Tiffany Yang]

Kees Cook <kees@kernel.org> writes:
>> -		/**
>> -		 * Error message on a free page can be false positive
>> -		 * if binder shrinker ran during binder_alloc_free_buf
>> -		 * calls above.
>> -		 */
>>   	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
>>   		if (list_empty(page_to_lru(alloc->pages[i]))) {
>>   			pr_err_size_seq(sizes, seq);

> Ah, I see the comment is removed entirely here. Better to move this
> hunk to patch 1, then.


This comment is actually "true" until we allow a test-specific freelist
in this change, but I've moved the "for" line back to where it was in
the previous change. Thanks for noting this!

>> @@ -162,8 +159,8 @@ static void binder_selftest_free_page(struct  
>> binder_alloc *alloc)
>>   	int i;
>>   	unsigned long count;

>> -	while ((count = list_lru_count(&binder_freelist))) {
>> -		list_lru_walk(&binder_freelist, binder_alloc_free_page,
>> +	while ((count = list_lru_count(&binder_selftest_freelist))) {
>> +		list_lru_walk(&binder_selftest_freelist, binder_alloc_free_page,
>>   			      NULL, count);
>>   	}

>> @@ -187,7 +184,7 @@ static void binder_selftest_alloc_free(struct  
>> binder_alloc *alloc,

>>   	/* 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");

>>   	binder_selftest_free_buf(alloc, buffers, sizes, seq, end);
>> @@ -275,6 +272,20 @@ static void binder_selftest_alloc_offset(struct  
>> binder_alloc *alloc,
>>   	}
>>   }

>> +int binder_selftest_alloc_get_page_count(struct binder_alloc *alloc)
>> +{
>> +	struct page *page;
>> +	int allocated = 0;
>> +	int i;
>> +
>> +	for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) {
>> +		page = 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];

>>   	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 = 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 = &binder_selftest_freelist;
>> +
>>   	pr_info("STARTED\n");
>>   	binder_selftest_alloc_offset(alloc, end_offset, 0);
>> -	binder_selftest_run = false;
>>   	if (binder_selftest_failures > 0)
>>   		pr_info("%d tests FAILED\n", binder_selftest_failures);
>>   	else
>>   		pr_info("PASSED\n");

>> +	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 = false;
>> +	alloc->freelist = prev_freelist;
>> +	list_lru_destroy(&binder_selftest_freelist);
>>   done:
>>   	mutex_unlock(&binder_selftest_lock);
>>   }
>> --
>> 2.50.0.727.gbf7dc18ff4-goog

> Otherwise looks good.

-- 
Tiffany Y. Yang

[PATCH v3 3/6] kunit: test: Export kunit_attach_mm()

[Tiffany Yang]

Tests can allocate from virtual memory using kunit_vm_mmap(), which
transparently creates and attaches an mm_struct to the test runner if
one is not already attached. This is suitable for most cases, except for
when the code under test must access a task's mm before performing an
mmap. Expose kunit_attach_mm() as part of the interface for those
cases. This does not change the existing behavior.

Cc: David Gow <davidgow@google.com>
Signed-off-by: Tiffany Yang <ynaffit@google.com>
---
 include/kunit/test.h   | 12 ++++++++++++
 lib/kunit/user_alloc.c |  4 ++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 39c768f87dc9..d958ee53050e 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -531,6 +531,18 @@ static inline char *kunit_kstrdup(struct kunit *test, const char *str, gfp_t gfp
  */
 const char *kunit_kstrdup_const(struct kunit *test, const char *str, gfp_t gfp);
 
+/**
+ * kunit_attach_mm() - Create and attach a new mm if it doesn't already exist.
+ *
+ * Allocates a &struct mm_struct and attaches it to @current. In most cases, call
+ * kunit_vm_mmap() without calling kunit_attach_mm() directly. Only necessary when
+ * code under test accesses the mm before executing the mmap (e.g., to perform
+ * additional initialization beforehand).
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+int kunit_attach_mm(void);
+
 /**
  * kunit_vm_mmap() - Allocate KUnit-tracked vm_mmap() area
  * @test: The test context object.
diff --git a/lib/kunit/user_alloc.c b/lib/kunit/user_alloc.c
index 46951be018be..b8cac765e620 100644
--- a/lib/kunit/user_alloc.c
+++ b/lib/kunit/user_alloc.c
@@ -22,8 +22,7 @@ struct kunit_vm_mmap_params {
 	unsigned long offset;
 };
 
-/* Create and attach a new mm if it doesn't already exist. */
-static int kunit_attach_mm(void)
+int kunit_attach_mm(void)
 {
 	struct mm_struct *mm;
 
@@ -49,6 +48,7 @@ static int kunit_attach_mm(void)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(kunit_attach_mm);
 
 static int kunit_vm_mmap_init(struct kunit_resource *res, void *context)
 {
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v3 3/6] kunit: test: Export kunit_attach_mm()

[Carlos Llamas]

On Mon, Jul 14, 2025 at 11:53:16AM -0700, Tiffany Yang wrote:
> Tests can allocate from virtual memory using kunit_vm_mmap(), which
> transparently creates and attaches an mm_struct to the test runner if
> one is not already attached. This is suitable for most cases, except for
> when the code under test must access a task's mm before performing an
> mmap. Expose kunit_attach_mm() as part of the interface for those
> cases. This does not change the existing behavior.
>
> Cc: David Gow <davidgow@google.com>
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
>  include/kunit/test.h   | 12 ++++++++++++
>  lib/kunit/user_alloc.c |  4 ++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 39c768f87dc9..d958ee53050e 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -531,6 +531,18 @@ static inline char *kunit_kstrdup(struct kunit *test, const char *str, gfp_t gfp
>   */
>  const char *kunit_kstrdup_const(struct kunit *test, const char *str, gfp_t gfp);
>
> +/**
> + * kunit_attach_mm() - Create and attach a new mm if it doesn't already exist.
> + *
> + * Allocates a &struct mm_struct and attaches it to @current. In most cases, call
> + * kunit_vm_mmap() without calling kunit_attach_mm() directly. Only necessary when
> + * code under test accesses the mm before executing the mmap (e.g., to perform
> + * additional initialization beforehand).
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +int kunit_attach_mm(void);
> +
>  /**
>   * kunit_vm_mmap() - Allocate KUnit-tracked vm_mmap() area
>   * @test: The test context object.
> diff --git a/lib/kunit/user_alloc.c b/lib/kunit/user_alloc.c
> index 46951be018be..b8cac765e620 100644
> --- a/lib/kunit/user_alloc.c
> +++ b/lib/kunit/user_alloc.c
> @@ -22,8 +22,7 @@ struct kunit_vm_mmap_params {
>       unsigned long offset;
>  };
>
> -/* Create and attach a new mm if it doesn't already exist. */
> -static int kunit_attach_mm(void)
> +int kunit_attach_mm(void)
>  {
>       struct mm_struct *mm;
>
> @@ -49,6 +48,7 @@ static int kunit_attach_mm(void)
>
>       return 0;
>  }
> +EXPORT_SYMBOL_GPL(kunit_attach_mm);
>
>  static int kunit_vm_mmap_init(struct kunit_resource *res, void *context)
>  {
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

LGTM!

Reviewed-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH v3 3/6] kunit: test: Export kunit_attach_mm()

[Kees Cook]

On Mon, Jul 14, 2025 at 11:53:16AM -0700, Tiffany Yang wrote:
> Tests can allocate from virtual memory using kunit_vm_mmap(), which
> transparently creates and attaches an mm_struct to the test runner if
> one is not already attached. This is suitable for most cases, except for
> when the code under test must access a task's mm before performing an
> mmap. Expose kunit_attach_mm() as part of the interface for those
> cases. This does not change the existing behavior.
> 
> Cc: David Gow <davidgow@google.com>
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
>  include/kunit/test.h   | 12 ++++++++++++
>  lib/kunit/user_alloc.c |  4 ++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 39c768f87dc9..d958ee53050e 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -531,6 +531,18 @@ static inline char *kunit_kstrdup(struct kunit *test, const char *str, gfp_t gfp
>   */
>  const char *kunit_kstrdup_const(struct kunit *test, const char *str, gfp_t gfp);
>  
> +/**
> + * kunit_attach_mm() - Create and attach a new mm if it doesn't already exist.
> + *
> + * Allocates a &struct mm_struct and attaches it to @current. In most cases, call
> + * kunit_vm_mmap() without calling kunit_attach_mm() directly. Only necessary when
> + * code under test accesses the mm before executing the mmap (e.g., to perform
> + * additional initialization beforehand).
> + *
> + * Return: 0 on success, -errno on failure.
> + */

Yay kern-doc! :)

> +int kunit_attach_mm(void);
> +
>  /**
>   * kunit_vm_mmap() - Allocate KUnit-tracked vm_mmap() area
>   * @test: The test context object.
> diff --git a/lib/kunit/user_alloc.c b/lib/kunit/user_alloc.c
> index 46951be018be..b8cac765e620 100644
> --- a/lib/kunit/user_alloc.c
> +++ b/lib/kunit/user_alloc.c
> @@ -22,8 +22,7 @@ struct kunit_vm_mmap_params {
>  	unsigned long offset;
>  };
>  
> -/* Create and attach a new mm if it doesn't already exist. */
> -static int kunit_attach_mm(void)
> +int kunit_attach_mm(void)
>  {
>  	struct mm_struct *mm;
>  
> @@ -49,6 +48,7 @@ static int kunit_attach_mm(void)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(kunit_attach_mm);
>  
>  static int kunit_vm_mmap_init(struct kunit_resource *res, void *context)
>  {
> -- 
> 2.50.0.727.gbf7dc18ff4-goog

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

[PATCH v3 4/6] binder: Scaffolding for binder_alloc KUnit tests

[Tiffany Yang]

Add setup and teardown for testing binder allocator code with KUnit.
Include minimal test cases to verify that tests are initialized
correctly.

Tested-by: Rae Moar <rmoar@google.com>
Signed-off-by: Tiffany Yang <ynaffit@google.com>
---
v2:
* Added tested-by tag
v3:
* Split kunit lib change into separate change
---
 drivers/android/Kconfig                    |  11 ++
 drivers/android/Makefile                   |   1 +
 drivers/android/binder.c                   |   5 +-
 drivers/android/binder_alloc.c             |  15 +-
 drivers/android/binder_alloc.h             |   6 +
 drivers/android/binder_internal.h          |   4 +
 drivers/android/tests/.kunitconfig         |   3 +
 drivers/android/tests/Makefile             |   3 +
 drivers/android/tests/binder_alloc_kunit.c | 166 +++++++++++++++++++++
 9 files changed, 208 insertions(+), 6 deletions(-)
 create mode 100644 drivers/android/tests/.kunitconfig
 create mode 100644 drivers/android/tests/Makefile
 create mode 100644 drivers/android/tests/binder_alloc_kunit.c

diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index 07aa8ae0a058..b1bc7183366c 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -47,4 +47,15 @@ config ANDROID_BINDER_IPC_SELFTEST
 	  exhaustively with combinations of various buffer sizes and
 	  alignments.
 
+config ANDROID_BINDER_ALLOC_KUNIT_TEST
+	tristate "KUnit Tests for Android Binder Alloc" if !KUNIT_ALL_TESTS
+	depends on ANDROID_BINDER_IPC && KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This feature builds the binder alloc KUnit tests.
+
+	  Each test case runs using a pared-down binder_alloc struct and
+	  test-specific freelist, which allows this KUnit module to be loaded
+	  for testing without interfering with a running system.
+
 endmenu
diff --git a/drivers/android/Makefile b/drivers/android/Makefile
index c9d3d0c99c25..74d02a335d4e 100644
--- a/drivers/android/Makefile
+++ b/drivers/android/Makefile
@@ -4,3 +4,4 @@ ccflags-y += -I$(src)			# needed for trace events
 obj-$(CONFIG_ANDROID_BINDERFS)		+= binderfs.o
 obj-$(CONFIG_ANDROID_BINDER_IPC)	+= binder.o binder_alloc.o
 obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
+obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)	+= tests/
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index c463ca4a8fff..9dfe90c284fc 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -68,6 +68,8 @@
 #include <linux/sizes.h>
 #include <linux/ktime.h>
 
+#include <kunit/visibility.h>
+
 #include <uapi/linux/android/binder.h>
 
 #include <linux/cacheflush.h>
@@ -5956,10 +5958,11 @@ static void binder_vma_close(struct vm_area_struct *vma)
 	binder_alloc_vma_close(&proc->alloc);
 }
 
-static vm_fault_t binder_vm_fault(struct vm_fault *vmf)
+VISIBLE_IF_KUNIT vm_fault_t binder_vm_fault(struct vm_fault *vmf)
 {
 	return VM_FAULT_SIGBUS;
 }
+EXPORT_SYMBOL_IF_KUNIT(binder_vm_fault);
 
 static const struct vm_operations_struct binder_vm_ops = {
 	.open = binder_vma_open,
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 2e89f9127883..c79e5c6721f0 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -23,6 +23,7 @@
 #include <linux/uaccess.h>
 #include <linux/highmem.h>
 #include <linux/sizes.h>
+#include <kunit/visibility.h>
 #include "binder_alloc.h"
 #include "binder_trace.h"
 
@@ -57,13 +58,14 @@ static struct binder_buffer *binder_buffer_prev(struct binder_buffer *buffer)
 	return list_entry(buffer->entry.prev, struct binder_buffer, entry);
 }
 
-static size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
-				       struct binder_buffer *buffer)
+VISIBLE_IF_KUNIT size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
+						 struct binder_buffer *buffer)
 {
 	if (list_is_last(&buffer->entry, &alloc->buffers))
 		return alloc->vm_start + alloc->buffer_size - buffer->user_data;
 	return binder_buffer_next(buffer)->user_data - buffer->user_data;
 }
+EXPORT_SYMBOL_IF_KUNIT(binder_alloc_buffer_size);
 
 static void binder_insert_free_buffer(struct binder_alloc *alloc,
 				      struct binder_buffer *new_buffer)
@@ -959,7 +961,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
 			   failure_string, ret);
 	return ret;
 }
-
+EXPORT_SYMBOL_IF_KUNIT(binder_alloc_mmap_handler);
 
 void binder_alloc_deferred_release(struct binder_alloc *alloc)
 {
@@ -1028,6 +1030,7 @@ void binder_alloc_deferred_release(struct binder_alloc *alloc)
 		     "%s: %d buffers %d, pages %d\n",
 		     __func__, alloc->pid, buffers, page_count);
 }
+EXPORT_SYMBOL_IF_KUNIT(binder_alloc_deferred_release);
 
 /**
  * binder_alloc_print_allocated() - print buffer info
@@ -1122,6 +1125,7 @@ void binder_alloc_vma_close(struct binder_alloc *alloc)
 {
 	binder_alloc_set_mapped(alloc, false);
 }
+EXPORT_SYMBOL_IF_KUNIT(binder_alloc_vma_close);
 
 /**
  * binder_alloc_free_page() - shrinker callback to free pages
@@ -1229,8 +1233,8 @@ binder_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 
 static struct shrinker *binder_shrinker;
 
-static void __binder_alloc_init(struct binder_alloc *alloc,
-				struct list_lru *freelist)
+VISIBLE_IF_KUNIT void __binder_alloc_init(struct binder_alloc *alloc,
+					  struct list_lru *freelist)
 {
 	alloc->pid = current->group_leader->pid;
 	alloc->mm = current->mm;
@@ -1239,6 +1243,7 @@ static void __binder_alloc_init(struct binder_alloc *alloc,
 	INIT_LIST_HEAD(&alloc->buffers);
 	alloc->freelist = freelist;
 }
+EXPORT_SYMBOL_IF_KUNIT(__binder_alloc_init);
 
 /**
  * binder_alloc_init() - called by binder_open() for per-proc initialization
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index aa05a9df1360..dc8dce2469a7 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -188,5 +188,11 @@ int binder_alloc_copy_from_buffer(struct binder_alloc *alloc,
 				  binder_size_t buffer_offset,
 				  size_t bytes);
 
+#if IS_ENABLED(CONFIG_KUNIT)
+void __binder_alloc_init(struct binder_alloc *alloc, struct list_lru *freelist);
+size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
+				struct binder_buffer *buffer);
+#endif
+
 #endif /* _LINUX_BINDER_ALLOC_H */
 
diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
index 1ba5caf1d88d..b5d3014fb4dc 100644
--- a/drivers/android/binder_internal.h
+++ b/drivers/android/binder_internal.h
@@ -592,4 +592,8 @@ void binder_add_device(struct binder_device *device);
  */
 void binder_remove_device(struct binder_device *device);
 
+#if IS_ENABLED(CONFIG_KUNIT)
+vm_fault_t binder_vm_fault(struct vm_fault *vmf);
+#endif
+
 #endif /* _LINUX_BINDER_INTERNAL_H */
diff --git a/drivers/android/tests/.kunitconfig b/drivers/android/tests/.kunitconfig
new file mode 100644
index 000000000000..a73601231049
--- /dev/null
+++ b/drivers/android/tests/.kunitconfig
@@ -0,0 +1,3 @@
+CONFIG_KUNIT=y
+CONFIG_ANDROID_BINDER_IPC=y
+CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST=y
diff --git a/drivers/android/tests/Makefile b/drivers/android/tests/Makefile
new file mode 100644
index 000000000000..6780967e573b
--- /dev/null
+++ b/drivers/android/tests/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)	+= binder_alloc_kunit.o
diff --git a/drivers/android/tests/binder_alloc_kunit.c b/drivers/android/tests/binder_alloc_kunit.c
new file mode 100644
index 000000000000..4b68b5687d33
--- /dev/null
+++ b/drivers/android/tests/binder_alloc_kunit.c
@@ -0,0 +1,166 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test cases for binder allocator code
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <kunit/test.h>
+#include <linux/anon_inodes.h>
+#include <linux/err.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include <linux/mman.h>
+#include <linux/sizes.h>
+
+#include "../binder_alloc.h"
+#include "../binder_internal.h"
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+
+#define BINDER_MMAP_SIZE SZ_128K
+
+struct binder_alloc_test {
+	struct binder_alloc alloc;
+	struct list_lru binder_test_freelist;
+	struct file *filp;
+	unsigned long mmap_uaddr;
+};
+
+static void binder_alloc_test_init_freelist(struct kunit *test)
+{
+	struct binder_alloc_test *priv = test->priv;
+
+	KUNIT_EXPECT_PTR_EQ(test, priv->alloc.freelist,
+			    &priv->binder_test_freelist);
+}
+
+static void binder_alloc_test_mmap(struct kunit *test)
+{
+	struct binder_alloc_test *priv = test->priv;
+	struct binder_alloc *alloc = &priv->alloc;
+	struct binder_buffer *buf;
+	struct rb_node *n;
+
+	KUNIT_EXPECT_EQ(test, alloc->mapped, true);
+	KUNIT_EXPECT_EQ(test, alloc->buffer_size, BINDER_MMAP_SIZE);
+
+	n = rb_first(&alloc->allocated_buffers);
+	KUNIT_EXPECT_PTR_EQ(test, n, NULL);
+
+	n = rb_first(&alloc->free_buffers);
+	buf = rb_entry(n, struct binder_buffer, rb_node);
+	KUNIT_EXPECT_EQ(test, binder_alloc_buffer_size(alloc, buf),
+			BINDER_MMAP_SIZE);
+	KUNIT_EXPECT_TRUE(test, list_is_last(&buf->entry, &alloc->buffers));
+}
+
+/* ===== End test cases ===== */
+
+static void binder_alloc_test_vma_close(struct vm_area_struct *vma)
+{
+	struct binder_alloc *alloc = vma->vm_private_data;
+
+	binder_alloc_vma_close(alloc);
+}
+
+static const struct vm_operations_struct binder_alloc_test_vm_ops = {
+	.close = binder_alloc_test_vma_close,
+	.fault = binder_vm_fault,
+};
+
+static int binder_alloc_test_mmap_handler(struct file *filp,
+					  struct vm_area_struct *vma)
+{
+	struct binder_alloc *alloc = filp->private_data;
+
+	vm_flags_mod(vma, VM_DONTCOPY | VM_MIXEDMAP, VM_MAYWRITE);
+
+	vma->vm_ops = &binder_alloc_test_vm_ops;
+	vma->vm_private_data = alloc;
+
+	return binder_alloc_mmap_handler(alloc, vma);
+}
+
+static const struct file_operations binder_alloc_test_fops = {
+	.mmap = binder_alloc_test_mmap_handler,
+};
+
+static int binder_alloc_test_init(struct kunit *test)
+{
+	struct binder_alloc_test *priv;
+	int ret;
+
+	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	test->priv = priv;
+
+	ret = list_lru_init(&priv->binder_test_freelist);
+	if (ret) {
+		kunit_err(test, "Failed to initialize test freelist\n");
+		return ret;
+	}
+
+	/* __binder_alloc_init requires mm to be attached */
+	ret = kunit_attach_mm();
+	if (ret) {
+		kunit_err(test, "Failed to attach mm\n");
+		return ret;
+	}
+	__binder_alloc_init(&priv->alloc, &priv->binder_test_freelist);
+
+	priv->filp = anon_inode_getfile("binder_alloc_kunit",
+					&binder_alloc_test_fops, &priv->alloc,
+					O_RDWR | O_CLOEXEC);
+	if (IS_ERR_OR_NULL(priv->filp)) {
+		kunit_err(test, "Failed to open binder alloc test driver file\n");
+		return priv->filp ? PTR_ERR(priv->filp) : -ENOMEM;
+	}
+
+	priv->mmap_uaddr = kunit_vm_mmap(test, priv->filp, 0, BINDER_MMAP_SIZE,
+					 PROT_READ, MAP_PRIVATE | MAP_NORESERVE,
+					 0);
+	if (!priv->mmap_uaddr) {
+		kunit_err(test, "Could not map the test's transaction memory\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static void binder_alloc_test_exit(struct kunit *test)
+{
+	struct binder_alloc_test *priv = test->priv;
+
+	/* Close the backing file to make sure binder_alloc_vma_close runs */
+	if (!IS_ERR_OR_NULL(priv->filp))
+		fput(priv->filp);
+
+	if (priv->alloc.mm)
+		binder_alloc_deferred_release(&priv->alloc);
+
+	/* Make sure freelist is empty */
+	KUNIT_EXPECT_EQ(test, list_lru_count(&priv->binder_test_freelist), 0);
+	list_lru_destroy(&priv->binder_test_freelist);
+}
+
+static struct kunit_case binder_alloc_test_cases[] = {
+	KUNIT_CASE(binder_alloc_test_init_freelist),
+	KUNIT_CASE(binder_alloc_test_mmap),
+	{}
+};
+
+static struct kunit_suite binder_alloc_test_suite = {
+	.name = "binder_alloc",
+	.test_cases = binder_alloc_test_cases,
+	.init = binder_alloc_test_init,
+	.exit = binder_alloc_test_exit,
+};
+
+kunit_test_suite(binder_alloc_test_suite);
+
+MODULE_AUTHOR("Tiffany Yang <ynaffit@google.com>");
+MODULE_DESCRIPTION("Binder Alloc KUnit tests");
+MODULE_LICENSE("GPL");
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v3 4/6] binder: Scaffolding for binder_alloc KUnit tests

[Carlos Llamas]

On Mon, Jul 14, 2025 at 11:53:17AM -0700, Tiffany Yang wrote:
> Add setup and teardown for testing binder allocator code with KUnit.
> Include minimal test cases to verify that tests are initialized
> correctly.
>
> Tested-by: Rae Moar <rmoar@google.com>
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
> v2:
> * Added tested-by tag
> v3:
> * Split kunit lib change into separate change
> ---
>  drivers/android/Kconfig                    |  11 ++
>  drivers/android/Makefile                   |   1 +
>  drivers/android/binder.c                   |   5 +-
>  drivers/android/binder_alloc.c             |  15 +-
>  drivers/android/binder_alloc.h             |   6 +
>  drivers/android/binder_internal.h          |   4 +
>  drivers/android/tests/.kunitconfig         |   3 +
>  drivers/android/tests/Makefile             |   3 +
>  drivers/android/tests/binder_alloc_kunit.c | 166 +++++++++++++++++++++
>  9 files changed, 208 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/android/tests/.kunitconfig
>  create mode 100644 drivers/android/tests/Makefile
>  create mode 100644 drivers/android/tests/binder_alloc_kunit.c
>
> diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
> index 07aa8ae0a058..b1bc7183366c 100644
> --- a/drivers/android/Kconfig
> +++ b/drivers/android/Kconfig
> @@ -47,4 +47,15 @@ config ANDROID_BINDER_IPC_SELFTEST
>         exhaustively with combinations of various buffer sizes and
>         alignments.
>
> +config ANDROID_BINDER_ALLOC_KUNIT_TEST

I think is OK to have "alloc" in the name. However, this is probably
going to grow into just "binder" at some point (hopefully). Then we can
just rename the kunit config as such.

> +     tristate "KUnit Tests for Android Binder Alloc" if !KUNIT_ALL_TESTS
> +     depends on ANDROID_BINDER_IPC && KUNIT
> +     default KUNIT_ALL_TESTS
> +     help
> +       This feature builds the binder alloc KUnit tests.
> +
> +       Each test case runs using a pared-down binder_alloc struct and
> +       test-specific freelist, which allows this KUnit module to be loaded
> +       for testing without interfering with a running system.
> +
>  endmenu
> diff --git a/drivers/android/Makefile b/drivers/android/Makefile
> index c9d3d0c99c25..74d02a335d4e 100644
> --- a/drivers/android/Makefile
> +++ b/drivers/android/Makefile
> @@ -4,3 +4,4 @@ ccflags-y += -I$(src)                 # needed for trace events
>  obj-$(CONFIG_ANDROID_BINDERFS)               += binderfs.o
>  obj-$(CONFIG_ANDROID_BINDER_IPC)     += binder.o binder_alloc.o
>  obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
> +obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)        += tests/
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c463ca4a8fff..9dfe90c284fc 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -68,6 +68,8 @@
>  #include <linux/sizes.h>
>  #include <linux/ktime.h>
>
> +#include <kunit/visibility.h>
> +
>  #include <uapi/linux/android/binder.h>
>
>  #include <linux/cacheflush.h>
> @@ -5956,10 +5958,11 @@ static void binder_vma_close(struct vm_area_struct *vma)
>       binder_alloc_vma_close(&proc->alloc);
>  }
>
> -static vm_fault_t binder_vm_fault(struct vm_fault *vmf)
> +VISIBLE_IF_KUNIT vm_fault_t binder_vm_fault(struct vm_fault *vmf)
>  {
>       return VM_FAULT_SIGBUS;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_vm_fault);
>
>  static const struct vm_operations_struct binder_vm_ops = {
>       .open = binder_vma_open,
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index 2e89f9127883..c79e5c6721f0 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -23,6 +23,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/highmem.h>
>  #include <linux/sizes.h>
> +#include <kunit/visibility.h>
>  #include "binder_alloc.h"
>  #include "binder_trace.h"
>
> @@ -57,13 +58,14 @@ static struct binder_buffer *binder_buffer_prev(struct binder_buffer *buffer)
>       return list_entry(buffer->entry.prev, struct binder_buffer, entry);
>  }
>
> -static size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
> -                                    struct binder_buffer *buffer)
> +VISIBLE_IF_KUNIT size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
> +                                              struct binder_buffer *buffer)
>  {
>       if (list_is_last(&buffer->entry, &alloc->buffers))
>               return alloc->vm_start + alloc->buffer_size - buffer->user_data;
>       return binder_buffer_next(buffer)->user_data - buffer->user_data;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_buffer_size);
>
>  static void binder_insert_free_buffer(struct binder_alloc *alloc,
>                                     struct binder_buffer *new_buffer)
> @@ -959,7 +961,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
>                          failure_string, ret);
>       return ret;
>  }
> -
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_mmap_handler);
>
>  void binder_alloc_deferred_release(struct binder_alloc *alloc)
>  {
> @@ -1028,6 +1030,7 @@ void binder_alloc_deferred_release(struct binder_alloc *alloc)
>                    "%s: %d buffers %d, pages %d\n",
>                    __func__, alloc->pid, buffers, page_count);
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_deferred_release);
>
>  /**
>   * binder_alloc_print_allocated() - print buffer info
> @@ -1122,6 +1125,7 @@ void binder_alloc_vma_close(struct binder_alloc *alloc)
>  {
>       binder_alloc_set_mapped(alloc, false);
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_vma_close);
>
>  /**
>   * binder_alloc_free_page() - shrinker callback to free pages
> @@ -1229,8 +1233,8 @@ binder_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>
>  static struct shrinker *binder_shrinker;
>
> -static void __binder_alloc_init(struct binder_alloc *alloc,
> -                             struct list_lru *freelist)
> +VISIBLE_IF_KUNIT void __binder_alloc_init(struct binder_alloc *alloc,
> +                                       struct list_lru *freelist)
>  {
>       alloc->pid = current->group_leader->pid;
>       alloc->mm = current->mm;
> @@ -1239,6 +1243,7 @@ static void __binder_alloc_init(struct binder_alloc *alloc,
>       INIT_LIST_HEAD(&alloc->buffers);
>       alloc->freelist = freelist;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(__binder_alloc_init);
>
>  /**
>   * binder_alloc_init() - called by binder_open() for per-proc initialization
> diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
> index aa05a9df1360..dc8dce2469a7 100644
> --- a/drivers/android/binder_alloc.h
> +++ b/drivers/android/binder_alloc.h
> @@ -188,5 +188,11 @@ int binder_alloc_copy_from_buffer(struct binder_alloc *alloc,
>                                 binder_size_t buffer_offset,
>                                 size_t bytes);
>
> +#if IS_ENABLED(CONFIG_KUNIT)
> +void __binder_alloc_init(struct binder_alloc *alloc, struct list_lru *freelist);
> +size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
> +                             struct binder_buffer *buffer);
> +#endif
> +
>  #endif /* _LINUX_BINDER_ALLOC_H */
>
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> index 1ba5caf1d88d..b5d3014fb4dc 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -592,4 +592,8 @@ void binder_add_device(struct binder_device *device);
>   */
>  void binder_remove_device(struct binder_device *device);
>
> +#if IS_ENABLED(CONFIG_KUNIT)
> +vm_fault_t binder_vm_fault(struct vm_fault *vmf);
> +#endif
> +
>  #endif /* _LINUX_BINDER_INTERNAL_H */
> diff --git a/drivers/android/tests/.kunitconfig b/drivers/android/tests/.kunitconfig
> new file mode 100644
> index 000000000000..a73601231049
> --- /dev/null
> +++ b/drivers/android/tests/.kunitconfig
> @@ -0,0 +1,3 @@
> +CONFIG_KUNIT=y
> +CONFIG_ANDROID_BINDER_IPC=y
> +CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST=y
> diff --git a/drivers/android/tests/Makefile b/drivers/android/tests/Makefile
> new file mode 100644
> index 000000000000..6780967e573b
> --- /dev/null
> +++ b/drivers/android/tests/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only

All the new files need the copyright notice. Something like:
        "Copyright 2025 Google LCC"

> +
> +obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)        += binder_alloc_kunit.o
> diff --git a/drivers/android/tests/binder_alloc_kunit.c b/drivers/android/tests/binder_alloc_kunit.c
> new file mode 100644
> index 000000000000..4b68b5687d33
> --- /dev/null
> +++ b/drivers/android/tests/binder_alloc_kunit.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Test cases for binder allocator code
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <kunit/test.h>
> +#include <linux/anon_inodes.h>
> +#include <linux/err.h>
> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/mm.h>
> +#include <linux/mman.h>
> +#include <linux/sizes.h>
> +
> +#include "../binder_alloc.h"
> +#include "../binder_internal.h"
> +
> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
> +
> +#define BINDER_MMAP_SIZE SZ_128K
> +
> +struct binder_alloc_test {
> +     struct binder_alloc alloc;
> +     struct list_lru binder_test_freelist;
> +     struct file *filp;
> +     unsigned long mmap_uaddr;
> +};
> +
> +static void binder_alloc_test_init_freelist(struct kunit *test)
> +{
> +     struct binder_alloc_test *priv = test->priv;
> +
> +     KUNIT_EXPECT_PTR_EQ(test, priv->alloc.freelist,
> +                         &priv->binder_test_freelist);
> +}
> +
> +static void binder_alloc_test_mmap(struct kunit *test)
> +{
> +     struct binder_alloc_test *priv = test->priv;
> +     struct binder_alloc *alloc = &priv->alloc;
> +     struct binder_buffer *buf;
> +     struct rb_node *n;
> +
> +     KUNIT_EXPECT_EQ(test, alloc->mapped, true);
> +     KUNIT_EXPECT_EQ(test, alloc->buffer_size, BINDER_MMAP_SIZE);
> +
> +     n = rb_first(&alloc->allocated_buffers);
> +     KUNIT_EXPECT_PTR_EQ(test, n, NULL);
> +
> +     n = rb_first(&alloc->free_buffers);
> +     buf = rb_entry(n, struct binder_buffer, rb_node);
> +     KUNIT_EXPECT_EQ(test, binder_alloc_buffer_size(alloc, buf),
> +                     BINDER_MMAP_SIZE);
> +     KUNIT_EXPECT_TRUE(test, list_is_last(&buf->entry, &alloc->buffers));
> +}
> +
> +/* ===== End test cases ===== */
> +
> +static void binder_alloc_test_vma_close(struct vm_area_struct *vma)
> +{
> +     struct binder_alloc *alloc = vma->vm_private_data;
> +
> +     binder_alloc_vma_close(alloc);
> +}
> +
> +static const struct vm_operations_struct binder_alloc_test_vm_ops = {
> +     .close = binder_alloc_test_vma_close,
> +     .fault = binder_vm_fault,
> +};
> +
> +static int binder_alloc_test_mmap_handler(struct file *filp,
> +                                       struct vm_area_struct *vma)
> +{
> +     struct binder_alloc *alloc = filp->private_data;
> +
> +     vm_flags_mod(vma, VM_DONTCOPY | VM_MIXEDMAP, VM_MAYWRITE);
> +
> +     vma->vm_ops = &binder_alloc_test_vm_ops;
> +     vma->vm_private_data = alloc;
> +
> +     return binder_alloc_mmap_handler(alloc, vma);
> +}
> +
> +static const struct file_operations binder_alloc_test_fops = {
> +     .mmap = binder_alloc_test_mmap_handler,
> +};
> +
> +static int binder_alloc_test_init(struct kunit *test)
> +{
> +     struct binder_alloc_test *priv;
> +     int ret;
> +
> +     priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
> +     if (!priv)
> +             return -ENOMEM;
> +     test->priv = priv;
> +
> +     ret = list_lru_init(&priv->binder_test_freelist);
> +     if (ret) {
> +             kunit_err(test, "Failed to initialize test freelist\n");
> +             return ret;
> +     }
> +
> +     /* __binder_alloc_init requires mm to be attached */
> +     ret = kunit_attach_mm();
> +     if (ret) {
> +             kunit_err(test, "Failed to attach mm\n");
> +             return ret;
> +     }
> +     __binder_alloc_init(&priv->alloc, &priv->binder_test_freelist);
> +
> +     priv->filp = anon_inode_getfile("binder_alloc_kunit",
> +                                     &binder_alloc_test_fops, &priv->alloc,
> +                                     O_RDWR | O_CLOEXEC);
> +     if (IS_ERR_OR_NULL(priv->filp)) {
> +             kunit_err(test, "Failed to open binder alloc test driver file\n");
> +             return priv->filp ? PTR_ERR(priv->filp) : -ENOMEM;
> +     }
> +
> +     priv->mmap_uaddr = kunit_vm_mmap(test, priv->filp, 0, BINDER_MMAP_SIZE,
> +                                      PROT_READ, MAP_PRIVATE | MAP_NORESERVE,
> +                                      0);
> +     if (!priv->mmap_uaddr) {
> +             kunit_err(test, "Could not map the test's transaction memory\n");
> +             return -ENOMEM;
> +     }
> +
> +     return 0;
> +}
> +
> +static void binder_alloc_test_exit(struct kunit *test)
> +{
> +     struct binder_alloc_test *priv = test->priv;
> +
> +     /* Close the backing file to make sure binder_alloc_vma_close runs */
> +     if (!IS_ERR_OR_NULL(priv->filp))
> +             fput(priv->filp);
> +
> +     if (priv->alloc.mm)
> +             binder_alloc_deferred_release(&priv->alloc);
> +
> +     /* Make sure freelist is empty */
> +     KUNIT_EXPECT_EQ(test, list_lru_count(&priv->binder_test_freelist), 0);
> +     list_lru_destroy(&priv->binder_test_freelist);
> +}
> +
> +static struct kunit_case binder_alloc_test_cases[] = {
> +     KUNIT_CASE(binder_alloc_test_init_freelist),
> +     KUNIT_CASE(binder_alloc_test_mmap),
> +     {}
> +};
> +
> +static struct kunit_suite binder_alloc_test_suite = {
> +     .name = "binder_alloc",
> +     .test_cases = binder_alloc_test_cases,
> +     .init = binder_alloc_test_init,
> +     .exit = binder_alloc_test_exit,
> +};
> +
> +kunit_test_suite(binder_alloc_test_suite);
> +
> +MODULE_AUTHOR("Tiffany Yang <ynaffit@google.com>");
> +MODULE_DESCRIPTION("Binder Alloc KUnit tests");
> +MODULE_LICENSE("GPL");
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

Aside from the missing Copyright, feel free to add:

Acked-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH v3 4/6] binder: Scaffolding for binder_alloc KUnit tests

[Tiffany Yang]

Carlos Llamas <cmllamas@google.com> writes:

> On Mon, Jul 14, 2025 at 11:53:17AM -0700, Tiffany Yang wrote:
>> Add setup and teardown for testing binder allocator code with KUnit.
>> Include minimal test cases to verify that tests are initialized
>> correctly.

>> Tested-by: Rae Moar <rmoar@google.com>
>> Signed-off-by: Tiffany Yang <ynaffit@google.com>
>> ---
>> v2:
>> * Added tested-by tag
>> v3:
>> * Split kunit lib change into separate change
>> ---
>>   drivers/android/Kconfig                    |  11 ++
>>   drivers/android/Makefile                   |   1 +
>>   drivers/android/binder.c                   |   5 +-
>>   drivers/android/binder_alloc.c             |  15 +-
>>   drivers/android/binder_alloc.h             |   6 +
>>   drivers/android/binder_internal.h          |   4 +
>>   drivers/android/tests/.kunitconfig         |   3 +
>>   drivers/android/tests/Makefile             |   3 +
>>   drivers/android/tests/binder_alloc_kunit.c | 166 +++++++++++++++++++++
>>   9 files changed, 208 insertions(+), 6 deletions(-)
>>   create mode 100644 drivers/android/tests/.kunitconfig
>>   create mode 100644 drivers/android/tests/Makefile
>>   create mode 100644 drivers/android/tests/binder_alloc_kunit.c

>> diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
>> index 07aa8ae0a058..b1bc7183366c 100644
>> --- a/drivers/android/Kconfig
>> +++ b/drivers/android/Kconfig
>> @@ -47,4 +47,15 @@ config ANDROID_BINDER_IPC_SELFTEST
>>          exhaustively with combinations of various buffer sizes and
>>          alignments.

>> +config ANDROID_BINDER_ALLOC_KUNIT_TEST

> I think is OK to have "alloc" in the name. However, this is probably
> going to grow into just "binder" at some point (hopefully). Then we can
> just rename the kunit config as such.


My original thinking was that it would be nice to have multiple configs
for various binder tests (i.e., if we wanted to have binderfs unit tests
and other binder driver unit tests), but upon reflection, I'm not sure
what purpose that would serve... I'll leave the "alloc" in it for now,
but I agree we'll probably remove it for the future binder kunit tests.

>> +     tristate "KUnit Tests for Android Binder Alloc" if !KUNIT_ALL_TESTS
>> +     depends on ANDROID_BINDER_IPC && KUNIT
>> +     default KUNIT_ALL_TESTS
>> +     help
>> +       This feature builds the binder alloc KUnit tests.
>> +
>> +       Each test case runs using a pared-down binder_alloc struct and
>> +       test-specific freelist, which allows this KUnit module to be  
>> loaded
>> +       for testing without interfering with a running system.
>> +
>>   endmenu
>> diff --git a/drivers/android/Makefile b/drivers/android/Makefile
>> index c9d3d0c99c25..74d02a335d4e 100644
>> --- a/drivers/android/Makefile
>> +++ b/drivers/android/Makefile
>> @@ -4,3 +4,4 @@ ccflags-y += -I$(src)                 # needed for trace  
>> events
>>   obj-$(CONFIG_ANDROID_BINDERFS)               += binderfs.o
>>   obj-$(CONFIG_ANDROID_BINDER_IPC)     += binder.o binder_alloc.o
>>   obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
>> +obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)        += tests/
>> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
>> index c463ca4a8fff..9dfe90c284fc 100644
>> --- a/drivers/android/binder.c
>> +++ b/drivers/android/binder.c
>> @@ -68,6 +68,8 @@
>>   #include <linux/sizes.h>
>>   #include <linux/ktime.h>

>> +#include <kunit/visibility.h>
>> +
>>   #include <uapi/linux/android/binder.h>

>>   #include <linux/cacheflush.h>
>> @@ -5956,10 +5958,11 @@ static void binder_vma_close(struct  
>> vm_area_struct *vma)
>>        binder_alloc_vma_close(&proc->alloc);
>>   }

>> -static vm_fault_t binder_vm_fault(struct vm_fault *vmf)
>> +VISIBLE_IF_KUNIT vm_fault_t binder_vm_fault(struct vm_fault *vmf)
>>   {
>>        return VM_FAULT_SIGBUS;
>>   }
>> +EXPORT_SYMBOL_IF_KUNIT(binder_vm_fault);

>>   static const struct vm_operations_struct binder_vm_ops = {
>>        .open = binder_vma_open,
>> diff --git a/drivers/android/binder_alloc.c  
>> b/drivers/android/binder_alloc.c
>> index 2e89f9127883..c79e5c6721f0 100644
>> --- a/drivers/android/binder_alloc.c
>> +++ b/drivers/android/binder_alloc.c
>> @@ -23,6 +23,7 @@
>>   #include <linux/uaccess.h>
>>   #include <linux/highmem.h>
>>   #include <linux/sizes.h>
>> +#include <kunit/visibility.h>
>>   #include "binder_alloc.h"
>>   #include "binder_trace.h"

>> @@ -57,13 +58,14 @@ static struct binder_buffer  
>> *binder_buffer_prev(struct binder_buffer *buffer)
>>        return list_entry(buffer->entry.prev, struct binder_buffer, entry);
>>   }

>> -static size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
>> -                                    struct binder_buffer *buffer)
>> +VISIBLE_IF_KUNIT size_t binder_alloc_buffer_size(struct binder_alloc  
>> *alloc,
>> +                                              struct binder_buffer  
>> *buffer)
>>   {
>>        if (list_is_last(&buffer->entry, &alloc->buffers))
>>                return alloc->vm_start + alloc->buffer_size -  
>> buffer->user_data;
>>        return binder_buffer_next(buffer)->user_data - buffer->user_data;
>>   }
>> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_buffer_size);

>>   static void binder_insert_free_buffer(struct binder_alloc *alloc,
>>                                      struct binder_buffer *new_buffer)
>> @@ -959,7 +961,7 @@ int binder_alloc_mmap_handler(struct binder_alloc  
>> *alloc,
>>                           failure_string, ret);
>>        return ret;
>>   }
>> -
>> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_mmap_handler);

>>   void binder_alloc_deferred_release(struct binder_alloc *alloc)
>>   {
>> @@ -1028,6 +1030,7 @@ void binder_alloc_deferred_release(struct  
>> binder_alloc *alloc)
>>                     "%s: %d buffers %d, pages %d\n",
>>                     __func__, alloc->pid, buffers, page_count);
>>   }
>> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_deferred_release);

>>   /**
>>    * binder_alloc_print_allocated() - print buffer info
>> @@ -1122,6 +1125,7 @@ void binder_alloc_vma_close(struct binder_alloc  
>> *alloc)
>>   {
>>        binder_alloc_set_mapped(alloc, false);
>>   }
>> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_vma_close);

>>   /**
>>    * binder_alloc_free_page() - shrinker callback to free pages
>> @@ -1229,8 +1233,8 @@ binder_shrink_scan(struct shrinker *shrink, struct  
>> shrink_control *sc)

>>   static struct shrinker *binder_shrinker;

>> -static void __binder_alloc_init(struct binder_alloc *alloc,
>> -                             struct list_lru *freelist)
>> +VISIBLE_IF_KUNIT void __binder_alloc_init(struct binder_alloc *alloc,
>> +                                       struct list_lru *freelist)
>>   {
>>        alloc->pid = current->group_leader->pid;
>>        alloc->mm = current->mm;
>> @@ -1239,6 +1243,7 @@ static void __binder_alloc_init(struct  
>> binder_alloc *alloc,
>>        INIT_LIST_HEAD(&alloc->buffers);
>>        alloc->freelist = freelist;
>>   }
>> +EXPORT_SYMBOL_IF_KUNIT(__binder_alloc_init);

>>   /**
>>    * binder_alloc_init() - called by binder_open() for per-proc  
>> initialization
>> diff --git a/drivers/android/binder_alloc.h  
>> b/drivers/android/binder_alloc.h
>> index aa05a9df1360..dc8dce2469a7 100644
>> --- a/drivers/android/binder_alloc.h
>> +++ b/drivers/android/binder_alloc.h
>> @@ -188,5 +188,11 @@ int binder_alloc_copy_from_buffer(struct  
>> binder_alloc *alloc,
>>                                  binder_size_t buffer_offset,
>>                                  size_t bytes);

>> +#if IS_ENABLED(CONFIG_KUNIT)
>> +void __binder_alloc_init(struct binder_alloc *alloc, struct list_lru  
>> *freelist);
>> +size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
>> +                             struct binder_buffer *buffer);
>> +#endif
>> +
>>   #endif /* _LINUX_BINDER_ALLOC_H */

>> diff --git a/drivers/android/binder_internal.h  
>> b/drivers/android/binder_internal.h
>> index 1ba5caf1d88d..b5d3014fb4dc 100644
>> --- a/drivers/android/binder_internal.h
>> +++ b/drivers/android/binder_internal.h
>> @@ -592,4 +592,8 @@ void binder_add_device(struct binder_device *device);
>>    */
>>   void binder_remove_device(struct binder_device *device);

>> +#if IS_ENABLED(CONFIG_KUNIT)
>> +vm_fault_t binder_vm_fault(struct vm_fault *vmf);
>> +#endif
>> +
>>   #endif /* _LINUX_BINDER_INTERNAL_H */
>> diff --git a/drivers/android/tests/.kunitconfig  
>> b/drivers/android/tests/.kunitconfig
>> new file mode 100644
>> index 000000000000..a73601231049
>> --- /dev/null
>> +++ b/drivers/android/tests/.kunitconfig
>> @@ -0,0 +1,3 @@
>> +CONFIG_KUNIT=y
>> +CONFIG_ANDROID_BINDER_IPC=y
>> +CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST=y
>> diff --git a/drivers/android/tests/Makefile  
>> b/drivers/android/tests/Makefile
>> new file mode 100644
>> index 000000000000..6780967e573b
>> --- /dev/null
>> +++ b/drivers/android/tests/Makefile
>> @@ -0,0 +1,3 @@
>> +# SPDX-License-Identifier: GPL-2.0-only

> All the new files need the copyright notice. Something like:
>          "Copyright 2025 Google LCC"


Done.

>> +
>> +obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)        +=  
>> binder_alloc_kunit.o
>> diff --git a/drivers/android/tests/binder_alloc_kunit.c  
>> b/drivers/android/tests/binder_alloc_kunit.c
>> new file mode 100644
>> index 000000000000..4b68b5687d33
>> --- /dev/null
>> +++ b/drivers/android/tests/binder_alloc_kunit.c
>> @@ -0,0 +1,166 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Test cases for binder allocator code
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <kunit/test.h>
>> +#include <linux/anon_inodes.h>
>> +#include <linux/err.h>
>> +#include <linux/file.h>
>> +#include <linux/fs.h>
>> +#include <linux/mm.h>
>> +#include <linux/mman.h>
>> +#include <linux/sizes.h>
>> +
>> +#include "../binder_alloc.h"
>> +#include "../binder_internal.h"
>> +
>> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
>> +
>> +#define BINDER_MMAP_SIZE SZ_128K
>> +
>> +struct binder_alloc_test {
>> +     struct binder_alloc alloc;
>> +     struct list_lru binder_test_freelist;
>> +     struct file *filp;
>> +     unsigned long mmap_uaddr;
>> +};
>> +
>> +static void binder_alloc_test_init_freelist(struct kunit *test)
>> +{
>> +     struct binder_alloc_test *priv = test->priv;
>> +
>> +     KUNIT_EXPECT_PTR_EQ(test, priv->alloc.freelist,
>> +                         &priv->binder_test_freelist);
>> +}
>> +
>> +static void binder_alloc_test_mmap(struct kunit *test)
>> +{
>> +     struct binder_alloc_test *priv = test->priv;
>> +     struct binder_alloc *alloc = &priv->alloc;
>> +     struct binder_buffer *buf;
>> +     struct rb_node *n;
>> +
>> +     KUNIT_EXPECT_EQ(test, alloc->mapped, true);
>> +     KUNIT_EXPECT_EQ(test, alloc->buffer_size, BINDER_MMAP_SIZE);
>> +
>> +     n = rb_first(&alloc->allocated_buffers);
>> +     KUNIT_EXPECT_PTR_EQ(test, n, NULL);
>> +
>> +     n = rb_first(&alloc->free_buffers);
>> +     buf = rb_entry(n, struct binder_buffer, rb_node);
>> +     KUNIT_EXPECT_EQ(test, binder_alloc_buffer_size(alloc, buf),
>> +                     BINDER_MMAP_SIZE);
>> +     KUNIT_EXPECT_TRUE(test, list_is_last(&buf->entry,  
>> &alloc->buffers));
>> +}
>> +
>> +/* ===== End test cases ===== */
>> +
>> +static void binder_alloc_test_vma_close(struct vm_area_struct *vma)
>> +{
>> +     struct binder_alloc *alloc = vma->vm_private_data;
>> +
>> +     binder_alloc_vma_close(alloc);
>> +}
>> +
>> +static const struct vm_operations_struct binder_alloc_test_vm_ops = {
>> +     .close = binder_alloc_test_vma_close,
>> +     .fault = binder_vm_fault,
>> +};
>> +
>> +static int binder_alloc_test_mmap_handler(struct file *filp,
>> +                                       struct vm_area_struct *vma)
>> +{
>> +     struct binder_alloc *alloc = filp->private_data;
>> +
>> +     vm_flags_mod(vma, VM_DONTCOPY | VM_MIXEDMAP, VM_MAYWRITE);
>> +
>> +     vma->vm_ops = &binder_alloc_test_vm_ops;
>> +     vma->vm_private_data = alloc;
>> +
>> +     return binder_alloc_mmap_handler(alloc, vma);
>> +}
>> +
>> +static const struct file_operations binder_alloc_test_fops = {
>> +     .mmap = binder_alloc_test_mmap_handler,
>> +};
>> +
>> +static int binder_alloc_test_init(struct kunit *test)
>> +{
>> +     struct binder_alloc_test *priv;
>> +     int ret;
>> +
>> +     priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
>> +     if (!priv)
>> +             return -ENOMEM;
>> +     test->priv = priv;
>> +
>> +     ret = list_lru_init(&priv->binder_test_freelist);
>> +     if (ret) {
>> +             kunit_err(test, "Failed to initialize test freelist\n");
>> +             return ret;
>> +     }
>> +
>> +     /* __binder_alloc_init requires mm to be attached */
>> +     ret = kunit_attach_mm();
>> +     if (ret) {
>> +             kunit_err(test, "Failed to attach mm\n");
>> +             return ret;
>> +     }
>> +     __binder_alloc_init(&priv->alloc, &priv->binder_test_freelist);
>> +
>> +     priv->filp = anon_inode_getfile("binder_alloc_kunit",
>> +                                     &binder_alloc_test_fops,  
>> &priv->alloc,
>> +                                     O_RDWR | O_CLOEXEC);
>> +     if (IS_ERR_OR_NULL(priv->filp)) {
>> +             kunit_err(test, "Failed to open binder alloc test driver  
>> file\n");
>> +             return priv->filp ? PTR_ERR(priv->filp) : -ENOMEM;
>> +     }
>> +
>> +     priv->mmap_uaddr = kunit_vm_mmap(test, priv->filp, 0,  
>> BINDER_MMAP_SIZE,
>> +                                      PROT_READ, MAP_PRIVATE |  
>> MAP_NORESERVE,
>> +                                      0);
>> +     if (!priv->mmap_uaddr) {
>> +             kunit_err(test, "Could not map the test's transaction  
>> memory\n");
>> +             return -ENOMEM;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static void binder_alloc_test_exit(struct kunit *test)
>> +{
>> +     struct binder_alloc_test *priv = test->priv;
>> +
>> +     /* Close the backing file to make sure binder_alloc_vma_close runs  
>> */
>> +     if (!IS_ERR_OR_NULL(priv->filp))
>> +             fput(priv->filp);
>> +
>> +     if (priv->alloc.mm)
>> +             binder_alloc_deferred_release(&priv->alloc);
>> +
>> +     /* Make sure freelist is empty */
>> +     KUNIT_EXPECT_EQ(test, list_lru_count(&priv->binder_test_freelist),  
>> 0);
>> +     list_lru_destroy(&priv->binder_test_freelist);
>> +}
>> +
>> +static struct kunit_case binder_alloc_test_cases[] = {
>> +     KUNIT_CASE(binder_alloc_test_init_freelist),
>> +     KUNIT_CASE(binder_alloc_test_mmap),
>> +     {}
>> +};
>> +
>> +static struct kunit_suite binder_alloc_test_suite = {
>> +     .name = "binder_alloc",
>> +     .test_cases = binder_alloc_test_cases,
>> +     .init = binder_alloc_test_init,
>> +     .exit = binder_alloc_test_exit,
>> +};
>> +
>> +kunit_test_suite(binder_alloc_test_suite);
>> +
>> +MODULE_AUTHOR("Tiffany Yang <ynaffit@google.com>");
>> +MODULE_DESCRIPTION("Binder Alloc KUnit tests");
>> +MODULE_LICENSE("GPL");
>> --
>> 2.50.0.727.gbf7dc18ff4-goog


> Aside from the missing Copyright, feel free to add:

> Acked-by: Carlos Llamas <cmllamas@google.com>

Thanks for the review!

-- 
Tiffany Y. Yang

Re: [PATCH v3 4/6] binder: Scaffolding for binder_alloc KUnit tests

[Kees Cook]

On Mon, Jul 14, 2025 at 11:53:17AM -0700, Tiffany Yang wrote:
> Add setup and teardown for testing binder allocator code with KUnit.
> Include minimal test cases to verify that tests are initialized
> correctly.
> 
> Tested-by: Rae Moar <rmoar@google.com>
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
> v2:
> * Added tested-by tag
> v3:
> * Split kunit lib change into separate change
> ---
>  drivers/android/Kconfig                    |  11 ++
>  drivers/android/Makefile                   |   1 +
>  drivers/android/binder.c                   |   5 +-
>  drivers/android/binder_alloc.c             |  15 +-
>  drivers/android/binder_alloc.h             |   6 +
>  drivers/android/binder_internal.h          |   4 +
>  drivers/android/tests/.kunitconfig         |   3 +
>  drivers/android/tests/Makefile             |   3 +
>  drivers/android/tests/binder_alloc_kunit.c | 166 +++++++++++++++++++++
>  9 files changed, 208 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/android/tests/.kunitconfig
>  create mode 100644 drivers/android/tests/Makefile
>  create mode 100644 drivers/android/tests/binder_alloc_kunit.c
> 
> diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
> index 07aa8ae0a058..b1bc7183366c 100644
> --- a/drivers/android/Kconfig
> +++ b/drivers/android/Kconfig
> @@ -47,4 +47,15 @@ config ANDROID_BINDER_IPC_SELFTEST
>  	  exhaustively with combinations of various buffer sizes and
>  	  alignments.
>  
> +config ANDROID_BINDER_ALLOC_KUNIT_TEST
> +	tristate "KUnit Tests for Android Binder Alloc" if !KUNIT_ALL_TESTS
> +	depends on ANDROID_BINDER_IPC && KUNIT
> +	default KUNIT_ALL_TESTS
> +	help
> +	  This feature builds the binder alloc KUnit tests.
> +
> +	  Each test case runs using a pared-down binder_alloc struct and
> +	  test-specific freelist, which allows this KUnit module to be loaded
> +	  for testing without interfering with a running system.
> +
>  endmenu
> diff --git a/drivers/android/Makefile b/drivers/android/Makefile
> index c9d3d0c99c25..74d02a335d4e 100644
> --- a/drivers/android/Makefile
> +++ b/drivers/android/Makefile
> @@ -4,3 +4,4 @@ ccflags-y += -I$(src)			# needed for trace events
>  obj-$(CONFIG_ANDROID_BINDERFS)		+= binderfs.o
>  obj-$(CONFIG_ANDROID_BINDER_IPC)	+= binder.o binder_alloc.o
>  obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
> +obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)	+= tests/
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index c463ca4a8fff..9dfe90c284fc 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -68,6 +68,8 @@
>  #include <linux/sizes.h>
>  #include <linux/ktime.h>
>  
> +#include <kunit/visibility.h>
> +
>  #include <uapi/linux/android/binder.h>
>  
>  #include <linux/cacheflush.h>
> @@ -5956,10 +5958,11 @@ static void binder_vma_close(struct vm_area_struct *vma)
>  	binder_alloc_vma_close(&proc->alloc);
>  }
>  
> -static vm_fault_t binder_vm_fault(struct vm_fault *vmf)
> +VISIBLE_IF_KUNIT vm_fault_t binder_vm_fault(struct vm_fault *vmf)
>  {
>  	return VM_FAULT_SIGBUS;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_vm_fault);
>  
>  static const struct vm_operations_struct binder_vm_ops = {
>  	.open = binder_vma_open,
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index 2e89f9127883..c79e5c6721f0 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -23,6 +23,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/highmem.h>
>  #include <linux/sizes.h>
> +#include <kunit/visibility.h>
>  #include "binder_alloc.h"
>  #include "binder_trace.h"
>  
> @@ -57,13 +58,14 @@ static struct binder_buffer *binder_buffer_prev(struct binder_buffer *buffer)
>  	return list_entry(buffer->entry.prev, struct binder_buffer, entry);
>  }
>  
> -static size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
> -				       struct binder_buffer *buffer)
> +VISIBLE_IF_KUNIT size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
> +						 struct binder_buffer *buffer)
>  {
>  	if (list_is_last(&buffer->entry, &alloc->buffers))
>  		return alloc->vm_start + alloc->buffer_size - buffer->user_data;
>  	return binder_buffer_next(buffer)->user_data - buffer->user_data;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_buffer_size);
>  
>  static void binder_insert_free_buffer(struct binder_alloc *alloc,
>  				      struct binder_buffer *new_buffer)
> @@ -959,7 +961,7 @@ int binder_alloc_mmap_handler(struct binder_alloc *alloc,
>  			   failure_string, ret);
>  	return ret;
>  }
> -
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_mmap_handler);
>  
>  void binder_alloc_deferred_release(struct binder_alloc *alloc)
>  {
> @@ -1028,6 +1030,7 @@ void binder_alloc_deferred_release(struct binder_alloc *alloc)
>  		     "%s: %d buffers %d, pages %d\n",
>  		     __func__, alloc->pid, buffers, page_count);
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_deferred_release);
>  
>  /**
>   * binder_alloc_print_allocated() - print buffer info
> @@ -1122,6 +1125,7 @@ void binder_alloc_vma_close(struct binder_alloc *alloc)
>  {
>  	binder_alloc_set_mapped(alloc, false);
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_vma_close);
>  
>  /**
>   * binder_alloc_free_page() - shrinker callback to free pages
> @@ -1229,8 +1233,8 @@ binder_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
>  
>  static struct shrinker *binder_shrinker;
>  
> -static void __binder_alloc_init(struct binder_alloc *alloc,
> -				struct list_lru *freelist)
> +VISIBLE_IF_KUNIT void __binder_alloc_init(struct binder_alloc *alloc,
> +					  struct list_lru *freelist)
>  {
>  	alloc->pid = current->group_leader->pid;
>  	alloc->mm = current->mm;
> @@ -1239,6 +1243,7 @@ static void __binder_alloc_init(struct binder_alloc *alloc,
>  	INIT_LIST_HEAD(&alloc->buffers);
>  	alloc->freelist = freelist;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(__binder_alloc_init);
>  
>  /**
>   * binder_alloc_init() - called by binder_open() for per-proc initialization
> diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
> index aa05a9df1360..dc8dce2469a7 100644
> --- a/drivers/android/binder_alloc.h
> +++ b/drivers/android/binder_alloc.h
> @@ -188,5 +188,11 @@ int binder_alloc_copy_from_buffer(struct binder_alloc *alloc,
>  				  binder_size_t buffer_offset,
>  				  size_t bytes);
>  
> +#if IS_ENABLED(CONFIG_KUNIT)
> +void __binder_alloc_init(struct binder_alloc *alloc, struct list_lru *freelist);
> +size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
> +				struct binder_buffer *buffer);
> +#endif
> +
>  #endif /* _LINUX_BINDER_ALLOC_H */
>  
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> index 1ba5caf1d88d..b5d3014fb4dc 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -592,4 +592,8 @@ void binder_add_device(struct binder_device *device);
>   */
>  void binder_remove_device(struct binder_device *device);
>  
> +#if IS_ENABLED(CONFIG_KUNIT)
> +vm_fault_t binder_vm_fault(struct vm_fault *vmf);
> +#endif

I'm used to the "#ifdef CONFIG_..." idiom, but looking at the tree, I
see that "#if IS_ENANLED(CONFIG...)" is relatively common too. I don't
think there is a function difference, so I leave the style choice up to
you! ;)

> +
>  #endif /* _LINUX_BINDER_INTERNAL_H */
> diff --git a/drivers/android/tests/.kunitconfig b/drivers/android/tests/.kunitconfig
> new file mode 100644
> index 000000000000..a73601231049
> --- /dev/null
> +++ b/drivers/android/tests/.kunitconfig
> @@ -0,0 +1,3 @@
> +CONFIG_KUNIT=y
> +CONFIG_ANDROID_BINDER_IPC=y
> +CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST=y
> diff --git a/drivers/android/tests/Makefile b/drivers/android/tests/Makefile
> new file mode 100644
> index 000000000000..6780967e573b
> --- /dev/null
> +++ b/drivers/android/tests/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)	+= binder_alloc_kunit.o
> diff --git a/drivers/android/tests/binder_alloc_kunit.c b/drivers/android/tests/binder_alloc_kunit.c
> new file mode 100644
> index 000000000000..4b68b5687d33
> --- /dev/null
> +++ b/drivers/android/tests/binder_alloc_kunit.c
> @@ -0,0 +1,166 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Test cases for binder allocator code
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <kunit/test.h>
> +#include <linux/anon_inodes.h>
> +#include <linux/err.h>
> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/mm.h>
> +#include <linux/mman.h>
> +#include <linux/sizes.h>
> +
> +#include "../binder_alloc.h"
> +#include "../binder_internal.h"
> +
> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
> +
> +#define BINDER_MMAP_SIZE SZ_128K
> +
> +struct binder_alloc_test {
> +	struct binder_alloc alloc;
> +	struct list_lru binder_test_freelist;
> +	struct file *filp;
> +	unsigned long mmap_uaddr;
> +};
> +
> +static void binder_alloc_test_init_freelist(struct kunit *test)
> +{
> +	struct binder_alloc_test *priv = test->priv;
> +
> +	KUNIT_EXPECT_PTR_EQ(test, priv->alloc.freelist,
> +			    &priv->binder_test_freelist);
> +}
> +
> +static void binder_alloc_test_mmap(struct kunit *test)
> +{
> +	struct binder_alloc_test *priv = test->priv;
> +	struct binder_alloc *alloc = &priv->alloc;
> +	struct binder_buffer *buf;
> +	struct rb_node *n;
> +
> +	KUNIT_EXPECT_EQ(test, alloc->mapped, true);
> +	KUNIT_EXPECT_EQ(test, alloc->buffer_size, BINDER_MMAP_SIZE);
> +
> +	n = rb_first(&alloc->allocated_buffers);
> +	KUNIT_EXPECT_PTR_EQ(test, n, NULL);
> +
> +	n = rb_first(&alloc->free_buffers);
> +	buf = rb_entry(n, struct binder_buffer, rb_node);
> +	KUNIT_EXPECT_EQ(test, binder_alloc_buffer_size(alloc, buf),
> +			BINDER_MMAP_SIZE);
> +	KUNIT_EXPECT_TRUE(test, list_is_last(&buf->entry, &alloc->buffers));
> +}
> +
> +/* ===== End test cases ===== */
> +
> +static void binder_alloc_test_vma_close(struct vm_area_struct *vma)
> +{
> +	struct binder_alloc *alloc = vma->vm_private_data;
> +
> +	binder_alloc_vma_close(alloc);
> +}
> +
> +static const struct vm_operations_struct binder_alloc_test_vm_ops = {
> +	.close = binder_alloc_test_vma_close,
> +	.fault = binder_vm_fault,
> +};
> +
> +static int binder_alloc_test_mmap_handler(struct file *filp,
> +					  struct vm_area_struct *vma)
> +{
> +	struct binder_alloc *alloc = filp->private_data;
> +
> +	vm_flags_mod(vma, VM_DONTCOPY | VM_MIXEDMAP, VM_MAYWRITE);
> +
> +	vma->vm_ops = &binder_alloc_test_vm_ops;
> +	vma->vm_private_data = alloc;
> +
> +	return binder_alloc_mmap_handler(alloc, vma);
> +}
> +
> +static const struct file_operations binder_alloc_test_fops = {
> +	.mmap = binder_alloc_test_mmap_handler,
> +};
> +
> +static int binder_alloc_test_init(struct kunit *test)
> +{
> +	struct binder_alloc_test *priv;
> +	int ret;
> +
> +	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	test->priv = priv;
> +
> +	ret = list_lru_init(&priv->binder_test_freelist);
> +	if (ret) {
> +		kunit_err(test, "Failed to initialize test freelist\n");
> +		return ret;
> +	}
> +
> +	/* __binder_alloc_init requires mm to be attached */
> +	ret = kunit_attach_mm();
> +	if (ret) {
> +		kunit_err(test, "Failed to attach mm\n");
> +		return ret;
> +	}
> +	__binder_alloc_init(&priv->alloc, &priv->binder_test_freelist);
> +
> +	priv->filp = anon_inode_getfile("binder_alloc_kunit",
> +					&binder_alloc_test_fops, &priv->alloc,
> +					O_RDWR | O_CLOEXEC);
> +	if (IS_ERR_OR_NULL(priv->filp)) {
> +		kunit_err(test, "Failed to open binder alloc test driver file\n");
> +		return priv->filp ? PTR_ERR(priv->filp) : -ENOMEM;
> +	}
> +
> +	priv->mmap_uaddr = kunit_vm_mmap(test, priv->filp, 0, BINDER_MMAP_SIZE,
> +					 PROT_READ, MAP_PRIVATE | MAP_NORESERVE,
> +					 0);
> +	if (!priv->mmap_uaddr) {
> +		kunit_err(test, "Could not map the test's transaction memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
> +static void binder_alloc_test_exit(struct kunit *test)
> +{
> +	struct binder_alloc_test *priv = test->priv;
> +
> +	/* Close the backing file to make sure binder_alloc_vma_close runs */
> +	if (!IS_ERR_OR_NULL(priv->filp))
> +		fput(priv->filp);
> +
> +	if (priv->alloc.mm)
> +		binder_alloc_deferred_release(&priv->alloc);
> +
> +	/* Make sure freelist is empty */
> +	KUNIT_EXPECT_EQ(test, list_lru_count(&priv->binder_test_freelist), 0);
> +	list_lru_destroy(&priv->binder_test_freelist);
> +}
> +
> +static struct kunit_case binder_alloc_test_cases[] = {
> +	KUNIT_CASE(binder_alloc_test_init_freelist),
> +	KUNIT_CASE(binder_alloc_test_mmap),
> +	{}
> +};
> +
> +static struct kunit_suite binder_alloc_test_suite = {
> +	.name = "binder_alloc",
> +	.test_cases = binder_alloc_test_cases,
> +	.init = binder_alloc_test_init,
> +	.exit = binder_alloc_test_exit,
> +};
> +
> +kunit_test_suite(binder_alloc_test_suite);
> +
> +MODULE_AUTHOR("Tiffany Yang <ynaffit@google.com>");
> +MODULE_DESCRIPTION("Binder Alloc KUnit tests");
> +MODULE_LICENSE("GPL");

Reviewed-by: Kees Cook <kees@kernel.org>


-- 
Kees Cook

Re: [PATCH v3 4/6] binder: Scaffolding for binder_alloc KUnit tests

[Tiffany Yang]

Kees Cook <kees@kernel.org> writes:

>> ...

> I'm used to the "#ifdef CONFIG_..." idiom, but looking at the tree, I
> see that "#if IS_ENANLED(CONFIG...)" is relatively common too. I don't
> think there is a function difference, so I leave the style choice up to
> you! ;)


IIRC, I had tried using the #ifdef to inject test-specific code in some
places, but that created issues when we were loading KUnit as a module
because it causes the built-in code to be built as though the
"CONFIG_..." is undefined. Consequently, I started using IS_ENABLED by
default, but I'm not sure if it's strictly necessary for exposing
functions with a header file since I'd assume "CONFIG_..." will be
defined when we're building the module?

>> +
>>   #endif /* _LINUX_BINDER_INTERNAL_H */
>> diff --git a/drivers/android/tests/.kunitconfig  
>> b/drivers/android/tests/.kunitconfig
>> new file mode 100644
>> index 000000000000..a73601231049
>> --- /dev/null
>> +++ b/drivers/android/tests/.kunitconfig
>> @@ -0,0 +1,3 @@
>> +CONFIG_KUNIT=y
>> +CONFIG_ANDROID_BINDER_IPC=y
>> +CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST=y
>> diff --git a/drivers/android/tests/Makefile  
>> b/drivers/android/tests/Makefile
>> new file mode 100644
>> index 000000000000..6780967e573b
>> --- /dev/null
>> +++ b/drivers/android/tests/Makefile
>> @@ -0,0 +1,3 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +
>> +obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)	+= binder_alloc_kunit.o
>> diff --git a/drivers/android/tests/binder_alloc_kunit.c  
>> b/drivers/android/tests/binder_alloc_kunit.c
>> new file mode 100644
>> index 000000000000..4b68b5687d33
>> --- /dev/null
>> +++ b/drivers/android/tests/binder_alloc_kunit.c
>> @@ -0,0 +1,166 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Test cases for binder allocator code
>> + */
>> +
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> +
>> +#include <kunit/test.h>
>> +#include <linux/anon_inodes.h>
>> +#include <linux/err.h>
>> +#include <linux/file.h>
>> +#include <linux/fs.h>
>> +#include <linux/mm.h>
>> +#include <linux/mman.h>
>> +#include <linux/sizes.h>
>> +
>> +#include "../binder_alloc.h"
>> +#include "../binder_internal.h"
>> +
>> +MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
>> +
>> +#define BINDER_MMAP_SIZE SZ_128K
>> +
>> +struct binder_alloc_test {
>> +	struct binder_alloc alloc;
>> +	struct list_lru binder_test_freelist;
>> +	struct file *filp;
>> +	unsigned long mmap_uaddr;
>> +};
>> +
>> +static void binder_alloc_test_init_freelist(struct kunit *test)
>> +{
>> +	struct binder_alloc_test *priv = test->priv;
>> +
>> +	KUNIT_EXPECT_PTR_EQ(test, priv->alloc.freelist,
>> +			    &priv->binder_test_freelist);
>> +}
>> +
>> +static void binder_alloc_test_mmap(struct kunit *test)
>> +{
>> +	struct binder_alloc_test *priv = test->priv;
>> +	struct binder_alloc *alloc = &priv->alloc;
>> +	struct binder_buffer *buf;
>> +	struct rb_node *n;
>> +
>> +	KUNIT_EXPECT_EQ(test, alloc->mapped, true);
>> +	KUNIT_EXPECT_EQ(test, alloc->buffer_size, BINDER_MMAP_SIZE);
>> +
>> +	n = rb_first(&alloc->allocated_buffers);
>> +	KUNIT_EXPECT_PTR_EQ(test, n, NULL);
>> +
>> +	n = rb_first(&alloc->free_buffers);
>> +	buf = rb_entry(n, struct binder_buffer, rb_node);
>> +	KUNIT_EXPECT_EQ(test, binder_alloc_buffer_size(alloc, buf),
>> +			BINDER_MMAP_SIZE);
>> +	KUNIT_EXPECT_TRUE(test, list_is_last(&buf->entry, &alloc->buffers));
>> +}
>> +
>> +/* ===== End test cases ===== */
>> +
>> +static void binder_alloc_test_vma_close(struct vm_area_struct *vma)
>> +{
>> +	struct binder_alloc *alloc = vma->vm_private_data;
>> +
>> +	binder_alloc_vma_close(alloc);
>> +}
>> +
>> +static const struct vm_operations_struct binder_alloc_test_vm_ops = {
>> +	.close = binder_alloc_test_vma_close,
>> +	.fault = binder_vm_fault,
>> +};
>> +
>> +static int binder_alloc_test_mmap_handler(struct file *filp,
>> +					  struct vm_area_struct *vma)
>> +{
>> +	struct binder_alloc *alloc = filp->private_data;
>> +
>> +	vm_flags_mod(vma, VM_DONTCOPY | VM_MIXEDMAP, VM_MAYWRITE);
>> +
>> +	vma->vm_ops = &binder_alloc_test_vm_ops;
>> +	vma->vm_private_data = alloc;
>> +
>> +	return binder_alloc_mmap_handler(alloc, vma);
>> +}
>> +
>> +static const struct file_operations binder_alloc_test_fops = {
>> +	.mmap = binder_alloc_test_mmap_handler,
>> +};
>> +
>> +static int binder_alloc_test_init(struct kunit *test)
>> +{
>> +	struct binder_alloc_test *priv;
>> +	int ret;
>> +
>> +	priv = kunit_kzalloc(test, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +	test->priv = priv;
>> +
>> +	ret = list_lru_init(&priv->binder_test_freelist);
>> +	if (ret) {
>> +		kunit_err(test, "Failed to initialize test freelist\n");
>> +		return ret;
>> +	}
>> +
>> +	/* __binder_alloc_init requires mm to be attached */
>> +	ret = kunit_attach_mm();
>> +	if (ret) {
>> +		kunit_err(test, "Failed to attach mm\n");
>> +		return ret;
>> +	}
>> +	__binder_alloc_init(&priv->alloc, &priv->binder_test_freelist);
>> +
>> +	priv->filp = anon_inode_getfile("binder_alloc_kunit",
>> +					&binder_alloc_test_fops, &priv->alloc,
>> +					O_RDWR | O_CLOEXEC);
>> +	if (IS_ERR_OR_NULL(priv->filp)) {
>> +		kunit_err(test, "Failed to open binder alloc test driver file\n");
>> +		return priv->filp ? PTR_ERR(priv->filp) : -ENOMEM;
>> +	}
>> +
>> +	priv->mmap_uaddr = kunit_vm_mmap(test, priv->filp, 0, BINDER_MMAP_SIZE,
>> +					 PROT_READ, MAP_PRIVATE | MAP_NORESERVE,
>> +					 0);
>> +	if (!priv->mmap_uaddr) {
>> +		kunit_err(test, "Could not map the test's transaction memory\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static void binder_alloc_test_exit(struct kunit *test)
>> +{
>> +	struct binder_alloc_test *priv = test->priv;
>> +
>> +	/* Close the backing file to make sure binder_alloc_vma_close runs */
>> +	if (!IS_ERR_OR_NULL(priv->filp))
>> +		fput(priv->filp);
>> +
>> +	if (priv->alloc.mm)
>> +		binder_alloc_deferred_release(&priv->alloc);
>> +
>> +	/* Make sure freelist is empty */
>> +	KUNIT_EXPECT_EQ(test, list_lru_count(&priv->binder_test_freelist), 0);
>> +	list_lru_destroy(&priv->binder_test_freelist);
>> +}
>> +
>> +static struct kunit_case binder_alloc_test_cases[] = {
>> +	KUNIT_CASE(binder_alloc_test_init_freelist),
>> +	KUNIT_CASE(binder_alloc_test_mmap),
>> +	{}
>> +};
>> +
>> +static struct kunit_suite binder_alloc_test_suite = {
>> +	.name = "binder_alloc",
>> +	.test_cases = binder_alloc_test_cases,
>> +	.init = binder_alloc_test_init,
>> +	.exit = binder_alloc_test_exit,
>> +};
>> +
>> +kunit_test_suite(binder_alloc_test_suite);
>> +
>> +MODULE_AUTHOR("Tiffany Yang <ynaffit@google.com>");
>> +MODULE_DESCRIPTION("Binder Alloc KUnit tests");
>> +MODULE_LICENSE("GPL");

> Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Tiffany Y. Yang

Re: [PATCH v3 4/6] binder: Scaffolding for binder_alloc KUnit tests

[Carlos Llamas]

On Wed, Jul 16, 2025 at 03:28:49PM -0700, Tiffany Yang wrote:
> Kees Cook <kees@kernel.org> writes:
> 
> > > ...
> 
> > I'm used to the "#ifdef CONFIG_..." idiom, but looking at the tree, I
> > see that "#if IS_ENANLED(CONFIG...)" is relatively common too. I don't
> > think there is a function difference, so I leave the style choice up to
> > you! ;)
> 
> 
> IIRC, I had tried using the #ifdef to inject test-specific code in some
> places, but that created issues when we were loading KUnit as a module
> because it causes the built-in code to be built as though the
> "CONFIG_..." is undefined. Consequently, I started using IS_ENABLED by
> default, but I'm not sure if it's strictly necessary for exposing
> functions with a header file since I'd assume "CONFIG_..." will be
> defined when we're building the module?

Right, IS_ENABLED() is the "short-hand" for ...
	#if defined(CONFIG_x) || defined(CONFIG_x_MODULE)
... which is what we need here.

--
Carlos Llamas

Re: [PATCH v3 4/6] binder: Scaffolding for binder_alloc KUnit tests

[Kees Cook]

On Wed, Jul 16, 2025 at 10:42:58PM +0000, Carlos Llamas wrote:
> On Wed, Jul 16, 2025 at 03:28:49PM -0700, Tiffany Yang wrote:
> > Kees Cook <kees@kernel.org> writes:
> > 
> > > > ...
> > 
> > > I'm used to the "#ifdef CONFIG_..." idiom, but looking at the tree, I
> > > see that "#if IS_ENANLED(CONFIG...)" is relatively common too. I don't
> > > think there is a function difference, so I leave the style choice up to
> > > you! ;)
> > 
> > 
> > IIRC, I had tried using the #ifdef to inject test-specific code in some
> > places, but that created issues when we were loading KUnit as a module
> > because it causes the built-in code to be built as though the
> > "CONFIG_..." is undefined. Consequently, I started using IS_ENABLED by
> > default, but I'm not sure if it's strictly necessary for exposing
> > functions with a header file since I'd assume "CONFIG_..." will be
> > defined when we're building the module?
> 
> Right, IS_ENABLED() is the "short-hand" for ...
> 	#if defined(CONFIG_x) || defined(CONFIG_x_MODULE)
> ... which is what we need here.

Ah-ha! Okay, thanks for the explanation. I don't do much module work,
it seems. ;)

-- 
Kees Cook

[PATCH v3 5/6] binder: Convert binder_alloc selftests to KUnit

[Tiffany Yang]

Convert the existing binder_alloc_selftest tests into KUnit tests. These
tests allocate and free an exhaustive combination of buffers with
various sizes and alignments. This change allows them to be run without
blocking or otherwise interfering with other processes in binder.

This test is refactored into more meaningful cases in the subsequent
patch.

Signed-off-by: Tiffany Yang <ynaffit@google.com>
---
v2:
* Fix build warning Reported-by: kernel test robot <lkp@intel.com>
  Closes: https://lore.kernel.org/oe-kbuild-all/202506281837.hReNHJjO-lkp@intel.com/
---
 drivers/android/Kconfig                    |  10 -
 drivers/android/Makefile                   |   1 -
 drivers/android/binder.c                   |   5 -
 drivers/android/binder_alloc.c             |   3 +
 drivers/android/binder_alloc.h             |   5 -
 drivers/android/binder_alloc_selftest.c    | 345 ---------------------
 drivers/android/tests/binder_alloc_kunit.c | 279 +++++++++++++++++
 7 files changed, 282 insertions(+), 366 deletions(-)
 delete mode 100644 drivers/android/binder_alloc_selftest.c

diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
index b1bc7183366c..5b3b8041f827 100644
--- a/drivers/android/Kconfig
+++ b/drivers/android/Kconfig
@@ -37,16 +37,6 @@ config ANDROID_BINDER_DEVICES
 	  created. Each binder device has its own context manager, and is
 	  therefore logically separated from the other devices.
 
-config ANDROID_BINDER_IPC_SELFTEST
-	bool "Android Binder IPC Driver Selftest"
-	depends on ANDROID_BINDER_IPC
-	help
-	  This feature allows binder selftest to run.
-
-	  Binder selftest checks the allocation and free of binder buffers
-	  exhaustively with combinations of various buffer sizes and
-	  alignments.
-
 config ANDROID_BINDER_ALLOC_KUNIT_TEST
 	tristate "KUnit Tests for Android Binder Alloc" if !KUNIT_ALL_TESTS
 	depends on ANDROID_BINDER_IPC && KUNIT
diff --git a/drivers/android/Makefile b/drivers/android/Makefile
index 74d02a335d4e..c5d47be0276c 100644
--- a/drivers/android/Makefile
+++ b/drivers/android/Makefile
@@ -3,5 +3,4 @@ ccflags-y += -I$(src)			# needed for trace events
 
 obj-$(CONFIG_ANDROID_BINDERFS)		+= binderfs.o
 obj-$(CONFIG_ANDROID_BINDER_IPC)	+= binder.o binder_alloc.o
-obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
 obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)	+= tests/
diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9dfe90c284fc..7b2653a5d59c 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5718,11 +5718,6 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	struct binder_thread *thread;
 	void __user *ubuf = (void __user *)arg;
 
-	/*pr_info("binder_ioctl: %d:%d %x %lx\n",
-			proc->pid, current->pid, cmd, arg);*/
-
-	binder_selftest_alloc(&proc->alloc);
-
 	trace_binder_ioctl(cmd, arg);
 
 	ret = wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index c79e5c6721f0..74a184014fa7 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -701,6 +701,7 @@ struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
 out:
 	return buffer;
 }
+EXPORT_SYMBOL_IF_KUNIT(binder_alloc_new_buf);
 
 static unsigned long buffer_start_page(struct binder_buffer *buffer)
 {
@@ -879,6 +880,7 @@ void binder_alloc_free_buf(struct binder_alloc *alloc,
 	binder_free_buf_locked(alloc, buffer);
 	mutex_unlock(&alloc->mutex);
 }
+EXPORT_SYMBOL_IF_KUNIT(binder_alloc_free_buf);
 
 /**
  * binder_alloc_mmap_handler() - map virtual address space for proc
@@ -1217,6 +1219,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
 err_mmget:
 	return LRU_SKIP;
 }
+EXPORT_SYMBOL_IF_KUNIT(binder_alloc_free_page);
 
 static unsigned long
 binder_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index dc8dce2469a7..bed97c2cad92 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -121,11 +121,6 @@ struct binder_alloc {
 	bool oneway_spam_detected;
 };
 
-#ifdef CONFIG_ANDROID_BINDER_IPC_SELFTEST
-void binder_selftest_alloc(struct binder_alloc *alloc);
-#else
-static inline void binder_selftest_alloc(struct binder_alloc *alloc) {}
-#endif
 enum lru_status binder_alloc_free_page(struct list_head *item,
 				       struct list_lru_one *lru,
 				       void *cb_arg);
diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/binder_alloc_selftest.c
deleted file mode 100644
index 8b18b22aa3de..000000000000
--- a/drivers/android/binder_alloc_selftest.c
+++ /dev/null
@@ -1,345 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/* binder_alloc_selftest.c
- *
- * Android IPC Subsystem
- *
- * Copyright (C) 2017 Google, Inc.
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/err.h>
-#include <linux/list_lru.h>
-#include <linux/mm_types.h>
-#include "binder_alloc.h"
-
-#define BUFFER_NUM 5
-#define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
-
-static bool binder_selftest_run = true;
-static int binder_selftest_failures;
-static DEFINE_MUTEX(binder_selftest_lock);
-static struct list_lru binder_selftest_freelist;
-
-/**
- * enum buf_end_align_type - Page alignment of a buffer
- * end with regard to the end of the previous buffer.
- *
- * In the pictures below, buf2 refers to the buffer we
- * are aligning. buf1 refers to previous buffer by addr.
- * Symbol [ means the start of a buffer, ] means the end
- * of a buffer, and | means page boundaries.
- */
-enum buf_end_align_type {
-	/**
-	 * @SAME_PAGE_UNALIGNED: The end of this buffer is on
-	 * the same page as the end of the previous buffer and
-	 * is not page aligned. Examples:
-	 * buf1 ][ buf2 ][ ...
-	 * buf1 ]|[ buf2 ][ ...
-	 */
-	SAME_PAGE_UNALIGNED = 0,
-	/**
-	 * @SAME_PAGE_ALIGNED: When the end of the previous buffer
-	 * is not page aligned, the end of this buffer is on the
-	 * same page as the end of the previous buffer and is page
-	 * aligned. When the previous buffer is page aligned, the
-	 * end of this buffer is aligned to the next page boundary.
-	 * Examples:
-	 * buf1 ][ buf2 ]| ...
-	 * buf1 ]|[ buf2 ]| ...
-	 */
-	SAME_PAGE_ALIGNED,
-	/**
-	 * @NEXT_PAGE_UNALIGNED: The end of this buffer is on
-	 * the page next to the end of the previous buffer and
-	 * is not page aligned. Examples:
-	 * buf1 ][ buf2 | buf2 ][ ...
-	 * buf1 ]|[ buf2 | buf2 ][ ...
-	 */
-	NEXT_PAGE_UNALIGNED,
-	/**
-	 * @NEXT_PAGE_ALIGNED: The end of this buffer is on
-	 * the page next to the end of the previous buffer and
-	 * is page aligned. Examples:
-	 * buf1 ][ buf2 | buf2 ]| ...
-	 * buf1 ]|[ buf2 | buf2 ]| ...
-	 */
-	NEXT_PAGE_ALIGNED,
-	/**
-	 * @NEXT_NEXT_UNALIGNED: The end of this buffer is on
-	 * the page that follows the page after the end of the
-	 * previous buffer and is not page aligned. Examples:
-	 * buf1 ][ buf2 | buf2 | buf2 ][ ...
-	 * buf1 ]|[ buf2 | buf2 | buf2 ][ ...
-	 */
-	NEXT_NEXT_UNALIGNED,
-	/**
-	 * @LOOP_END: The number of enum values in &buf_end_align_type.
-	 * It is used for controlling loop termination.
-	 */
-	LOOP_END,
-};
-
-static void pr_err_size_seq(size_t *sizes, int *seq)
-{
-	int i;
-
-	pr_err("alloc sizes: ");
-	for (i = 0; i < BUFFER_NUM; i++)
-		pr_cont("[%zu]", sizes[i]);
-	pr_cont("\n");
-	pr_err("free seq: ");
-	for (i = 0; i < BUFFER_NUM; i++)
-		pr_cont("[%d]", seq[i]);
-	pr_cont("\n");
-}
-
-static bool check_buffer_pages_allocated(struct binder_alloc *alloc,
-					 struct binder_buffer *buffer,
-					 size_t size)
-{
-	unsigned long page_addr;
-	unsigned long end;
-	int page_index;
-
-	end = PAGE_ALIGN(buffer->user_data + size);
-	page_addr = buffer->user_data;
-	for (; page_addr < end; page_addr += PAGE_SIZE) {
-		page_index = (page_addr - alloc->vm_start) / PAGE_SIZE;
-		if (!alloc->pages[page_index] ||
-		    !list_empty(page_to_lru(alloc->pages[page_index]))) {
-			pr_err("expect alloc but is %s at page index %d\n",
-			       alloc->pages[page_index] ?
-			       "lru" : "free", page_index);
-			return false;
-		}
-	}
-	return true;
-}
-
-static void binder_selftest_alloc_buf(struct binder_alloc *alloc,
-				      struct binder_buffer *buffers[],
-				      size_t *sizes, int *seq)
-{
-	int i;
-
-	for (i = 0; i < BUFFER_NUM; i++) {
-		buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
-		if (IS_ERR(buffers[i]) ||
-		    !check_buffer_pages_allocated(alloc, buffers[i],
-						  sizes[i])) {
-			pr_err_size_seq(sizes, seq);
-			binder_selftest_failures++;
-		}
-	}
-}
-
-static void binder_selftest_free_buf(struct binder_alloc *alloc,
-				     struct binder_buffer *buffers[],
-				     size_t *sizes, int *seq, size_t end)
-{
-	int i;
-
-	for (i = 0; i < BUFFER_NUM; i++)
-		binder_alloc_free_buf(alloc, buffers[seq[i]]);
-
-	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
-		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",
-			       alloc->pages[i] ? "alloc" : "free", i);
-			binder_selftest_failures++;
-		}
-	}
-}
-
-static void binder_selftest_free_page(struct binder_alloc *alloc)
-{
-	int i;
-	unsigned long count;
-
-	while ((count = list_lru_count(&binder_selftest_freelist))) {
-		list_lru_walk(&binder_selftest_freelist, binder_alloc_free_page,
-			      NULL, count);
-	}
-
-	for (i = 0; i < (alloc->buffer_size / PAGE_SIZE); i++) {
-		if (alloc->pages[i]) {
-			pr_err("expect free but is %s at page index %d\n",
-			       list_empty(page_to_lru(alloc->pages[i])) ?
-			       "alloc" : "lru", i);
-			binder_selftest_failures++;
-		}
-	}
-}
-
-static void binder_selftest_alloc_free(struct binder_alloc *alloc,
-				       size_t *sizes, int *seq, size_t end)
-{
-	struct binder_buffer *buffers[BUFFER_NUM];
-
-	binder_selftest_alloc_buf(alloc, buffers, sizes, seq);
-	binder_selftest_free_buf(alloc, buffers, sizes, seq, end);
-
-	/* Allocate from lru. */
-	binder_selftest_alloc_buf(alloc, buffers, sizes, seq);
-	if (list_lru_count(&binder_selftest_freelist))
-		pr_err("lru list should be empty but is not\n");
-
-	binder_selftest_free_buf(alloc, buffers, sizes, seq, end);
-	binder_selftest_free_page(alloc);
-}
-
-static bool is_dup(int *seq, int index, int val)
-{
-	int i;
-
-	for (i = 0; i < index; i++) {
-		if (seq[i] == val)
-			return true;
-	}
-	return false;
-}
-
-/* Generate BUFFER_NUM factorial free orders. */
-static void binder_selftest_free_seq(struct binder_alloc *alloc,
-				     size_t *sizes, int *seq,
-				     int index, size_t end)
-{
-	int i;
-
-	if (index == BUFFER_NUM) {
-		binder_selftest_alloc_free(alloc, sizes, seq, end);
-		return;
-	}
-	for (i = 0; i < BUFFER_NUM; i++) {
-		if (is_dup(seq, index, i))
-			continue;
-		seq[index] = i;
-		binder_selftest_free_seq(alloc, sizes, seq, index + 1, end);
-	}
-}
-
-static void binder_selftest_alloc_size(struct binder_alloc *alloc,
-				       size_t *end_offset)
-{
-	int i;
-	int seq[BUFFER_NUM] = {0};
-	size_t front_sizes[BUFFER_NUM];
-	size_t back_sizes[BUFFER_NUM];
-	size_t last_offset, offset = 0;
-
-	for (i = 0; i < BUFFER_NUM; i++) {
-		last_offset = offset;
-		offset = end_offset[i];
-		front_sizes[i] = offset - last_offset;
-		back_sizes[BUFFER_NUM - i - 1] = front_sizes[i];
-	}
-	/*
-	 * Buffers share the first or last few pages.
-	 * Only BUFFER_NUM - 1 buffer sizes are adjustable since
-	 * we need one giant buffer before getting to the last page.
-	 */
-	back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
-	binder_selftest_free_seq(alloc, front_sizes, seq, 0,
-				 end_offset[BUFFER_NUM - 1]);
-	binder_selftest_free_seq(alloc, back_sizes, seq, 0, alloc->buffer_size);
-}
-
-static void binder_selftest_alloc_offset(struct binder_alloc *alloc,
-					 size_t *end_offset, int index)
-{
-	int align;
-	size_t end, prev;
-
-	if (index == BUFFER_NUM) {
-		binder_selftest_alloc_size(alloc, end_offset);
-		return;
-	}
-	prev = index == 0 ? 0 : end_offset[index - 1];
-	end = prev;
-
-	BUILD_BUG_ON(BUFFER_MIN_SIZE * BUFFER_NUM >= PAGE_SIZE);
-
-	for (align = SAME_PAGE_UNALIGNED; align < LOOP_END; align++) {
-		if (align % 2)
-			end = ALIGN(end, PAGE_SIZE);
-		else
-			end += BUFFER_MIN_SIZE;
-		end_offset[index] = end;
-		binder_selftest_alloc_offset(alloc, end_offset, index + 1);
-	}
-}
-
-int binder_selftest_alloc_get_page_count(struct binder_alloc *alloc)
-{
-	struct page *page;
-	int allocated = 0;
-	int i;
-
-	for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) {
-		page = alloc->pages[i];
-		if (page)
-			allocated++;
-	}
-	return allocated;
-}
-
-/**
- * binder_selftest_alloc() - Test alloc and free of buffer pages.
- * @alloc: Pointer to alloc struct.
- *
- * Allocate BUFFER_NUM buffers to cover all page alignment cases,
- * then free them in all orders possible. Check that pages are
- * correctly allocated, put onto lru when buffers are freed, and
- * are freed when binder_alloc_free_page is called.
- */
-void binder_selftest_alloc(struct binder_alloc *alloc)
-{
-	struct list_lru *prev_freelist;
-	size_t end_offset[BUFFER_NUM];
-
-	if (!binder_selftest_run)
-		return;
-	mutex_lock(&binder_selftest_lock);
-	if (!binder_selftest_run || !alloc->mapped)
-		goto done;
-
-	prev_freelist = 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 = &binder_selftest_freelist;
-
-	pr_info("STARTED\n");
-	binder_selftest_alloc_offset(alloc, end_offset, 0);
-	if (binder_selftest_failures > 0)
-		pr_info("%d tests FAILED\n", binder_selftest_failures);
-	else
-		pr_info("PASSED\n");
-
-	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 = false;
-	alloc->freelist = prev_freelist;
-	list_lru_destroy(&binder_selftest_freelist);
-done:
-	mutex_unlock(&binder_selftest_lock);
-}
diff --git a/drivers/android/tests/binder_alloc_kunit.c b/drivers/android/tests/binder_alloc_kunit.c
index 4b68b5687d33..9e185e2036e5 100644
--- a/drivers/android/tests/binder_alloc_kunit.c
+++ b/drivers/android/tests/binder_alloc_kunit.c
@@ -21,6 +21,265 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
 
 #define BINDER_MMAP_SIZE SZ_128K
 
+#define BUFFER_NUM 5
+#define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
+
+static int binder_alloc_test_failures;
+
+/**
+ * enum buf_end_align_type - Page alignment of a buffer
+ * end with regard to the end of the previous buffer.
+ *
+ * In the pictures below, buf2 refers to the buffer we
+ * are aligning. buf1 refers to previous buffer by addr.
+ * Symbol [ means the start of a buffer, ] means the end
+ * of a buffer, and | means page boundaries.
+ */
+enum buf_end_align_type {
+	/**
+	 * @SAME_PAGE_UNALIGNED: The end of this buffer is on
+	 * the same page as the end of the previous buffer and
+	 * is not page aligned. Examples:
+	 * buf1 ][ buf2 ][ ...
+	 * buf1 ]|[ buf2 ][ ...
+	 */
+	SAME_PAGE_UNALIGNED = 0,
+	/**
+	 * @SAME_PAGE_ALIGNED: When the end of the previous buffer
+	 * is not page aligned, the end of this buffer is on the
+	 * same page as the end of the previous buffer and is page
+	 * aligned. When the previous buffer is page aligned, the
+	 * end of this buffer is aligned to the next page boundary.
+	 * Examples:
+	 * buf1 ][ buf2 ]| ...
+	 * buf1 ]|[ buf2 ]| ...
+	 */
+	SAME_PAGE_ALIGNED,
+	/**
+	 * @NEXT_PAGE_UNALIGNED: The end of this buffer is on
+	 * the page next to the end of the previous buffer and
+	 * is not page aligned. Examples:
+	 * buf1 ][ buf2 | buf2 ][ ...
+	 * buf1 ]|[ buf2 | buf2 ][ ...
+	 */
+	NEXT_PAGE_UNALIGNED,
+	/**
+	 * @NEXT_PAGE_ALIGNED: The end of this buffer is on
+	 * the page next to the end of the previous buffer and
+	 * is page aligned. Examples:
+	 * buf1 ][ buf2 | buf2 ]| ...
+	 * buf1 ]|[ buf2 | buf2 ]| ...
+	 */
+	NEXT_PAGE_ALIGNED,
+	/**
+	 * @NEXT_NEXT_UNALIGNED: The end of this buffer is on
+	 * the page that follows the page after the end of the
+	 * previous buffer and is not page aligned. Examples:
+	 * buf1 ][ buf2 | buf2 | buf2 ][ ...
+	 * buf1 ]|[ buf2 | buf2 | buf2 ][ ...
+	 */
+	NEXT_NEXT_UNALIGNED,
+	/**
+	 * @LOOP_END: The number of enum values in &buf_end_align_type.
+	 * It is used for controlling loop termination.
+	 */
+	LOOP_END,
+};
+
+static void pr_err_size_seq(struct kunit *test, size_t *sizes, int *seq)
+{
+	int i;
+
+	kunit_err(test, "alloc sizes: ");
+	for (i = 0; i < BUFFER_NUM; i++)
+		pr_cont("[%zu]", sizes[i]);
+	pr_cont("\n");
+	kunit_err(test, "free seq: ");
+	for (i = 0; i < BUFFER_NUM; i++)
+		pr_cont("[%d]", seq[i]);
+	pr_cont("\n");
+}
+
+static bool check_buffer_pages_allocated(struct kunit *test,
+					 struct binder_alloc *alloc,
+					 struct binder_buffer *buffer,
+					 size_t size)
+{
+	unsigned long page_addr;
+	unsigned long end;
+	int page_index;
+
+	end = PAGE_ALIGN(buffer->user_data + size);
+	page_addr = buffer->user_data;
+	for (; page_addr < end; page_addr += PAGE_SIZE) {
+		page_index = (page_addr - alloc->vm_start) / PAGE_SIZE;
+		if (!alloc->pages[page_index] ||
+		    !list_empty(page_to_lru(alloc->pages[page_index]))) {
+			kunit_err(test, "expect alloc but is %s at page index %d\n",
+				  alloc->pages[page_index] ?
+				  "lru" : "free", page_index);
+			return false;
+		}
+	}
+	return true;
+}
+
+static void binder_alloc_test_alloc_buf(struct kunit *test,
+					struct binder_alloc *alloc,
+					struct binder_buffer *buffers[],
+					size_t *sizes, int *seq)
+{
+	int i;
+
+	for (i = 0; i < BUFFER_NUM; i++) {
+		buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
+		if (IS_ERR(buffers[i]) ||
+		    !check_buffer_pages_allocated(test, alloc, buffers[i], sizes[i])) {
+			pr_err_size_seq(test, sizes, seq);
+			binder_alloc_test_failures++;
+		}
+	}
+}
+
+static void binder_alloc_test_free_buf(struct kunit *test,
+				       struct binder_alloc *alloc,
+				       struct binder_buffer *buffers[],
+				       size_t *sizes, int *seq, size_t end)
+{
+	int i;
+
+	for (i = 0; i < BUFFER_NUM; i++)
+		binder_alloc_free_buf(alloc, buffers[seq[i]]);
+
+	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
+		if (list_empty(page_to_lru(alloc->pages[i]))) {
+			pr_err_size_seq(test, sizes, seq);
+			kunit_err(test, "expect lru but is %s at page index %d\n",
+				  alloc->pages[i] ? "alloc" : "free", i);
+			binder_alloc_test_failures++;
+		}
+	}
+}
+
+static void binder_alloc_test_free_page(struct kunit *test,
+					struct binder_alloc *alloc)
+{
+	unsigned long count;
+	int i;
+
+	while ((count = list_lru_count(alloc->freelist))) {
+		list_lru_walk(alloc->freelist, binder_alloc_free_page,
+			      NULL, count);
+	}
+
+	for (i = 0; i < (alloc->buffer_size / PAGE_SIZE); i++) {
+		if (alloc->pages[i]) {
+			kunit_err(test, "expect free but is %s at page index %d\n",
+				  list_empty(page_to_lru(alloc->pages[i])) ?
+				  "alloc" : "lru", i);
+			binder_alloc_test_failures++;
+		}
+	}
+}
+
+static void binder_alloc_test_alloc_free(struct kunit *test,
+					 struct binder_alloc *alloc,
+					 size_t *sizes, int *seq, size_t end)
+{
+	struct binder_buffer *buffers[BUFFER_NUM];
+
+	binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
+	binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
+
+	/* Allocate from lru. */
+	binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
+	if (list_lru_count(alloc->freelist))
+		kunit_err(test, "lru list should be empty but is not\n");
+
+	binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
+	binder_alloc_test_free_page(test, alloc);
+}
+
+static bool is_dup(int *seq, int index, int val)
+{
+	int i;
+
+	for (i = 0; i < index; i++) {
+		if (seq[i] == val)
+			return true;
+	}
+	return false;
+}
+
+/* Generate BUFFER_NUM factorial free orders. */
+static void permute_frees(struct kunit *test, struct binder_alloc *alloc,
+			  size_t *sizes, int *seq, int index, size_t end)
+{
+	int i;
+
+	if (index == BUFFER_NUM) {
+		binder_alloc_test_alloc_free(test, alloc, sizes, seq, end);
+		return;
+	}
+	for (i = 0; i < BUFFER_NUM; i++) {
+		if (is_dup(seq, index, i))
+			continue;
+		seq[index] = i;
+		permute_frees(test, alloc, sizes, seq, index + 1, end);
+	}
+}
+
+static void gen_buf_sizes(struct kunit *test, struct binder_alloc *alloc,
+			  size_t *end_offset)
+{
+	size_t last_offset, offset = 0;
+	size_t front_sizes[BUFFER_NUM];
+	size_t back_sizes[BUFFER_NUM];
+	int seq[BUFFER_NUM] = {0};
+	int i;
+
+	for (i = 0; i < BUFFER_NUM; i++) {
+		last_offset = offset;
+		offset = end_offset[i];
+		front_sizes[i] = offset - last_offset;
+		back_sizes[BUFFER_NUM - i - 1] = front_sizes[i];
+	}
+	/*
+	 * Buffers share the first or last few pages.
+	 * Only BUFFER_NUM - 1 buffer sizes are adjustable since
+	 * we need one giant buffer before getting to the last page.
+	 */
+	back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
+	permute_frees(test, alloc, front_sizes, seq, 0,
+		      end_offset[BUFFER_NUM - 1]);
+	permute_frees(test, alloc, back_sizes, seq, 0, alloc->buffer_size);
+}
+
+static void gen_buf_offsets(struct kunit *test, struct binder_alloc *alloc,
+			    size_t *end_offset, int index)
+{
+	size_t end, prev;
+	int align;
+
+	if (index == BUFFER_NUM) {
+		gen_buf_sizes(test, alloc, end_offset);
+		return;
+	}
+	prev = index == 0 ? 0 : end_offset[index - 1];
+	end = prev;
+
+	BUILD_BUG_ON(BUFFER_MIN_SIZE * BUFFER_NUM >= PAGE_SIZE);
+
+	for (align = SAME_PAGE_UNALIGNED; align < LOOP_END; align++) {
+		if (align % 2)
+			end = ALIGN(end, PAGE_SIZE);
+		else
+			end += BUFFER_MIN_SIZE;
+		end_offset[index] = end;
+		gen_buf_offsets(test, alloc, end_offset, index + 1);
+	}
+}
+
 struct binder_alloc_test {
 	struct binder_alloc alloc;
 	struct list_lru binder_test_freelist;
@@ -56,6 +315,25 @@ static void binder_alloc_test_mmap(struct kunit *test)
 	KUNIT_EXPECT_TRUE(test, list_is_last(&buf->entry, &alloc->buffers));
 }
 
+/**
+ * binder_alloc_exhaustive_test() - Exhaustively test alloc and free of buffer pages.
+ * @test: The test context object.
+ *
+ * Allocate BUFFER_NUM buffers to cover all page alignment cases,
+ * then free them in all orders possible. Check that pages are
+ * correctly allocated, put onto lru when buffers are freed, and
+ * are freed when binder_alloc_free_page() is called.
+ */
+static void binder_alloc_exhaustive_test(struct kunit *test)
+{
+	struct binder_alloc_test *priv = test->priv;
+	size_t end_offset[BUFFER_NUM];
+
+	gen_buf_offsets(test, &priv->alloc, end_offset, 0);
+
+	KUNIT_EXPECT_EQ(test, binder_alloc_test_failures, 0);
+}
+
 /* ===== End test cases ===== */
 
 static void binder_alloc_test_vma_close(struct vm_area_struct *vma)
@@ -149,6 +427,7 @@ static void binder_alloc_test_exit(struct kunit *test)
 static struct kunit_case binder_alloc_test_cases[] = {
 	KUNIT_CASE(binder_alloc_test_init_freelist),
 	KUNIT_CASE(binder_alloc_test_mmap),
+	KUNIT_CASE(binder_alloc_exhaustive_test),
 	{}
 };
 
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v3 5/6] binder: Convert binder_alloc selftests to KUnit

[Carlos Llamas]

On Mon, Jul 14, 2025 at 11:53:18AM -0700, Tiffany Yang wrote:
> Convert the existing binder_alloc_selftest tests into KUnit tests. These
> tests allocate and free an exhaustive combination of buffers with
> various sizes and alignments. This change allows them to be run without
> blocking or otherwise interfering with other processes in binder.
>
> This test is refactored into more meaningful cases in the subsequent
> patch.
>
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
> v2:
> * Fix build warning Reported-by: kernel test robot <lkp@intel.com>
>   Closes: https://lore.kernel.org/oe-kbuild-all/202506281837.hReNHJjO-lkp@intel.com/
> ---
>  drivers/android/Kconfig                    |  10 -
>  drivers/android/Makefile                   |   1 -
>  drivers/android/binder.c                   |   5 -
>  drivers/android/binder_alloc.c             |   3 +
>  drivers/android/binder_alloc.h             |   5 -
>  drivers/android/binder_alloc_selftest.c    | 345 ---------------------
>  drivers/android/tests/binder_alloc_kunit.c | 279 +++++++++++++++++
>  7 files changed, 282 insertions(+), 366 deletions(-)
>  delete mode 100644 drivers/android/binder_alloc_selftest.c
>
> diff --git a/drivers/android/Kconfig b/drivers/android/Kconfig
> index b1bc7183366c..5b3b8041f827 100644
> --- a/drivers/android/Kconfig
> +++ b/drivers/android/Kconfig
> @@ -37,16 +37,6 @@ config ANDROID_BINDER_DEVICES
>         created. Each binder device has its own context manager, and is
>         therefore logically separated from the other devices.
>
> -config ANDROID_BINDER_IPC_SELFTEST
> -     bool "Android Binder IPC Driver Selftest"
> -     depends on ANDROID_BINDER_IPC
> -     help
> -       This feature allows binder selftest to run.
> -
> -       Binder selftest checks the allocation and free of binder buffers
> -       exhaustively with combinations of various buffer sizes and
> -       alignments.
> -
>  config ANDROID_BINDER_ALLOC_KUNIT_TEST
>       tristate "KUnit Tests for Android Binder Alloc" if !KUNIT_ALL_TESTS
>       depends on ANDROID_BINDER_IPC && KUNIT
> diff --git a/drivers/android/Makefile b/drivers/android/Makefile
> index 74d02a335d4e..c5d47be0276c 100644
> --- a/drivers/android/Makefile
> +++ b/drivers/android/Makefile
> @@ -3,5 +3,4 @@ ccflags-y += -I$(src)                 # needed for trace events
>
>  obj-$(CONFIG_ANDROID_BINDERFS)               += binderfs.o
>  obj-$(CONFIG_ANDROID_BINDER_IPC)     += binder.o binder_alloc.o
> -obj-$(CONFIG_ANDROID_BINDER_IPC_SELFTEST) += binder_alloc_selftest.o
>  obj-$(CONFIG_ANDROID_BINDER_ALLOC_KUNIT_TEST)        += tests/
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 9dfe90c284fc..7b2653a5d59c 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5718,11 +5718,6 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>       struct binder_thread *thread;
>       void __user *ubuf = (void __user *)arg;
>
> -     /*pr_info("binder_ioctl: %d:%d %x %lx\n",
> -                     proc->pid, current->pid, cmd, arg);*/

Thanks, this had been bothering me for a while.

> -
> -     binder_selftest_alloc(&proc->alloc);
> -
>       trace_binder_ioctl(cmd, arg);
>
>       ret = wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
> diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
> index c79e5c6721f0..74a184014fa7 100644
> --- a/drivers/android/binder_alloc.c
> +++ b/drivers/android/binder_alloc.c
> @@ -701,6 +701,7 @@ struct binder_buffer *binder_alloc_new_buf(struct binder_alloc *alloc,
>  out:
>       return buffer;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_new_buf);
>
>  static unsigned long buffer_start_page(struct binder_buffer *buffer)
>  {
> @@ -879,6 +880,7 @@ void binder_alloc_free_buf(struct binder_alloc *alloc,
>       binder_free_buf_locked(alloc, buffer);
>       mutex_unlock(&alloc->mutex);
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_free_buf);
>
>  /**
>   * binder_alloc_mmap_handler() - map virtual address space for proc
> @@ -1217,6 +1219,7 @@ enum lru_status binder_alloc_free_page(struct list_head *item,
>  err_mmget:
>       return LRU_SKIP;
>  }
> +EXPORT_SYMBOL_IF_KUNIT(binder_alloc_free_page);
>
>  static unsigned long
>  binder_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
> diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
> index dc8dce2469a7..bed97c2cad92 100644
> --- a/drivers/android/binder_alloc.h
> +++ b/drivers/android/binder_alloc.h
> @@ -121,11 +121,6 @@ struct binder_alloc {
>       bool oneway_spam_detected;
>  };
>
> -#ifdef CONFIG_ANDROID_BINDER_IPC_SELFTEST
> -void binder_selftest_alloc(struct binder_alloc *alloc);
> -#else
> -static inline void binder_selftest_alloc(struct binder_alloc *alloc) {}
> -#endif
>  enum lru_status binder_alloc_free_page(struct list_head *item,
>                                      struct list_lru_one *lru,
>                                      void *cb_arg);
> diff --git a/drivers/android/binder_alloc_selftest.c b/drivers/android/binder_alloc_selftest.c
> deleted file mode 100644
> index 8b18b22aa3de..000000000000
> --- a/drivers/android/binder_alloc_selftest.c
> +++ /dev/null
> @@ -1,345 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/* binder_alloc_selftest.c
> - *
> - * Android IPC Subsystem
> - *
> - * Copyright (C) 2017 Google, Inc.
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/err.h>
> -#include <linux/list_lru.h>
> -#include <linux/mm_types.h>
> -#include "binder_alloc.h"
> -
> -#define BUFFER_NUM 5
> -#define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
> -
> -static bool binder_selftest_run = true;
> -static int binder_selftest_failures;
> -static DEFINE_MUTEX(binder_selftest_lock);
> -static struct list_lru binder_selftest_freelist;
> -
> -/**
> - * enum buf_end_align_type - Page alignment of a buffer
> - * end with regard to the end of the previous buffer.
> - *
> - * In the pictures below, buf2 refers to the buffer we
> - * are aligning. buf1 refers to previous buffer by addr.
> - * Symbol [ means the start of a buffer, ] means the end
> - * of a buffer, and | means page boundaries.
> - */
> -enum buf_end_align_type {
> -     /**
> -      * @SAME_PAGE_UNALIGNED: The end of this buffer is on
> -      * the same page as the end of the previous buffer and
> -      * is not page aligned. Examples:
> -      * buf1 ][ buf2 ][ ...
> -      * buf1 ]|[ buf2 ][ ...
> -      */
> -     SAME_PAGE_UNALIGNED = 0,
> -     /**
> -      * @SAME_PAGE_ALIGNED: When the end of the previous buffer
> -      * is not page aligned, the end of this buffer is on the
> -      * same page as the end of the previous buffer and is page
> -      * aligned. When the previous buffer is page aligned, the
> -      * end of this buffer is aligned to the next page boundary.
> -      * Examples:
> -      * buf1 ][ buf2 ]| ...
> -      * buf1 ]|[ buf2 ]| ...
> -      */
> -     SAME_PAGE_ALIGNED,
> -     /**
> -      * @NEXT_PAGE_UNALIGNED: The end of this buffer is on
> -      * the page next to the end of the previous buffer and
> -      * is not page aligned. Examples:
> -      * buf1 ][ buf2 | buf2 ][ ...
> -      * buf1 ]|[ buf2 | buf2 ][ ...
> -      */
> -     NEXT_PAGE_UNALIGNED,
> -     /**
> -      * @NEXT_PAGE_ALIGNED: The end of this buffer is on
> -      * the page next to the end of the previous buffer and
> -      * is page aligned. Examples:
> -      * buf1 ][ buf2 | buf2 ]| ...
> -      * buf1 ]|[ buf2 | buf2 ]| ...
> -      */
> -     NEXT_PAGE_ALIGNED,
> -     /**
> -      * @NEXT_NEXT_UNALIGNED: The end of this buffer is on
> -      * the page that follows the page after the end of the
> -      * previous buffer and is not page aligned. Examples:
> -      * buf1 ][ buf2 | buf2 | buf2 ][ ...
> -      * buf1 ]|[ buf2 | buf2 | buf2 ][ ...
> -      */
> -     NEXT_NEXT_UNALIGNED,
> -     /**
> -      * @LOOP_END: The number of enum values in &buf_end_align_type.
> -      * It is used for controlling loop termination.
> -      */
> -     LOOP_END,
> -};
> -
> -static void pr_err_size_seq(size_t *sizes, int *seq)
> -{
> -     int i;
> -
> -     pr_err("alloc sizes: ");
> -     for (i = 0; i < BUFFER_NUM; i++)
> -             pr_cont("[%zu]", sizes[i]);
> -     pr_cont("\n");
> -     pr_err("free seq: ");
> -     for (i = 0; i < BUFFER_NUM; i++)
> -             pr_cont("[%d]", seq[i]);
> -     pr_cont("\n");
> -}
> -
> -static bool check_buffer_pages_allocated(struct binder_alloc *alloc,
> -                                      struct binder_buffer *buffer,
> -                                      size_t size)
> -{
> -     unsigned long page_addr;
> -     unsigned long end;
> -     int page_index;
> -
> -     end = PAGE_ALIGN(buffer->user_data + size);
> -     page_addr = buffer->user_data;
> -     for (; page_addr < end; page_addr += PAGE_SIZE) {
> -             page_index = (page_addr - alloc->vm_start) / PAGE_SIZE;
> -             if (!alloc->pages[page_index] ||
> -                 !list_empty(page_to_lru(alloc->pages[page_index]))) {
> -                     pr_err("expect alloc but is %s at page index %d\n",
> -                            alloc->pages[page_index] ?
> -                            "lru" : "free", page_index);
> -                     return false;
> -             }
> -     }
> -     return true;
> -}
> -
> -static void binder_selftest_alloc_buf(struct binder_alloc *alloc,
> -                                   struct binder_buffer *buffers[],
> -                                   size_t *sizes, int *seq)
> -{
> -     int i;
> -
> -     for (i = 0; i < BUFFER_NUM; i++) {
> -             buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
> -             if (IS_ERR(buffers[i]) ||
> -                 !check_buffer_pages_allocated(alloc, buffers[i],
> -                                               sizes[i])) {
> -                     pr_err_size_seq(sizes, seq);
> -                     binder_selftest_failures++;
> -             }
> -     }
> -}
> -
> -static void binder_selftest_free_buf(struct binder_alloc *alloc,
> -                                  struct binder_buffer *buffers[],
> -                                  size_t *sizes, int *seq, size_t end)
> -{
> -     int i;
> -
> -     for (i = 0; i < BUFFER_NUM; i++)
> -             binder_alloc_free_buf(alloc, buffers[seq[i]]);
> -
> -     for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
> -             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",
> -                            alloc->pages[i] ? "alloc" : "free", i);
> -                     binder_selftest_failures++;
> -             }
> -     }
> -}
> -
> -static void binder_selftest_free_page(struct binder_alloc *alloc)
> -{
> -     int i;
> -     unsigned long count;
> -
> -     while ((count = list_lru_count(&binder_selftest_freelist))) {
> -             list_lru_walk(&binder_selftest_freelist, binder_alloc_free_page,
> -                           NULL, count);
> -     }
> -
> -     for (i = 0; i < (alloc->buffer_size / PAGE_SIZE); i++) {
> -             if (alloc->pages[i]) {
> -                     pr_err("expect free but is %s at page index %d\n",
> -                            list_empty(page_to_lru(alloc->pages[i])) ?
> -                            "alloc" : "lru", i);
> -                     binder_selftest_failures++;
> -             }
> -     }
> -}
> -
> -static void binder_selftest_alloc_free(struct binder_alloc *alloc,
> -                                    size_t *sizes, int *seq, size_t end)
> -{
> -     struct binder_buffer *buffers[BUFFER_NUM];
> -
> -     binder_selftest_alloc_buf(alloc, buffers, sizes, seq);
> -     binder_selftest_free_buf(alloc, buffers, sizes, seq, end);
> -
> -     /* Allocate from lru. */
> -     binder_selftest_alloc_buf(alloc, buffers, sizes, seq);
> -     if (list_lru_count(&binder_selftest_freelist))
> -             pr_err("lru list should be empty but is not\n");
> -
> -     binder_selftest_free_buf(alloc, buffers, sizes, seq, end);
> -     binder_selftest_free_page(alloc);
> -}
> -
> -static bool is_dup(int *seq, int index, int val)
> -{
> -     int i;
> -
> -     for (i = 0; i < index; i++) {
> -             if (seq[i] == val)
> -                     return true;
> -     }
> -     return false;
> -}
> -
> -/* Generate BUFFER_NUM factorial free orders. */
> -static void binder_selftest_free_seq(struct binder_alloc *alloc,
> -                                  size_t *sizes, int *seq,
> -                                  int index, size_t end)
> -{
> -     int i;
> -
> -     if (index == BUFFER_NUM) {
> -             binder_selftest_alloc_free(alloc, sizes, seq, end);
> -             return;
> -     }
> -     for (i = 0; i < BUFFER_NUM; i++) {
> -             if (is_dup(seq, index, i))
> -                     continue;
> -             seq[index] = i;
> -             binder_selftest_free_seq(alloc, sizes, seq, index + 1, end);
> -     }
> -}
> -
> -static void binder_selftest_alloc_size(struct binder_alloc *alloc,
> -                                    size_t *end_offset)
> -{
> -     int i;
> -     int seq[BUFFER_NUM] = {0};
> -     size_t front_sizes[BUFFER_NUM];
> -     size_t back_sizes[BUFFER_NUM];
> -     size_t last_offset, offset = 0;
> -
> -     for (i = 0; i < BUFFER_NUM; i++) {
> -             last_offset = offset;
> -             offset = end_offset[i];
> -             front_sizes[i] = offset - last_offset;
> -             back_sizes[BUFFER_NUM - i - 1] = front_sizes[i];
> -     }
> -     /*
> -      * Buffers share the first or last few pages.
> -      * Only BUFFER_NUM - 1 buffer sizes are adjustable since
> -      * we need one giant buffer before getting to the last page.
> -      */
> -     back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
> -     binder_selftest_free_seq(alloc, front_sizes, seq, 0,
> -                              end_offset[BUFFER_NUM - 1]);
> -     binder_selftest_free_seq(alloc, back_sizes, seq, 0, alloc->buffer_size);
> -}
> -
> -static void binder_selftest_alloc_offset(struct binder_alloc *alloc,
> -                                      size_t *end_offset, int index)
> -{
> -     int align;
> -     size_t end, prev;
> -
> -     if (index == BUFFER_NUM) {
> -             binder_selftest_alloc_size(alloc, end_offset);
> -             return;
> -     }
> -     prev = index == 0 ? 0 : end_offset[index - 1];
> -     end = prev;
> -
> -     BUILD_BUG_ON(BUFFER_MIN_SIZE * BUFFER_NUM >= PAGE_SIZE);
> -
> -     for (align = SAME_PAGE_UNALIGNED; align < LOOP_END; align++) {
> -             if (align % 2)
> -                     end = ALIGN(end, PAGE_SIZE);
> -             else
> -                     end += BUFFER_MIN_SIZE;
> -             end_offset[index] = end;
> -             binder_selftest_alloc_offset(alloc, end_offset, index + 1);
> -     }
> -}
> -
> -int binder_selftest_alloc_get_page_count(struct binder_alloc *alloc)
> -{
> -     struct page *page;
> -     int allocated = 0;
> -     int i;
> -
> -     for (i = 0; i < alloc->buffer_size / PAGE_SIZE; i++) {
> -             page = alloc->pages[i];
> -             if (page)
> -                     allocated++;
> -     }
> -     return allocated;
> -}
> -
> -/**
> - * binder_selftest_alloc() - Test alloc and free of buffer pages.
> - * @alloc: Pointer to alloc struct.
> - *
> - * Allocate BUFFER_NUM buffers to cover all page alignment cases,
> - * then free them in all orders possible. Check that pages are
> - * correctly allocated, put onto lru when buffers are freed, and
> - * are freed when binder_alloc_free_page is called.
> - */
> -void binder_selftest_alloc(struct binder_alloc *alloc)
> -{
> -     struct list_lru *prev_freelist;
> -     size_t end_offset[BUFFER_NUM];
> -
> -     if (!binder_selftest_run)
> -             return;
> -     mutex_lock(&binder_selftest_lock);
> -     if (!binder_selftest_run || !alloc->mapped)
> -             goto done;
> -
> -     prev_freelist = 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 = &binder_selftest_freelist;
> -
> -     pr_info("STARTED\n");
> -     binder_selftest_alloc_offset(alloc, end_offset, 0);
> -     if (binder_selftest_failures > 0)
> -             pr_info("%d tests FAILED\n", binder_selftest_failures);
> -     else
> -             pr_info("PASSED\n");
> -
> -     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 = false;
> -     alloc->freelist = prev_freelist;
> -     list_lru_destroy(&binder_selftest_freelist);
> -done:
> -     mutex_unlock(&binder_selftest_lock);
> -}
> diff --git a/drivers/android/tests/binder_alloc_kunit.c b/drivers/android/tests/binder_alloc_kunit.c
> index 4b68b5687d33..9e185e2036e5 100644
> --- a/drivers/android/tests/binder_alloc_kunit.c
> +++ b/drivers/android/tests/binder_alloc_kunit.c
> @@ -21,6 +21,265 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
>
>  #define BINDER_MMAP_SIZE SZ_128K
>
> +#define BUFFER_NUM 5
> +#define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
> +
> +static int binder_alloc_test_failures;
> +
> +/**
> + * enum buf_end_align_type - Page alignment of a buffer
> + * end with regard to the end of the previous buffer.
> + *
> + * In the pictures below, buf2 refers to the buffer we
> + * are aligning. buf1 refers to previous buffer by addr.
> + * Symbol [ means the start of a buffer, ] means the end
> + * of a buffer, and | means page boundaries.
> + */
> +enum buf_end_align_type {
> +     /**
> +      * @SAME_PAGE_UNALIGNED: The end of this buffer is on
> +      * the same page as the end of the previous buffer and
> +      * is not page aligned. Examples:
> +      * buf1 ][ buf2 ][ ...
> +      * buf1 ]|[ buf2 ][ ...
> +      */
> +     SAME_PAGE_UNALIGNED = 0,
> +     /**
> +      * @SAME_PAGE_ALIGNED: When the end of the previous buffer
> +      * is not page aligned, the end of this buffer is on the
> +      * same page as the end of the previous buffer and is page
> +      * aligned. When the previous buffer is page aligned, the
> +      * end of this buffer is aligned to the next page boundary.
> +      * Examples:
> +      * buf1 ][ buf2 ]| ...
> +      * buf1 ]|[ buf2 ]| ...
> +      */
> +     SAME_PAGE_ALIGNED,
> +     /**
> +      * @NEXT_PAGE_UNALIGNED: The end of this buffer is on
> +      * the page next to the end of the previous buffer and
> +      * is not page aligned. Examples:
> +      * buf1 ][ buf2 | buf2 ][ ...
> +      * buf1 ]|[ buf2 | buf2 ][ ...
> +      */
> +     NEXT_PAGE_UNALIGNED,
> +     /**
> +      * @NEXT_PAGE_ALIGNED: The end of this buffer is on
> +      * the page next to the end of the previous buffer and
> +      * is page aligned. Examples:
> +      * buf1 ][ buf2 | buf2 ]| ...
> +      * buf1 ]|[ buf2 | buf2 ]| ...
> +      */
> +     NEXT_PAGE_ALIGNED,
> +     /**
> +      * @NEXT_NEXT_UNALIGNED: The end of this buffer is on
> +      * the page that follows the page after the end of the
> +      * previous buffer and is not page aligned. Examples:
> +      * buf1 ][ buf2 | buf2 | buf2 ][ ...
> +      * buf1 ]|[ buf2 | buf2 | buf2 ][ ...
> +      */
> +     NEXT_NEXT_UNALIGNED,
> +     /**
> +      * @LOOP_END: The number of enum values in &buf_end_align_type.
> +      * It is used for controlling loop termination.
> +      */
> +     LOOP_END,
> +};
> +
> +static void pr_err_size_seq(struct kunit *test, size_t *sizes, int *seq)
> +{
> +     int i;
> +
> +     kunit_err(test, "alloc sizes: ");
> +     for (i = 0; i < BUFFER_NUM; i++)
> +             pr_cont("[%zu]", sizes[i]);
> +     pr_cont("\n");
> +     kunit_err(test, "free seq: ");
> +     for (i = 0; i < BUFFER_NUM; i++)
> +             pr_cont("[%d]", seq[i]);
> +     pr_cont("\n");
> +}
> +
> +static bool check_buffer_pages_allocated(struct kunit *test,
> +                                      struct binder_alloc *alloc,
> +                                      struct binder_buffer *buffer,
> +                                      size_t size)
> +{
> +     unsigned long page_addr;
> +     unsigned long end;
> +     int page_index;
> +
> +     end = PAGE_ALIGN(buffer->user_data + size);
> +     page_addr = buffer->user_data;
> +     for (; page_addr < end; page_addr += PAGE_SIZE) {
> +             page_index = (page_addr - alloc->vm_start) / PAGE_SIZE;
> +             if (!alloc->pages[page_index] ||
> +                 !list_empty(page_to_lru(alloc->pages[page_index]))) {
> +                     kunit_err(test, "expect alloc but is %s at page index %d\n",
> +                               alloc->pages[page_index] ?
> +                               "lru" : "free", page_index);
> +                     return false;
> +             }
> +     }
> +     return true;
> +}
> +
> +static void binder_alloc_test_alloc_buf(struct kunit *test,
> +                                     struct binder_alloc *alloc,
> +                                     struct binder_buffer *buffers[],
> +                                     size_t *sizes, int *seq)
> +{
> +     int i;
> +
> +     for (i = 0; i < BUFFER_NUM; i++) {
> +             buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
> +             if (IS_ERR(buffers[i]) ||
> +                 !check_buffer_pages_allocated(test, alloc, buffers[i], sizes[i])) {
> +                     pr_err_size_seq(test, sizes, seq);
> +                     binder_alloc_test_failures++;
> +             }
> +     }
> +}
> +
> +static void binder_alloc_test_free_buf(struct kunit *test,
> +                                    struct binder_alloc *alloc,
> +                                    struct binder_buffer *buffers[],
> +                                    size_t *sizes, int *seq, size_t end)
> +{
> +     int i;
> +
> +     for (i = 0; i < BUFFER_NUM; i++)
> +             binder_alloc_free_buf(alloc, buffers[seq[i]]);
> +
> +     for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
> +             if (list_empty(page_to_lru(alloc->pages[i]))) {
> +                     pr_err_size_seq(test, sizes, seq);
> +                     kunit_err(test, "expect lru but is %s at page index %d\n",
> +                               alloc->pages[i] ? "alloc" : "free", i);
> +                     binder_alloc_test_failures++;
> +             }
> +     }
> +}
> +
> +static void binder_alloc_test_free_page(struct kunit *test,
> +                                     struct binder_alloc *alloc)
> +{
> +     unsigned long count;
> +     int i;
> +
> +     while ((count = list_lru_count(alloc->freelist))) {
> +             list_lru_walk(alloc->freelist, binder_alloc_free_page,
> +                           NULL, count);
> +     }
> +
> +     for (i = 0; i < (alloc->buffer_size / PAGE_SIZE); i++) {
> +             if (alloc->pages[i]) {
> +                     kunit_err(test, "expect free but is %s at page index %d\n",
> +                               list_empty(page_to_lru(alloc->pages[i])) ?
> +                               "alloc" : "lru", i);
> +                     binder_alloc_test_failures++;
> +             }
> +     }
> +}
> +
> +static void binder_alloc_test_alloc_free(struct kunit *test,
> +                                      struct binder_alloc *alloc,
> +                                      size_t *sizes, int *seq, size_t end)
> +{
> +     struct binder_buffer *buffers[BUFFER_NUM];
> +
> +     binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
> +     binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
> +
> +     /* Allocate from lru. */
> +     binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
> +     if (list_lru_count(alloc->freelist))
> +             kunit_err(test, "lru list should be empty but is not\n");
> +
> +     binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
> +     binder_alloc_test_free_page(test, alloc);
> +}
> +
> +static bool is_dup(int *seq, int index, int val)
> +{
> +     int i;
> +
> +     for (i = 0; i < index; i++) {
> +             if (seq[i] == val)
> +                     return true;
> +     }
> +     return false;
> +}
> +
> +/* Generate BUFFER_NUM factorial free orders. */
> +static void permute_frees(struct kunit *test, struct binder_alloc *alloc,
> +                       size_t *sizes, int *seq, int index, size_t end)
> +{
> +     int i;
> +
> +     if (index == BUFFER_NUM) {
> +             binder_alloc_test_alloc_free(test, alloc, sizes, seq, end);
> +             return;
> +     }
> +     for (i = 0; i < BUFFER_NUM; i++) {
> +             if (is_dup(seq, index, i))
> +                     continue;
> +             seq[index] = i;
> +             permute_frees(test, alloc, sizes, seq, index + 1, end);
> +     }
> +}
> +
> +static void gen_buf_sizes(struct kunit *test, struct binder_alloc *alloc,
> +                       size_t *end_offset)
> +{
> +     size_t last_offset, offset = 0;
> +     size_t front_sizes[BUFFER_NUM];
> +     size_t back_sizes[BUFFER_NUM];
> +     int seq[BUFFER_NUM] = {0};
> +     int i;
> +
> +     for (i = 0; i < BUFFER_NUM; i++) {
> +             last_offset = offset;
> +             offset = end_offset[i];
> +             front_sizes[i] = offset - last_offset;
> +             back_sizes[BUFFER_NUM - i - 1] = front_sizes[i];
> +     }
> +     /*
> +      * Buffers share the first or last few pages.
> +      * Only BUFFER_NUM - 1 buffer sizes are adjustable since
> +      * we need one giant buffer before getting to the last page.
> +      */
> +     back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
> +     permute_frees(test, alloc, front_sizes, seq, 0,
> +                   end_offset[BUFFER_NUM - 1]);
> +     permute_frees(test, alloc, back_sizes, seq, 0, alloc->buffer_size);
> +}
> +
> +static void gen_buf_offsets(struct kunit *test, struct binder_alloc *alloc,
> +                         size_t *end_offset, int index)
> +{
> +     size_t end, prev;
> +     int align;
> +
> +     if (index == BUFFER_NUM) {
> +             gen_buf_sizes(test, alloc, end_offset);
> +             return;
> +     }
> +     prev = index == 0 ? 0 : end_offset[index - 1];
> +     end = prev;
> +
> +     BUILD_BUG_ON(BUFFER_MIN_SIZE * BUFFER_NUM >= PAGE_SIZE);
> +
> +     for (align = SAME_PAGE_UNALIGNED; align < LOOP_END; align++) {
> +             if (align % 2)
> +                     end = ALIGN(end, PAGE_SIZE);
> +             else
> +                     end += BUFFER_MIN_SIZE;
> +             end_offset[index] = end;
> +             gen_buf_offsets(test, alloc, end_offset, index + 1);
> +     }
> +}
> +
>  struct binder_alloc_test {
>       struct binder_alloc alloc;
>       struct list_lru binder_test_freelist;
> @@ -56,6 +315,25 @@ static void binder_alloc_test_mmap(struct kunit *test)
>       KUNIT_EXPECT_TRUE(test, list_is_last(&buf->entry, &alloc->buffers));
>  }
>
> +/**
> + * binder_alloc_exhaustive_test() - Exhaustively test alloc and free of buffer pages.
> + * @test: The test context object.
> + *
> + * Allocate BUFFER_NUM buffers to cover all page alignment cases,
> + * then free them in all orders possible. Check that pages are
> + * correctly allocated, put onto lru when buffers are freed, and
> + * are freed when binder_alloc_free_page() is called.
> + */
> +static void binder_alloc_exhaustive_test(struct kunit *test)
> +{
> +     struct binder_alloc_test *priv = test->priv;
> +     size_t end_offset[BUFFER_NUM];
> +
> +     gen_buf_offsets(test, &priv->alloc, end_offset, 0);
> +
> +     KUNIT_EXPECT_EQ(test, binder_alloc_test_failures, 0);
> +}
> +
>  /* ===== End test cases ===== */
>
>  static void binder_alloc_test_vma_close(struct vm_area_struct *vma)
> @@ -149,6 +427,7 @@ static void binder_alloc_test_exit(struct kunit *test)
>  static struct kunit_case binder_alloc_test_cases[] = {
>       KUNIT_CASE(binder_alloc_test_init_freelist),
>       KUNIT_CASE(binder_alloc_test_mmap),
> +     KUNIT_CASE(binder_alloc_exhaustive_test),
>       {}
>  };
>
> --
> 2.50.0.727.gbf7dc18ff4-goog
>

LGTM!

Acked-by: Carlos Llamas <cmllamas@google.com>

[PATCH v3 6/6] binder: encapsulate individual alloc test cases

[Tiffany Yang]

Each case tested by the binder allocator test is defined by 3 parameters:
the end alignment type of each requested buffer allocation, whether those
buffers share the front or back pages of the allotted address space, and
the order in which those buffers should be released. The alignment type
represents how a binder buffer may be laid out within or across page
boundaries and relative to other buffers, and it's used along with
whether the buffers cover part (sharing the front pages) of or all
(sharing the back pages) of the vma to calculate the sizes passed into
each test.

binder_alloc_test_alloc recursively generates each possible arrangement
of alignment types and then tests that the binder_alloc code tracks pages
correctly when those buffers are allocated and then freed in every
possible order at both ends of the address space. While they provide
comprehensive coverage, they are poor candidates to be represented as
KUnit test cases, which must be statically enumerated. For 5 buffers and
5 end alignment types, the test case array would have 750,000 entries.
This change structures the recursive calls into meaningful test cases so
that failures are easier to interpret.

Signed-off-by: Tiffany Yang <ynaffit@google.com>
---
v2:
* Fix build warning Reported-by: kernel test robot <lkp@intel.com>
  Closes: https://lore.kernel.org/oe-kbuild-all/202506281959.hfOTIUjS-lkp@intel.com/
---
 drivers/android/tests/binder_alloc_kunit.c | 234 ++++++++++++++++-----
 1 file changed, 181 insertions(+), 53 deletions(-)

diff --git a/drivers/android/tests/binder_alloc_kunit.c b/drivers/android/tests/binder_alloc_kunit.c
index 9e185e2036e5..02aa4a135eb5 100644
--- a/drivers/android/tests/binder_alloc_kunit.c
+++ b/drivers/android/tests/binder_alloc_kunit.c
@@ -24,7 +24,16 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
 #define BUFFER_NUM 5
 #define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
 
-static int binder_alloc_test_failures;
+#define FREESEQ_BUFLEN ((3 * BUFFER_NUM) + 1)
+
+#define ALIGN_TYPE_STRLEN (12)
+
+#define ALIGNMENTS_BUFLEN (((ALIGN_TYPE_STRLEN + 6) * BUFFER_NUM) + 1)
+
+#define PRINT_ALL_CASES (0)
+
+/* 5^5 alignment combinations * 2 places to share pages * 5! free sequences */
+#define TOTAL_EXHAUSTIVE_CASES (3125 * 2 * 120)
 
 /**
  * enum buf_end_align_type - Page alignment of a buffer
@@ -86,18 +95,49 @@ enum buf_end_align_type {
 	LOOP_END,
 };
 
-static void pr_err_size_seq(struct kunit *test, size_t *sizes, int *seq)
+static const char *const buf_end_align_type_strs[LOOP_END] = {
+	[SAME_PAGE_UNALIGNED] = "SP_UNALIGNED",
+	[SAME_PAGE_ALIGNED]   = " SP_ALIGNED ",
+	[NEXT_PAGE_UNALIGNED] = "NP_UNALIGNED",
+	[NEXT_PAGE_ALIGNED]   = " NP_ALIGNED ",
+	[NEXT_NEXT_UNALIGNED] = "NN_UNALIGNED",
+};
+
+struct binder_alloc_test_case_info {
+	size_t *buffer_sizes;
+	int *free_sequence;
+	char alignments[ALIGNMENTS_BUFLEN];
+	bool front_pages;
+};
+
+static void stringify_free_seq(struct kunit *test, int *seq, char *buf,
+			       size_t buf_len)
 {
+	size_t bytes = 0;
 	int i;
 
-	kunit_err(test, "alloc sizes: ");
-	for (i = 0; i < BUFFER_NUM; i++)
-		pr_cont("[%zu]", sizes[i]);
-	pr_cont("\n");
-	kunit_err(test, "free seq: ");
-	for (i = 0; i < BUFFER_NUM; i++)
-		pr_cont("[%d]", seq[i]);
-	pr_cont("\n");
+	for (i = 0; i < BUFFER_NUM; i++) {
+		bytes += snprintf(buf + bytes, buf_len - bytes, "[%d]", seq[i]);
+		if (bytes >= buf_len)
+			break;
+	}
+	KUNIT_EXPECT_LT(test, bytes, buf_len);
+}
+
+static void stringify_alignments(struct kunit *test, int *alignments,
+				 char *buf, size_t buf_len)
+{
+	size_t bytes = 0;
+	int i;
+
+	for (i = 0; i < BUFFER_NUM; i++) {
+		bytes += snprintf(buf + bytes, buf_len - bytes, "[ %d:%s ]", i,
+				  buf_end_align_type_strs[alignments[i]]);
+		if (bytes >= buf_len)
+			break;
+	}
+
+	KUNIT_EXPECT_LT(test, bytes, buf_len);
 }
 
 static bool check_buffer_pages_allocated(struct kunit *test,
@@ -124,28 +164,30 @@ static bool check_buffer_pages_allocated(struct kunit *test,
 	return true;
 }
 
-static void binder_alloc_test_alloc_buf(struct kunit *test,
-					struct binder_alloc *alloc,
-					struct binder_buffer *buffers[],
-					size_t *sizes, int *seq)
+static unsigned long binder_alloc_test_alloc_buf(struct kunit *test,
+						 struct binder_alloc *alloc,
+						 struct binder_buffer *buffers[],
+						 size_t *sizes, int *seq)
 {
+	unsigned long failures = 0;
 	int i;
 
 	for (i = 0; i < BUFFER_NUM; i++) {
 		buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
 		if (IS_ERR(buffers[i]) ||
-		    !check_buffer_pages_allocated(test, alloc, buffers[i], sizes[i])) {
-			pr_err_size_seq(test, sizes, seq);
-			binder_alloc_test_failures++;
-		}
+		    !check_buffer_pages_allocated(test, alloc, buffers[i], sizes[i]))
+			failures++;
 	}
+
+	return failures;
 }
 
-static void binder_alloc_test_free_buf(struct kunit *test,
-				       struct binder_alloc *alloc,
-				       struct binder_buffer *buffers[],
-				       size_t *sizes, int *seq, size_t end)
+static unsigned long binder_alloc_test_free_buf(struct kunit *test,
+						struct binder_alloc *alloc,
+						struct binder_buffer *buffers[],
+						size_t *sizes, int *seq, size_t end)
 {
+	unsigned long failures = 0;
 	int i;
 
 	for (i = 0; i < BUFFER_NUM; i++)
@@ -153,17 +195,19 @@ static void binder_alloc_test_free_buf(struct kunit *test,
 
 	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
 		if (list_empty(page_to_lru(alloc->pages[i]))) {
-			pr_err_size_seq(test, sizes, seq);
 			kunit_err(test, "expect lru but is %s at page index %d\n",
 				  alloc->pages[i] ? "alloc" : "free", i);
-			binder_alloc_test_failures++;
+			failures++;
 		}
 	}
+
+	return failures;
 }
 
-static void binder_alloc_test_free_page(struct kunit *test,
-					struct binder_alloc *alloc)
+static unsigned long binder_alloc_test_free_page(struct kunit *test,
+						 struct binder_alloc *alloc)
 {
+	unsigned long failures = 0;
 	unsigned long count;
 	int i;
 
@@ -177,27 +221,70 @@ static void binder_alloc_test_free_page(struct kunit *test,
 			kunit_err(test, "expect free but is %s at page index %d\n",
 				  list_empty(page_to_lru(alloc->pages[i])) ?
 				  "alloc" : "lru", i);
-			binder_alloc_test_failures++;
+			failures++;
 		}
 	}
+
+	return failures;
 }
 
-static void binder_alloc_test_alloc_free(struct kunit *test,
+/* Executes one full test run for the given test case. */
+static bool binder_alloc_test_alloc_free(struct kunit *test,
 					 struct binder_alloc *alloc,
-					 size_t *sizes, int *seq, size_t end)
+					 struct binder_alloc_test_case_info *tc,
+					 size_t end)
 {
+	unsigned long pages = PAGE_ALIGN(end) / PAGE_SIZE;
 	struct binder_buffer *buffers[BUFFER_NUM];
-
-	binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
-	binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
+	unsigned long failures;
+	bool failed = false;
+
+	failures = binder_alloc_test_alloc_buf(test, alloc, buffers,
+					       tc->buffer_sizes,
+					       tc->free_sequence);
+	failed = failed || failures;
+	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
+			    "Initial allocation failed: %lu/%u buffers with errors",
+			    failures, BUFFER_NUM);
+
+	failures = binder_alloc_test_free_buf(test, alloc, buffers,
+					      tc->buffer_sizes,
+					      tc->free_sequence, end);
+	failed = failed || failures;
+	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
+			    "Initial buffers not freed correctly: %lu/%lu pages not on lru list",
+			    failures, pages);
 
 	/* Allocate from lru. */
-	binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
-	if (list_lru_count(alloc->freelist))
-		kunit_err(test, "lru list should be empty but is not\n");
-
-	binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
-	binder_alloc_test_free_page(test, alloc);
+	failures = binder_alloc_test_alloc_buf(test, alloc, buffers,
+					       tc->buffer_sizes,
+					       tc->free_sequence);
+	failed = failed || failures;
+	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
+			    "Reallocation failed: %lu/%u buffers with errors",
+			    failures, BUFFER_NUM);
+
+	failures = list_lru_count(alloc->freelist);
+	failed = failed || failures;
+	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
+			    "lru list should be empty after reallocation but still has %lu pages",
+			    failures);
+
+	failures = binder_alloc_test_free_buf(test, alloc, buffers,
+					      tc->buffer_sizes,
+					      tc->free_sequence, end);
+	failed = failed || failures;
+	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
+			    "Reallocated buffers not freed correctly: %lu/%lu pages not on lru list",
+			    failures, pages);
+
+	failures = binder_alloc_test_free_page(test, alloc);
+	failed = failed || failures;
+	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
+			    "Failed to clean up allocated pages: %lu/%lu pages still installed",
+			    failures, (alloc->buffer_size / PAGE_SIZE));
+
+	return failed;
 }
 
 static bool is_dup(int *seq, int index, int val)
@@ -213,24 +300,44 @@ static bool is_dup(int *seq, int index, int val)
 
 /* Generate BUFFER_NUM factorial free orders. */
 static void permute_frees(struct kunit *test, struct binder_alloc *alloc,
-			  size_t *sizes, int *seq, int index, size_t end)
+			  struct binder_alloc_test_case_info *tc,
+			  unsigned long *runs, unsigned long *failures,
+			  int index, size_t end)
 {
+	bool case_failed;
 	int i;
 
 	if (index == BUFFER_NUM) {
-		binder_alloc_test_alloc_free(test, alloc, sizes, seq, end);
+		char freeseq_buf[FREESEQ_BUFLEN];
+
+		case_failed = binder_alloc_test_alloc_free(test, alloc, tc, end);
+		*runs += 1;
+		*failures += case_failed;
+
+		if (case_failed || PRINT_ALL_CASES) {
+			stringify_free_seq(test, tc->free_sequence, freeseq_buf,
+					   FREESEQ_BUFLEN);
+			kunit_err(test, "case %lu: [%s] | %s - %s - %s", *runs,
+				  case_failed ? "FAILED" : "PASSED",
+				  tc->front_pages ? "front" : "back ",
+				  tc->alignments, freeseq_buf);
+		}
+
 		return;
 	}
 	for (i = 0; i < BUFFER_NUM; i++) {
-		if (is_dup(seq, index, i))
+		if (is_dup(tc->free_sequence, index, i))
 			continue;
-		seq[index] = i;
-		permute_frees(test, alloc, sizes, seq, index + 1, end);
+		tc->free_sequence[index] = i;
+		permute_frees(test, alloc, tc, runs, failures, index + 1, end);
 	}
 }
 
-static void gen_buf_sizes(struct kunit *test, struct binder_alloc *alloc,
-			  size_t *end_offset)
+static void gen_buf_sizes(struct kunit *test,
+			  struct binder_alloc *alloc,
+			  struct binder_alloc_test_case_info *tc,
+			  size_t *end_offset, unsigned long *runs,
+			  unsigned long *failures)
 {
 	size_t last_offset, offset = 0;
 	size_t front_sizes[BUFFER_NUM];
@@ -238,31 +345,45 @@ static void gen_buf_sizes(struct kunit *test, struct binder_alloc *alloc,
 	int seq[BUFFER_NUM] = {0};
 	int i;
 
+	tc->free_sequence = seq;
 	for (i = 0; i < BUFFER_NUM; i++) {
 		last_offset = offset;
 		offset = end_offset[i];
 		front_sizes[i] = offset - last_offset;
 		back_sizes[BUFFER_NUM - i - 1] = front_sizes[i];
 	}
+	back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
+
 	/*
 	 * Buffers share the first or last few pages.
 	 * Only BUFFER_NUM - 1 buffer sizes are adjustable since
 	 * we need one giant buffer before getting to the last page.
 	 */
-	back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
-	permute_frees(test, alloc, front_sizes, seq, 0,
+	tc->front_pages = true;
+	tc->buffer_sizes = front_sizes;
+	permute_frees(test, alloc, tc, runs, failures, 0,
 		      end_offset[BUFFER_NUM - 1]);
-	permute_frees(test, alloc, back_sizes, seq, 0, alloc->buffer_size);
+
+	tc->front_pages = false;
+	tc->buffer_sizes = back_sizes;
+	permute_frees(test, alloc, tc, runs, failures, 0, alloc->buffer_size);
 }
 
 static void gen_buf_offsets(struct kunit *test, struct binder_alloc *alloc,
-			    size_t *end_offset, int index)
+			    size_t *end_offset, int *alignments,
+			    unsigned long *runs, unsigned long *failures,
+			    int index)
 {
 	size_t end, prev;
 	int align;
 
 	if (index == BUFFER_NUM) {
-		gen_buf_sizes(test, alloc, end_offset);
+		struct binder_alloc_test_case_info tc = {0};
+
+		stringify_alignments(test, alignments, tc.alignments,
+				     ALIGNMENTS_BUFLEN);
+
+		gen_buf_sizes(test, alloc, &tc, end_offset, runs, failures);
 		return;
 	}
 	prev = index == 0 ? 0 : end_offset[index - 1];
@@ -276,7 +397,9 @@ static void gen_buf_offsets(struct kunit *test, struct binder_alloc *alloc,
 		else
 			end += BUFFER_MIN_SIZE;
 		end_offset[index] = end;
-		gen_buf_offsets(test, alloc, end_offset, index + 1);
+		alignments[index] = align;
+		gen_buf_offsets(test, alloc, end_offset, alignments, runs,
+				failures, index + 1);
 	}
 }
 
@@ -328,10 +451,15 @@ static void binder_alloc_exhaustive_test(struct kunit *test)
 {
 	struct binder_alloc_test *priv = test->priv;
 	size_t end_offset[BUFFER_NUM];
+	int alignments[BUFFER_NUM];
+	unsigned long failures = 0;
+	unsigned long runs = 0;
 
-	gen_buf_offsets(test, &priv->alloc, end_offset, 0);
+	gen_buf_offsets(test, &priv->alloc, end_offset, alignments, &runs,
+			&failures, 0);
 
-	KUNIT_EXPECT_EQ(test, binder_alloc_test_failures, 0);
+	KUNIT_EXPECT_EQ(test, runs, TOTAL_EXHAUSTIVE_CASES);
+	KUNIT_EXPECT_EQ(test, failures, 0);
 }
 
 /* ===== End test cases ===== */
-- 
2.50.0.727.gbf7dc18ff4-goog

Re: [PATCH v3 6/6] binder: encapsulate individual alloc test cases

[Carlos Llamas]

On Mon, Jul 14, 2025 at 11:53:19AM -0700, Tiffany Yang wrote:
> Each case tested by the binder allocator test is defined by 3 parameters:
> the end alignment type of each requested buffer allocation, whether those
> buffers share the front or back pages of the allotted address space, and
> the order in which those buffers should be released. The alignment type
> represents how a binder buffer may be laid out within or across page
> boundaries and relative to other buffers, and it's used along with
> whether the buffers cover part (sharing the front pages) of or all
> (sharing the back pages) of the vma to calculate the sizes passed into
> each test.
>
> binder_alloc_test_alloc recursively generates each possible arrangement
> of alignment types and then tests that the binder_alloc code tracks pages
> correctly when those buffers are allocated and then freed in every
> possible order at both ends of the address space. While they provide
> comprehensive coverage, they are poor candidates to be represented as
> KUnit test cases, which must be statically enumerated. For 5 buffers and
> 5 end alignment types, the test case array would have 750,000 entries.
> This change structures the recursive calls into meaningful test cases so
> that failures are easier to interpret.
>
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---

Great job here. Thanks!

Acked-by: Carlos Llamas <cmllamas@google.com>

Re: [PATCH v3 6/6] binder: encapsulate individual alloc test cases

[Kees Cook]

On Mon, Jul 14, 2025 at 11:53:19AM -0700, Tiffany Yang wrote:
> Each case tested by the binder allocator test is defined by 3 parameters:
> the end alignment type of each requested buffer allocation, whether those
> buffers share the front or back pages of the allotted address space, and
> the order in which those buffers should be released. The alignment type
> represents how a binder buffer may be laid out within or across page
> boundaries and relative to other buffers, and it's used along with
> whether the buffers cover part (sharing the front pages) of or all
> (sharing the back pages) of the vma to calculate the sizes passed into
> each test.
> 
> binder_alloc_test_alloc recursively generates each possible arrangement
> of alignment types and then tests that the binder_alloc code tracks pages
> correctly when those buffers are allocated and then freed in every
> possible order at both ends of the address space. While they provide
> comprehensive coverage, they are poor candidates to be represented as
> KUnit test cases, which must be statically enumerated. For 5 buffers and
> 5 end alignment types, the test case array would have 750,000 entries.
> This change structures the recursive calls into meaningful test cases so
> that failures are easier to interpret.
> 
> Signed-off-by: Tiffany Yang <ynaffit@google.com>
> ---
> v2:
> * Fix build warning Reported-by: kernel test robot <lkp@intel.com>
>   Closes: https://lore.kernel.org/oe-kbuild-all/202506281959.hfOTIUjS-lkp@intel.com/
> ---
>  drivers/android/tests/binder_alloc_kunit.c | 234 ++++++++++++++++-----
>  1 file changed, 181 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/android/tests/binder_alloc_kunit.c b/drivers/android/tests/binder_alloc_kunit.c
> index 9e185e2036e5..02aa4a135eb5 100644
> --- a/drivers/android/tests/binder_alloc_kunit.c
> +++ b/drivers/android/tests/binder_alloc_kunit.c
> @@ -24,7 +24,16 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
>  #define BUFFER_NUM 5
>  #define BUFFER_MIN_SIZE (PAGE_SIZE / 8)
>  
> -static int binder_alloc_test_failures;
> +#define FREESEQ_BUFLEN ((3 * BUFFER_NUM) + 1)
> +
> +#define ALIGN_TYPE_STRLEN (12)
> +
> +#define ALIGNMENTS_BUFLEN (((ALIGN_TYPE_STRLEN + 6) * BUFFER_NUM) + 1)
> +
> +#define PRINT_ALL_CASES (0)
> +
> +/* 5^5 alignment combinations * 2 places to share pages * 5! free sequences */
> +#define TOTAL_EXHAUSTIVE_CASES (3125 * 2 * 120)
>  
>  /**
>   * enum buf_end_align_type - Page alignment of a buffer
> @@ -86,18 +95,49 @@ enum buf_end_align_type {
>  	LOOP_END,
>  };
>  
> -static void pr_err_size_seq(struct kunit *test, size_t *sizes, int *seq)
> +static const char *const buf_end_align_type_strs[LOOP_END] = {
> +	[SAME_PAGE_UNALIGNED] = "SP_UNALIGNED",
> +	[SAME_PAGE_ALIGNED]   = " SP_ALIGNED ",
> +	[NEXT_PAGE_UNALIGNED] = "NP_UNALIGNED",
> +	[NEXT_PAGE_ALIGNED]   = " NP_ALIGNED ",
> +	[NEXT_NEXT_UNALIGNED] = "NN_UNALIGNED",
> +};
> +
> +struct binder_alloc_test_case_info {
> +	size_t *buffer_sizes;
> +	int *free_sequence;
> +	char alignments[ALIGNMENTS_BUFLEN];
> +	bool front_pages;
> +};
> +
> +static void stringify_free_seq(struct kunit *test, int *seq, char *buf,
> +			       size_t buf_len)
>  {
> +	size_t bytes = 0;
>  	int i;
>  
> -	kunit_err(test, "alloc sizes: ");
> -	for (i = 0; i < BUFFER_NUM; i++)
> -		pr_cont("[%zu]", sizes[i]);
> -	pr_cont("\n");
> -	kunit_err(test, "free seq: ");
> -	for (i = 0; i < BUFFER_NUM; i++)
> -		pr_cont("[%d]", seq[i]);
> -	pr_cont("\n");
> +	for (i = 0; i < BUFFER_NUM; i++) {
> +		bytes += snprintf(buf + bytes, buf_len - bytes, "[%d]", seq[i]);
> +		if (bytes >= buf_len)
> +			break;
> +	}
> +	KUNIT_EXPECT_LT(test, bytes, buf_len);
> +}
> +
> +static void stringify_alignments(struct kunit *test, int *alignments,
> +				 char *buf, size_t buf_len)
> +{
> +	size_t bytes = 0;
> +	int i;
> +
> +	for (i = 0; i < BUFFER_NUM; i++) {
> +		bytes += snprintf(buf + bytes, buf_len - bytes, "[ %d:%s ]", i,
> +				  buf_end_align_type_strs[alignments[i]]);
> +		if (bytes >= buf_len)
> +			break;
> +	}
> +
> +	KUNIT_EXPECT_LT(test, bytes, buf_len);
>  }

For both stringify functions, snprintf is potentially unsafe. In the
spirit of recent string API discussions, please switch to using a
seq_buf:


static void stringify_free_seq(struct kunit *test, int *seq, seq_buf *buf)
{
	unsigned int i;

	for (i = 0; i < BUFFER_NUM; i++)
		seq_buf_printf(buf, "[%d]", seq[i])
	KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(buf));
}
...

	DECLARE_SEQ_BUF(freeseq_buf, FREESEQ_BUFLEN);
	...
	stringify_free_seq(test, tc->free_sequence, &freeseq_buf);



>  
>  static bool check_buffer_pages_allocated(struct kunit *test,
> @@ -124,28 +164,30 @@ static bool check_buffer_pages_allocated(struct kunit *test,
>  	return true;
>  }
>  
> -static void binder_alloc_test_alloc_buf(struct kunit *test,
> -					struct binder_alloc *alloc,
> -					struct binder_buffer *buffers[],
> -					size_t *sizes, int *seq)
> +static unsigned long binder_alloc_test_alloc_buf(struct kunit *test,
> +						 struct binder_alloc *alloc,
> +						 struct binder_buffer *buffers[],
> +						 size_t *sizes, int *seq)
>  {
> +	unsigned long failures = 0;
>  	int i;
>  
>  	for (i = 0; i < BUFFER_NUM; i++) {
>  		buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
>  		if (IS_ERR(buffers[i]) ||
> -		    !check_buffer_pages_allocated(test, alloc, buffers[i], sizes[i])) {
> -			pr_err_size_seq(test, sizes, seq);
> -			binder_alloc_test_failures++;
> -		}
> +		    !check_buffer_pages_allocated(test, alloc, buffers[i], sizes[i]))
> +			failures++;
>  	}
> +
> +	return failures;
>  }
>  
> -static void binder_alloc_test_free_buf(struct kunit *test,
> -				       struct binder_alloc *alloc,
> -				       struct binder_buffer *buffers[],
> -				       size_t *sizes, int *seq, size_t end)
> +static unsigned long binder_alloc_test_free_buf(struct kunit *test,
> +						struct binder_alloc *alloc,
> +						struct binder_buffer *buffers[],
> +						size_t *sizes, int *seq, size_t end)
>  {
> +	unsigned long failures = 0;
>  	int i;
>  
>  	for (i = 0; i < BUFFER_NUM; i++)
> @@ -153,17 +195,19 @@ static void binder_alloc_test_free_buf(struct kunit *test,
>  
>  	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
>  		if (list_empty(page_to_lru(alloc->pages[i]))) {
> -			pr_err_size_seq(test, sizes, seq);
>  			kunit_err(test, "expect lru but is %s at page index %d\n",
>  				  alloc->pages[i] ? "alloc" : "free", i);
> -			binder_alloc_test_failures++;
> +			failures++;
>  		}
>  	}
> +
> +	return failures;
>  }
>  
> -static void binder_alloc_test_free_page(struct kunit *test,
> -					struct binder_alloc *alloc)
> +static unsigned long binder_alloc_test_free_page(struct kunit *test,
> +						 struct binder_alloc *alloc)
>  {
> +	unsigned long failures = 0;
>  	unsigned long count;
>  	int i;
>  
> @@ -177,27 +221,70 @@ static void binder_alloc_test_free_page(struct kunit *test,
>  			kunit_err(test, "expect free but is %s at page index %d\n",
>  				  list_empty(page_to_lru(alloc->pages[i])) ?
>  				  "alloc" : "lru", i);
> -			binder_alloc_test_failures++;
> +			failures++;
>  		}
>  	}
> +
> +	return failures;
>  }
>  
> -static void binder_alloc_test_alloc_free(struct kunit *test,
> +/* Executes one full test run for the given test case. */
> +static bool binder_alloc_test_alloc_free(struct kunit *test,
>  					 struct binder_alloc *alloc,
> -					 size_t *sizes, int *seq, size_t end)
> +					 struct binder_alloc_test_case_info *tc,
> +					 size_t end)
>  {
> +	unsigned long pages = PAGE_ALIGN(end) / PAGE_SIZE;
>  	struct binder_buffer *buffers[BUFFER_NUM];
> -
> -	binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
> -	binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
> +	unsigned long failures;
> +	bool failed = false;
> +
> +	failures = binder_alloc_test_alloc_buf(test, alloc, buffers,
> +					       tc->buffer_sizes,
> +					       tc->free_sequence);
> +	failed = failed || failures;
> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
> +			    "Initial allocation failed: %lu/%u buffers with errors",
> +			    failures, BUFFER_NUM);
> +
> +	failures = binder_alloc_test_free_buf(test, alloc, buffers,
> +					      tc->buffer_sizes,
> +					      tc->free_sequence, end);
> +	failed = failed || failures;
> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
> +			    "Initial buffers not freed correctly: %lu/%lu pages not on lru list",
> +			    failures, pages);
>  
>  	/* Allocate from lru. */
> -	binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
> -	if (list_lru_count(alloc->freelist))
> -		kunit_err(test, "lru list should be empty but is not\n");
> -
> -	binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
> -	binder_alloc_test_free_page(test, alloc);
> +	failures = binder_alloc_test_alloc_buf(test, alloc, buffers,
> +					       tc->buffer_sizes,
> +					       tc->free_sequence);
> +	failed = failed || failures;
> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
> +			    "Reallocation failed: %lu/%u buffers with errors",
> +			    failures, BUFFER_NUM);
> +
> +	failures = list_lru_count(alloc->freelist);
> +	failed = failed || failures;
> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
> +			    "lru list should be empty after reallocation but still has %lu pages",
> +			    failures);
> +
> +	failures = binder_alloc_test_free_buf(test, alloc, buffers,
> +					      tc->buffer_sizes,
> +					      tc->free_sequence, end);
> +	failed = failed || failures;
> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
> +			    "Reallocated buffers not freed correctly: %lu/%lu pages not on lru list",
> +			    failures, pages);
> +
> +	failures = binder_alloc_test_free_page(test, alloc);
> +	failed = failed || failures;
> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
> +			    "Failed to clean up allocated pages: %lu/%lu pages still installed",
> +			    failures, (alloc->buffer_size / PAGE_SIZE));
> +
> +	return failed;
>  }
>  
>  static bool is_dup(int *seq, int index, int val)
> @@ -213,24 +300,44 @@ static bool is_dup(int *seq, int index, int val)
>  
>  /* Generate BUFFER_NUM factorial free orders. */
>  static void permute_frees(struct kunit *test, struct binder_alloc *alloc,
> -			  size_t *sizes, int *seq, int index, size_t end)
> +			  struct binder_alloc_test_case_info *tc,
> +			  unsigned long *runs, unsigned long *failures,
> +			  int index, size_t end)
>  {
> +	bool case_failed;
>  	int i;
>  
>  	if (index == BUFFER_NUM) {
> -		binder_alloc_test_alloc_free(test, alloc, sizes, seq, end);
> +		char freeseq_buf[FREESEQ_BUFLEN];
> +
> +		case_failed = binder_alloc_test_alloc_free(test, alloc, tc, end);
> +		*runs += 1;
> +		*failures += case_failed;
> +
> +		if (case_failed || PRINT_ALL_CASES) {
> +			stringify_free_seq(test, tc->free_sequence, freeseq_buf,
> +					   FREESEQ_BUFLEN);
> +			kunit_err(test, "case %lu: [%s] | %s - %s - %s", *runs,
> +				  case_failed ? "FAILED" : "PASSED",
> +				  tc->front_pages ? "front" : "back ",
> +				  tc->alignments, freeseq_buf);
> +		}
> +
>  		return;
>  	}
>  	for (i = 0; i < BUFFER_NUM; i++) {
> -		if (is_dup(seq, index, i))
> +		if (is_dup(tc->free_sequence, index, i))
>  			continue;
> -		seq[index] = i;
> -		permute_frees(test, alloc, sizes, seq, index + 1, end);
> +		tc->free_sequence[index] = i;
> +		permute_frees(test, alloc, tc, runs, failures, index + 1, end);
>  	}
>  }
>  
> -static void gen_buf_sizes(struct kunit *test, struct binder_alloc *alloc,
> -			  size_t *end_offset)
> +static void gen_buf_sizes(struct kunit *test,
> +			  struct binder_alloc *alloc,
> +			  struct binder_alloc_test_case_info *tc,
> +			  size_t *end_offset, unsigned long *runs,
> +			  unsigned long *failures)
>  {
>  	size_t last_offset, offset = 0;
>  	size_t front_sizes[BUFFER_NUM];
> @@ -238,31 +345,45 @@ static void gen_buf_sizes(struct kunit *test, struct binder_alloc *alloc,
>  	int seq[BUFFER_NUM] = {0};
>  	int i;
>  
> +	tc->free_sequence = seq;
>  	for (i = 0; i < BUFFER_NUM; i++) {
>  		last_offset = offset;
>  		offset = end_offset[i];
>  		front_sizes[i] = offset - last_offset;
>  		back_sizes[BUFFER_NUM - i - 1] = front_sizes[i];
>  	}
> +	back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
> +
>  	/*
>  	 * Buffers share the first or last few pages.
>  	 * Only BUFFER_NUM - 1 buffer sizes are adjustable since
>  	 * we need one giant buffer before getting to the last page.
>  	 */
> -	back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
> -	permute_frees(test, alloc, front_sizes, seq, 0,
> +	tc->front_pages = true;
> +	tc->buffer_sizes = front_sizes;
> +	permute_frees(test, alloc, tc, runs, failures, 0,
>  		      end_offset[BUFFER_NUM - 1]);
> -	permute_frees(test, alloc, back_sizes, seq, 0, alloc->buffer_size);
> +
> +	tc->front_pages = false;
> +	tc->buffer_sizes = back_sizes;
> +	permute_frees(test, alloc, tc, runs, failures, 0, alloc->buffer_size);
>  }
>  
>  static void gen_buf_offsets(struct kunit *test, struct binder_alloc *alloc,
> -			    size_t *end_offset, int index)
> +			    size_t *end_offset, int *alignments,
> +			    unsigned long *runs, unsigned long *failures,
> +			    int index)
>  {
>  	size_t end, prev;
>  	int align;
>  
>  	if (index == BUFFER_NUM) {
> -		gen_buf_sizes(test, alloc, end_offset);
> +		struct binder_alloc_test_case_info tc = {0};
> +
> +		stringify_alignments(test, alignments, tc.alignments,
> +				     ALIGNMENTS_BUFLEN);
> +
> +		gen_buf_sizes(test, alloc, &tc, end_offset, runs, failures);
>  		return;
>  	}
>  	prev = index == 0 ? 0 : end_offset[index - 1];
> @@ -276,7 +397,9 @@ static void gen_buf_offsets(struct kunit *test, struct binder_alloc *alloc,
>  		else
>  			end += BUFFER_MIN_SIZE;
>  		end_offset[index] = end;
> -		gen_buf_offsets(test, alloc, end_offset, index + 1);
> +		alignments[index] = align;
> +		gen_buf_offsets(test, alloc, end_offset, alignments, runs,
> +				failures, index + 1);
>  	}
>  }
>  
> @@ -328,10 +451,15 @@ static void binder_alloc_exhaustive_test(struct kunit *test)
>  {
>  	struct binder_alloc_test *priv = test->priv;
>  	size_t end_offset[BUFFER_NUM];
> +	int alignments[BUFFER_NUM];
> +	unsigned long failures = 0;
> +	unsigned long runs = 0;
>  
> -	gen_buf_offsets(test, &priv->alloc, end_offset, 0);
> +	gen_buf_offsets(test, &priv->alloc, end_offset, alignments, &runs,
> +			&failures, 0);
>  
> -	KUNIT_EXPECT_EQ(test, binder_alloc_test_failures, 0);
> +	KUNIT_EXPECT_EQ(test, runs, TOTAL_EXHAUSTIVE_CASES);
> +	KUNIT_EXPECT_EQ(test, failures, 0);
>  }
>  
>  /* ===== End test cases ===== */
> -- 
> 2.50.0.727.gbf7dc18ff4-goog
> 

Otherwise looks good to me.

-- 
Kees Cook

Re: [PATCH v3 6/6] binder: encapsulate individual alloc test cases

[Tiffany Yang]

Kees Cook <kees@kernel.org> writes:


> For both stringify functions, snprintf is potentially unsafe. In the
> spirit of recent string API discussions, please switch to using a
> seq_buf:


> static void stringify_free_seq(struct kunit *test, int *seq, seq_buf *buf)
> {
> 	unsigned int i;

> 	for (i = 0; i < BUFFER_NUM; i++)
> 		seq_buf_printf(buf, "[%d]", seq[i])
> 	KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(buf));
> }
> ...

> 	DECLARE_SEQ_BUF(freeseq_buf, FREESEQ_BUFLEN);
> 	...
> 	stringify_free_seq(test, tc->free_sequence, &freeseq_buf);




Thanks for calling attention to this! Will be fixed for v4!


>>   static bool check_buffer_pages_allocated(struct kunit *test,
>> @@ -124,28 +164,30 @@ static bool check_buffer_pages_allocated(struct  
>> kunit *test,
>>   	return true;
>>   }

>> -static void binder_alloc_test_alloc_buf(struct kunit *test,
>> -					struct binder_alloc *alloc,
>> -					struct binder_buffer *buffers[],
>> -					size_t *sizes, int *seq)
>> +static unsigned long binder_alloc_test_alloc_buf(struct kunit *test,
>> +						 struct binder_alloc *alloc,
>> +						 struct binder_buffer *buffers[],
>> +						 size_t *sizes, int *seq)
>>   {
>> +	unsigned long failures = 0;
>>   	int i;

>>   	for (i = 0; i < BUFFER_NUM; i++) {
>>   		buffers[i] = binder_alloc_new_buf(alloc, sizes[i], 0, 0, 0);
>>   		if (IS_ERR(buffers[i]) ||
>> -		    !check_buffer_pages_allocated(test, alloc, buffers[i], sizes[i]))  
>> {
>> -			pr_err_size_seq(test, sizes, seq);
>> -			binder_alloc_test_failures++;
>> -		}
>> +		    !check_buffer_pages_allocated(test, alloc, buffers[i], sizes[i]))
>> +			failures++;
>>   	}
>> +
>> +	return failures;
>>   }

>> -static void binder_alloc_test_free_buf(struct kunit *test,
>> -				       struct binder_alloc *alloc,
>> -				       struct binder_buffer *buffers[],
>> -				       size_t *sizes, int *seq, size_t end)
>> +static unsigned long binder_alloc_test_free_buf(struct kunit *test,
>> +						struct binder_alloc *alloc,
>> +						struct binder_buffer *buffers[],
>> +						size_t *sizes, int *seq, size_t end)
>>   {
>> +	unsigned long failures = 0;
>>   	int i;

>>   	for (i = 0; i < BUFFER_NUM; i++)
>> @@ -153,17 +195,19 @@ static void binder_alloc_test_free_buf(struct  
>> kunit *test,

>>   	for (i = 0; i <= (end - 1) / PAGE_SIZE; i++) {
>>   		if (list_empty(page_to_lru(alloc->pages[i]))) {
>> -			pr_err_size_seq(test, sizes, seq);
>>   			kunit_err(test, "expect lru but is %s at page index %d\n",
>>   				  alloc->pages[i] ? "alloc" : "free", i);
>> -			binder_alloc_test_failures++;
>> +			failures++;
>>   		}
>>   	}
>> +
>> +	return failures;
>>   }

>> -static void binder_alloc_test_free_page(struct kunit *test,
>> -					struct binder_alloc *alloc)
>> +static unsigned long binder_alloc_test_free_page(struct kunit *test,
>> +						 struct binder_alloc *alloc)
>>   {
>> +	unsigned long failures = 0;
>>   	unsigned long count;
>>   	int i;

>> @@ -177,27 +221,70 @@ static void binder_alloc_test_free_page(struct  
>> kunit *test,
>>   			kunit_err(test, "expect free but is %s at page index %d\n",
>>   				  list_empty(page_to_lru(alloc->pages[i])) ?
>>   				  "alloc" : "lru", i);
>> -			binder_alloc_test_failures++;
>> +			failures++;
>>   		}
>>   	}
>> +
>> +	return failures;
>>   }

>> -static void binder_alloc_test_alloc_free(struct kunit *test,
>> +/* Executes one full test run for the given test case. */
>> +static bool binder_alloc_test_alloc_free(struct kunit *test,
>>   					 struct binder_alloc *alloc,
>> -					 size_t *sizes, int *seq, size_t end)
>> +					 struct binder_alloc_test_case_info *tc,
>> +					 size_t end)
>>   {
>> +	unsigned long pages = PAGE_ALIGN(end) / PAGE_SIZE;
>>   	struct binder_buffer *buffers[BUFFER_NUM];
>> -
>> -	binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
>> -	binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
>> +	unsigned long failures;
>> +	bool failed = false;
>> +
>> +	failures = binder_alloc_test_alloc_buf(test, alloc, buffers,
>> +					       tc->buffer_sizes,
>> +					       tc->free_sequence);
>> +	failed = failed || failures;
>> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
>> +			    "Initial allocation failed: %lu/%u buffers with errors",
>> +			    failures, BUFFER_NUM);
>> +
>> +	failures = binder_alloc_test_free_buf(test, alloc, buffers,
>> +					      tc->buffer_sizes,
>> +					      tc->free_sequence, end);
>> +	failed = failed || failures;
>> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
>> +			    "Initial buffers not freed correctly: %lu/%lu pages not on lru  
>> list",
>> +			    failures, pages);

>>   	/* Allocate from lru. */
>> -	binder_alloc_test_alloc_buf(test, alloc, buffers, sizes, seq);
>> -	if (list_lru_count(alloc->freelist))
>> -		kunit_err(test, "lru list should be empty but is not\n");
>> -
>> -	binder_alloc_test_free_buf(test, alloc, buffers, sizes, seq, end);
>> -	binder_alloc_test_free_page(test, alloc);
>> +	failures = binder_alloc_test_alloc_buf(test, alloc, buffers,
>> +					       tc->buffer_sizes,
>> +					       tc->free_sequence);
>> +	failed = failed || failures;
>> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
>> +			    "Reallocation failed: %lu/%u buffers with errors",
>> +			    failures, BUFFER_NUM);
>> +
>> +	failures = list_lru_count(alloc->freelist);
>> +	failed = failed || failures;
>> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
>> +			    "lru list should be empty after reallocation but still has %lu  
>> pages",
>> +			    failures);
>> +
>> +	failures = binder_alloc_test_free_buf(test, alloc, buffers,
>> +					      tc->buffer_sizes,
>> +					      tc->free_sequence, end);
>> +	failed = failed || failures;
>> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
>> +			    "Reallocated buffers not freed correctly: %lu/%lu pages not on  
>> lru list",
>> +			    failures, pages);
>> +
>> +	failures = binder_alloc_test_free_page(test, alloc);
>> +	failed = failed || failures;
>> +	KUNIT_EXPECT_EQ_MSG(test, failures, 0,
>> +			    "Failed to clean up allocated pages: %lu/%lu pages still  
>> installed",
>> +			    failures, (alloc->buffer_size / PAGE_SIZE));
>> +
>> +	return failed;
>>   }

>>   static bool is_dup(int *seq, int index, int val)
>> @@ -213,24 +300,44 @@ static bool is_dup(int *seq, int index, int val)

>>   /* Generate BUFFER_NUM factorial free orders. */
>>   static void permute_frees(struct kunit *test, struct binder_alloc  
>> *alloc,
>> -			  size_t *sizes, int *seq, int index, size_t end)
>> +			  struct binder_alloc_test_case_info *tc,
>> +			  unsigned long *runs, unsigned long *failures,
>> +			  int index, size_t end)
>>   {
>> +	bool case_failed;
>>   	int i;

>>   	if (index == BUFFER_NUM) {
>> -		binder_alloc_test_alloc_free(test, alloc, sizes, seq, end);
>> +		char freeseq_buf[FREESEQ_BUFLEN];
>> +
>> +		case_failed = binder_alloc_test_alloc_free(test, alloc, tc, end);
>> +		*runs += 1;
>> +		*failures += case_failed;
>> +
>> +		if (case_failed || PRINT_ALL_CASES) {
>> +			stringify_free_seq(test, tc->free_sequence, freeseq_buf,
>> +					   FREESEQ_BUFLEN);
>> +			kunit_err(test, "case %lu: [%s] | %s - %s - %s", *runs,
>> +				  case_failed ? "FAILED" : "PASSED",
>> +				  tc->front_pages ? "front" : "back ",
>> +				  tc->alignments, freeseq_buf);
>> +		}
>> +
>>   		return;
>>   	}
>>   	for (i = 0; i < BUFFER_NUM; i++) {
>> -		if (is_dup(seq, index, i))
>> +		if (is_dup(tc->free_sequence, index, i))
>>   			continue;
>> -		seq[index] = i;
>> -		permute_frees(test, alloc, sizes, seq, index + 1, end);
>> +		tc->free_sequence[index] = i;
>> +		permute_frees(test, alloc, tc, runs, failures, index + 1, end);
>>   	}
>>   }

>> -static void gen_buf_sizes(struct kunit *test, struct binder_alloc  
>> *alloc,
>> -			  size_t *end_offset)
>> +static void gen_buf_sizes(struct kunit *test,
>> +			  struct binder_alloc *alloc,
>> +			  struct binder_alloc_test_case_info *tc,
>> +			  size_t *end_offset, unsigned long *runs,
>> +			  unsigned long *failures)
>>   {
>>   	size_t last_offset, offset = 0;
>>   	size_t front_sizes[BUFFER_NUM];
>> @@ -238,31 +345,45 @@ static void gen_buf_sizes(struct kunit *test,  
>> struct binder_alloc *alloc,
>>   	int seq[BUFFER_NUM] = {0};
>>   	int i;

>> +	tc->free_sequence = seq;
>>   	for (i = 0; i < BUFFER_NUM; i++) {
>>   		last_offset = offset;
>>   		offset = end_offset[i];
>>   		front_sizes[i] = offset - last_offset;
>>   		back_sizes[BUFFER_NUM - i - 1] = front_sizes[i];
>>   	}
>> +	back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
>> +
>>   	/*
>>   	 * Buffers share the first or last few pages.
>>   	 * Only BUFFER_NUM - 1 buffer sizes are adjustable since
>>   	 * we need one giant buffer before getting to the last page.
>>   	 */
>> -	back_sizes[0] += alloc->buffer_size - end_offset[BUFFER_NUM - 1];
>> -	permute_frees(test, alloc, front_sizes, seq, 0,
>> +	tc->front_pages = true;
>> +	tc->buffer_sizes = front_sizes;
>> +	permute_frees(test, alloc, tc, runs, failures, 0,
>>   		      end_offset[BUFFER_NUM - 1]);
>> -	permute_frees(test, alloc, back_sizes, seq, 0, alloc->buffer_size);
>> +
>> +	tc->front_pages = false;
>> +	tc->buffer_sizes = back_sizes;
>> +	permute_frees(test, alloc, tc, runs, failures, 0, alloc->buffer_size);
>>   }

>>   static void gen_buf_offsets(struct kunit *test, struct binder_alloc  
>> *alloc,
>> -			    size_t *end_offset, int index)
>> +			    size_t *end_offset, int *alignments,
>> +			    unsigned long *runs, unsigned long *failures,
>> +			    int index)
>>   {
>>   	size_t end, prev;
>>   	int align;

>>   	if (index == BUFFER_NUM) {
>> -		gen_buf_sizes(test, alloc, end_offset);
>> +		struct binder_alloc_test_case_info tc = {0};
>> +
>> +		stringify_alignments(test, alignments, tc.alignments,
>> +				     ALIGNMENTS_BUFLEN);
>> +
>> +		gen_buf_sizes(test, alloc, &tc, end_offset, runs, failures);
>>   		return;
>>   	}
>>   	prev = index == 0 ? 0 : end_offset[index - 1];
>> @@ -276,7 +397,9 @@ static void gen_buf_offsets(struct kunit *test,  
>> struct binder_alloc *alloc,
>>   		else
>>   			end += BUFFER_MIN_SIZE;
>>   		end_offset[index] = end;
>> -		gen_buf_offsets(test, alloc, end_offset, index + 1);
>> +		alignments[index] = align;
>> +		gen_buf_offsets(test, alloc, end_offset, alignments, runs,
>> +				failures, index + 1);
>>   	}
>>   }

>> @@ -328,10 +451,15 @@ static void binder_alloc_exhaustive_test(struct  
>> kunit *test)
>>   {
>>   	struct binder_alloc_test *priv = test->priv;
>>   	size_t end_offset[BUFFER_NUM];
>> +	int alignments[BUFFER_NUM];
>> +	unsigned long failures = 0;
>> +	unsigned long runs = 0;

>> -	gen_buf_offsets(test, &priv->alloc, end_offset, 0);
>> +	gen_buf_offsets(test, &priv->alloc, end_offset, alignments, &runs,
>> +			&failures, 0);

>> -	KUNIT_EXPECT_EQ(test, binder_alloc_test_failures, 0);
>> +	KUNIT_EXPECT_EQ(test, runs, TOTAL_EXHAUSTIVE_CASES);
>> +	KUNIT_EXPECT_EQ(test, failures, 0);
>>   }

>>   /* ===== End test cases ===== */
>> --
>> 2.50.0.727.gbf7dc18ff4-goog


> Otherwise looks good to me.

-- 
Tiffany Y. Yang