[PATCH v1 0/8] Run KUnit tests late and handle faults

[PATCH v1 0/8] Run KUnit tests late and handle faults

[Mickaël Salaün]

Hi,

This patch series moves KUnit test execution at the very end of kernel
initialization, just before launching the init process.  This opens the
way to test any kernel code in its normal state (i.e. fully
initialized).

This patch series also teaches KUnit to handle kthread faults as errors,
and it brings a few related fixes and improvements.

New tests check NULL pointer dereference and read-only memory, which
wasn't possible before.

This is useful to test current kernel self-protection mechanisms or
future ones such as Heki: https://github.com/heki-linux

Regards,

Mickaël Salaün (8):
  kunit: Run tests when the kernel is fully setup
  kunit: Handle thread creation error
  kunit: Fix kthread reference
  kunit: Fix timeout message
  kunit: Handle test faults
  kunit: Fix KUNIT_SUCCESS() calls in iov_iter tests
  kunit: Print last test location on fault
  kunit: Add tests for faults

 include/kunit/test.h                |  24 +++++-
 include/kunit/try-catch.h           |   3 -
 init/main.c                         |   4 +-
 lib/bitfield_kunit.c                |   8 +-
 lib/checksum_kunit.c                |   2 +-
 lib/kunit/executor.c                |  81 ++++++++++++++------
 lib/kunit/kunit-example-test.c      |   6 +-
 lib/kunit/kunit-test.c              | 115 +++++++++++++++++++++++++++-
 lib/kunit/try-catch.c               |  33 +++++---
 lib/kunit_iov_iter.c                |  70 ++++++++---------
 tools/testing/kunit/kunit_kernel.py |   6 +-
 11 files changed, 261 insertions(+), 91 deletions(-)


base-commit: d206a76d7d2726f3b096037f2079ce0bd3ba329b
-- 
2.44.0

[PATCH v1 1/8] kunit: Run tests when the kernel is fully setup

[Mickaël Salaün]

Run all the KUnit tests just before the first userspace code is
launched.  This makes it it possible to write new tests that check the
kernel in its final state i.e., with all async __init code called,
memory and RCU properly set up, and sysctl boot arguments evaluated.

The initial motivation is to run hardening tests (e.g. memory
protection, Heki's CR-pinning), which require such security protection
to be fully setup (e.g. memory marked as read-only).

Because the suite set could refer to init data, initialize the suite set
with late_initcall(), before kunit_run_all_tests(), if KUnit is built-in
and enabled at boot time.  To make it more consistent and easier to
manage, whatever filters are used or not, always copy test suite entries
and free them after all tests are run.

Because of the prepare_namespace() call, we need to have a valid root
filesystem.  To make it simple, let's use tmpfs with an empty root.
Teach kunit_kernel.py:LinuxSourceTreeOperations*() about the related
kernel boot argument, and add this filesystem to the kunit.py's kernel
build requirements.

Remove __init and __refdata markers from iov_iter, bitfield, checksum,
and the example KUnit tests.  Without this change, the kernel tries to
execute NX-protected pages (because the pages are deallocated).

Tested with:
./tools/testing/kunit/kunit.py run --alltests
./tools/testing/kunit/kunit.py run --alltests --arch x86_64

Cc: Alan Maguire <alan.maguire@oracle.com>
Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Marco Pagani <marpagan@redhat.com>
Cc: Rae Moar <rmoar@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 init/main.c                         |  4 +-
 lib/bitfield_kunit.c                |  8 +--
 lib/checksum_kunit.c                |  2 +-
 lib/kunit/executor.c                | 81 +++++++++++++++++++++--------
 lib/kunit/kunit-example-test.c      |  6 +--
 lib/kunit_iov_iter.c                | 52 +++++++++---------
 tools/testing/kunit/kunit_kernel.py |  6 ++-
 7 files changed, 96 insertions(+), 63 deletions(-)

diff --git a/init/main.c b/init/main.c
index e24b0780fdff..b39d74727aad 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1463,6 +1463,8 @@ static int __ref kernel_init(void *unused)
 
 	do_sysctl_args();
 
+	kunit_run_all_tests();
+
 	if (ramdisk_execute_command) {
 		ret = run_init_process(ramdisk_execute_command);
 		if (!ret)
@@ -1550,8 +1552,6 @@ static noinline void __init kernel_init_freeable(void)
 
 	do_basic_setup();
 
-	kunit_run_all_tests();
-
 	wait_for_initramfs();
 	console_on_rootfs();
 
diff --git a/lib/bitfield_kunit.c b/lib/bitfield_kunit.c
index 1473d8b4bf0f..71e9f2e96496 100644
--- a/lib/bitfield_kunit.c
+++ b/lib/bitfield_kunit.c
@@ -57,7 +57,7 @@
 		CHECK_ENC_GET_BE(tp, v, field, res);			\
 	} while (0)
 
-static void __init test_bitfields_constants(struct kunit *context)
+static void test_bitfields_constants(struct kunit *context)
 {
 	/*
 	 * NOTE
@@ -100,7 +100,7 @@ static void __init test_bitfields_constants(struct kunit *context)
 				tp##_encode_bits(v, mask) != v << __ffs64(mask));\
 	} while (0)
 
-static void __init test_bitfields_variables(struct kunit *context)
+static void test_bitfields_variables(struct kunit *context)
 {
 	CHECK(u8, 0x0f);
 	CHECK(u8, 0xf0);
@@ -126,7 +126,7 @@ static void __init test_bitfields_variables(struct kunit *context)
 }
 
 #ifdef TEST_BITFIELD_COMPILE
-static void __init test_bitfields_compile(struct kunit *context)
+static void test_bitfields_compile(struct kunit *context)
 {
 	/* these should fail compilation */
 	CHECK_ENC_GET(16, 16, 0x0f00, 0x1000);
@@ -137,7 +137,7 @@ static void __init test_bitfields_compile(struct kunit *context)
 }
 #endif
 
-static struct kunit_case __refdata bitfields_test_cases[] = {
+static struct kunit_case bitfields_test_cases[] = {
 	KUNIT_CASE(test_bitfields_constants),
 	KUNIT_CASE(test_bitfields_variables),
 	{}
diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c
index 225bb7701460..41aaed3a4963 100644
--- a/lib/checksum_kunit.c
+++ b/lib/checksum_kunit.c
@@ -620,7 +620,7 @@ static void test_csum_ipv6_magic(struct kunit *test)
 #endif /* !CONFIG_NET */
 }
 
-static struct kunit_case __refdata checksum_test_cases[] = {
+static struct kunit_case checksum_test_cases[] = {
 	KUNIT_CASE(test_csum_fixed_random_inputs),
 	KUNIT_CASE(test_csum_all_carry_inputs),
 	KUNIT_CASE(test_csum_no_carry_inputs),
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 689fff2b2b10..ff3e66ffa739 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -15,6 +15,8 @@ extern struct kunit_suite * const __kunit_suites_end[];
 extern struct kunit_suite * const __kunit_init_suites_start[];
 extern struct kunit_suite * const __kunit_init_suites_end[];
 
+static struct kunit_suite_set final_suite_set = {};
+
 static char *action_param;
 
 module_param_named(action, action_param, charp, 0400);
@@ -233,6 +235,21 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set,
 		if (!filtered_suite)
 			continue;
 
+		if (filtered_suite == suite_set->start[i]) {
+			/*
+			 * To make memory allocation consistent whatever
+			 * filters are used or not, and to keep
+			 * kunit_free_suite_set() simple, always copy static
+			 * data.
+			 */
+			filtered_suite = kmemdup(filtered_suite, sizeof(*filtered_suite),
+					GFP_KERNEL);
+			if (!filtered_suite) {
+				*err = -ENOMEM;
+				goto free_parsed_filters;
+			}
+		}
+
 		*copy++ = filtered_suite;
 	}
 	filtered.start = copy_start;
@@ -348,7 +365,7 @@ static void kunit_handle_shutdown(void)
 
 }
 
-int kunit_run_all_tests(void)
+static int kunit_init_suites(void)
 {
 	struct kunit_suite_set suite_set = {NULL, NULL};
 	struct kunit_suite_set filtered_suite_set = {NULL, NULL};
@@ -361,6 +378,9 @@ int kunit_run_all_tests(void)
 	size_t init_num_suites = init_suite_set.end - init_suite_set.start;
 	int err = 0;
 
+	if (!kunit_enabled())
+		return 0;
+
 	if (init_num_suites > 0) {
 		suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
 		if (!suite_set.start)
@@ -368,41 +388,56 @@ int kunit_run_all_tests(void)
 	} else
 		suite_set = normal_suite_set;
 
-	if (!kunit_enabled()) {
-		pr_info("kunit: disabled\n");
+	filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
+			filter_param, filter_action_param, &err);
+
+	/* Free original suite set before using filtered suite set */
+	if (init_num_suites > 0)
+		kfree(suite_set.start);
+	suite_set = filtered_suite_set;
+
+	if (err) {
+		pr_err("kunit executor: error filtering suites: %d\n", err);
 		goto free_out;
 	}
 
-	if (filter_glob_param || filter_param) {
-		filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
-				filter_param, filter_action_param, &err);
+	final_suite_set = suite_set;
+	return 0;
 
-		/* Free original suite set before using filtered suite set */
-		if (init_num_suites > 0)
-			kfree(suite_set.start);
-		suite_set = filtered_suite_set;
+free_out:
+	kunit_free_suite_set(suite_set);
 
-		if (err) {
-			pr_err("kunit executor: error filtering suites: %d\n", err);
-			goto free_out;
-		}
+out:
+	kunit_handle_shutdown();
+	return err;
+}
+
+late_initcall(kunit_init_suites);
+
+int kunit_run_all_tests(void)
+{
+	int err = 0;
+
+	if (!kunit_enabled()) {
+		pr_info("kunit: disabled\n");
+		goto out;
 	}
 
+	if (!final_suite_set.start)
+		goto out;
+
 	if (!action_param)
-		kunit_exec_run_tests(&suite_set, true);
+		kunit_exec_run_tests(&final_suite_set, true);
 	else if (strcmp(action_param, "list") == 0)
-		kunit_exec_list_tests(&suite_set, false);
+		kunit_exec_list_tests(&final_suite_set, false);
 	else if (strcmp(action_param, "list_attr") == 0)
-		kunit_exec_list_tests(&suite_set, true);
+		kunit_exec_list_tests(&final_suite_set, true);
 	else
 		pr_err("kunit executor: unknown action '%s'\n", action_param);
 
-free_out:
-	if (filter_glob_param || filter_param)
-		kunit_free_suite_set(suite_set);
-	else if (init_num_suites > 0)
-		/* Don't use kunit_free_suite_set because suites aren't individually allocated */
-		kfree(suite_set.start);
+	kunit_free_suite_set(final_suite_set);
+	final_suite_set.start = NULL;
+	final_suite_set.end = NULL;
 
 out:
 	kunit_handle_shutdown();
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index 798924f7cc86..248949eb3b16 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -337,7 +337,7 @@ static struct kunit_suite example_test_suite = {
  */
 kunit_test_suites(&example_test_suite);
 
-static int __init init_add(int x, int y)
+static int init_add(int x, int y)
 {
 	return (x + y);
 }
@@ -345,7 +345,7 @@ static int __init init_add(int x, int y)
 /*
  * This test should always pass. Can be used to test init suites.
  */
-static void __init example_init_test(struct kunit *test)
+static void example_init_test(struct kunit *test)
 {
 	KUNIT_EXPECT_EQ(test, init_add(1, 1), 2);
 }
@@ -354,7 +354,7 @@ static void __init example_init_test(struct kunit *test)
  * The kunit_case struct cannot be marked as __initdata as this will be
  * used in debugfs to retrieve results after test has run
  */
-static struct kunit_case __refdata example_init_test_cases[] = {
+static struct kunit_case example_init_test_cases[] = {
 	KUNIT_CASE(example_init_test),
 	{}
 };
diff --git a/lib/kunit_iov_iter.c b/lib/kunit_iov_iter.c
index 859b67c4d697..a77991a9bffb 100644
--- a/lib/kunit_iov_iter.c
+++ b/lib/kunit_iov_iter.c
@@ -44,9 +44,8 @@ static void iov_kunit_unmap(void *data)
 	vunmap(data);
 }
 
-static void *__init iov_kunit_create_buffer(struct kunit *test,
-					    struct page ***ppages,
-					    size_t npages)
+static void *iov_kunit_create_buffer(struct kunit *test, struct page ***ppages,
+				     size_t npages)
 {
 	struct page **pages;
 	unsigned long got;
@@ -69,11 +68,10 @@ static void *__init iov_kunit_create_buffer(struct kunit *test,
 	return buffer;
 }
 
-static void __init iov_kunit_load_kvec(struct kunit *test,
-				       struct iov_iter *iter, int dir,
-				       struct kvec *kvec, unsigned int kvmax,
-				       void *buffer, size_t bufsize,
-				       const struct kvec_test_range *pr)
+static void iov_kunit_load_kvec(struct kunit *test, struct iov_iter *iter,
+				int dir, struct kvec *kvec, unsigned int kvmax,
+				void *buffer, size_t bufsize,
+				const struct kvec_test_range *pr)
 {
 	size_t size = 0;
 	int i;
@@ -95,7 +93,7 @@ static void __init iov_kunit_load_kvec(struct kunit *test,
 /*
  * Test copying to a ITER_KVEC-type iterator.
  */
-static void __init iov_kunit_copy_to_kvec(struct kunit *test)
+static void iov_kunit_copy_to_kvec(struct kunit *test)
 {
 	const struct kvec_test_range *pr;
 	struct iov_iter iter;
@@ -145,7 +143,7 @@ static void __init iov_kunit_copy_to_kvec(struct kunit *test)
 /*
  * Test copying from a ITER_KVEC-type iterator.
  */
-static void __init iov_kunit_copy_from_kvec(struct kunit *test)
+static void iov_kunit_copy_from_kvec(struct kunit *test)
 {
 	const struct kvec_test_range *pr;
 	struct iov_iter iter;
@@ -213,12 +211,11 @@ static const struct bvec_test_range bvec_test_ranges[] = {
 	{ -1, -1, -1 }
 };
 
-static void __init iov_kunit_load_bvec(struct kunit *test,
-				       struct iov_iter *iter, int dir,
-				       struct bio_vec *bvec, unsigned int bvmax,
-				       struct page **pages, size_t npages,
-				       size_t bufsize,
-				       const struct bvec_test_range *pr)
+static void iov_kunit_load_bvec(struct kunit *test, struct iov_iter *iter,
+				int dir, struct bio_vec *bvec,
+				unsigned int bvmax, struct page **pages,
+				size_t npages, size_t bufsize,
+				const struct bvec_test_range *pr)
 {
 	struct page *can_merge = NULL, *page;
 	size_t size = 0;
@@ -254,7 +251,7 @@ static void __init iov_kunit_load_bvec(struct kunit *test,
 /*
  * Test copying to a ITER_BVEC-type iterator.
  */
-static void __init iov_kunit_copy_to_bvec(struct kunit *test)
+static void iov_kunit_copy_to_bvec(struct kunit *test)
 {
 	const struct bvec_test_range *pr;
 	struct iov_iter iter;
@@ -308,7 +305,7 @@ static void __init iov_kunit_copy_to_bvec(struct kunit *test)
 /*
  * Test copying from a ITER_BVEC-type iterator.
  */
-static void __init iov_kunit_copy_from_bvec(struct kunit *test)
+static void iov_kunit_copy_from_bvec(struct kunit *test)
 {
 	const struct bvec_test_range *pr;
 	struct iov_iter iter;
@@ -370,10 +367,9 @@ static void iov_kunit_destroy_xarray(void *data)
 	kfree(xarray);
 }
 
-static void __init iov_kunit_load_xarray(struct kunit *test,
-					 struct iov_iter *iter, int dir,
-					 struct xarray *xarray,
-					 struct page **pages, size_t npages)
+static void iov_kunit_load_xarray(struct kunit *test, struct iov_iter *iter,
+				  int dir, struct xarray *xarray,
+				  struct page **pages, size_t npages)
 {
 	size_t size = 0;
 	int i;
@@ -401,7 +397,7 @@ static struct xarray *iov_kunit_create_xarray(struct kunit *test)
 /*
  * Test copying to a ITER_XARRAY-type iterator.
  */
-static void __init iov_kunit_copy_to_xarray(struct kunit *test)
+static void iov_kunit_copy_to_xarray(struct kunit *test)
 {
 	const struct kvec_test_range *pr;
 	struct iov_iter iter;
@@ -459,7 +455,7 @@ static void __init iov_kunit_copy_to_xarray(struct kunit *test)
 /*
  * Test copying from a ITER_XARRAY-type iterator.
  */
-static void __init iov_kunit_copy_from_xarray(struct kunit *test)
+static void iov_kunit_copy_from_xarray(struct kunit *test)
 {
 	const struct kvec_test_range *pr;
 	struct iov_iter iter;
@@ -522,7 +518,7 @@ static void __init iov_kunit_copy_from_xarray(struct kunit *test)
 /*
  * Test the extraction of ITER_KVEC-type iterators.
  */
-static void __init iov_kunit_extract_pages_kvec(struct kunit *test)
+static void iov_kunit_extract_pages_kvec(struct kunit *test)
 {
 	const struct kvec_test_range *pr;
 	struct iov_iter iter;
@@ -602,7 +598,7 @@ static void __init iov_kunit_extract_pages_kvec(struct kunit *test)
 /*
  * Test the extraction of ITER_BVEC-type iterators.
  */
-static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
+static void iov_kunit_extract_pages_bvec(struct kunit *test)
 {
 	const struct bvec_test_range *pr;
 	struct iov_iter iter;
@@ -680,7 +676,7 @@ static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
 /*
  * Test the extraction of ITER_XARRAY-type iterators.
  */
-static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
+static void iov_kunit_extract_pages_xarray(struct kunit *test)
 {
 	const struct kvec_test_range *pr;
 	struct iov_iter iter;
@@ -756,7 +752,7 @@ static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
 	KUNIT_SUCCEED();
 }
 
-static struct kunit_case __refdata iov_kunit_cases[] = {
+static struct kunit_case iov_kunit_cases[] = {
 	KUNIT_CASE(iov_kunit_copy_to_kvec),
 	KUNIT_CASE(iov_kunit_copy_from_kvec),
 	KUNIT_CASE(iov_kunit_copy_to_bvec),
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 0b6488efed47..e1980ea58118 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -104,12 +104,13 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
 		self._kconfig = qemu_arch_params.kconfig
 		self._qemu_arch = qemu_arch_params.qemu_arch
 		self._kernel_path = qemu_arch_params.kernel_path
-		self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
+		self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot rootfstype=tmpfs'
 		self._extra_qemu_params = qemu_arch_params.extra_qemu_params
 		self._serial = qemu_arch_params.serial
 
 	def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
 		kconfig = kunit_config.parse_from_string(self._kconfig)
+		kconfig.add_entry('TMPFS', 'y')
 		kconfig.merge_in_entries(base_kunitconfig)
 		return kconfig
 
@@ -139,13 +140,14 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
 
 	def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
 		kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
+		kconfig.add_entry('TMPFS', 'y')
 		kconfig.merge_in_entries(base_kunitconfig)
 		return kconfig
 
 	def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
 		"""Runs the Linux UML binary. Must be named 'linux'."""
 		linux_bin = os.path.join(build_dir, 'linux')
-		params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt'])
+		params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt', 'rootfstype=tmpfs'])
 		return subprocess.Popen([linux_bin] + params,
 					   stdin=subprocess.PIPE,
 					   stdout=subprocess.PIPE,
-- 
2.44.0

Re: [PATCH v1 1/8] kunit: Run tests when the kernel is fully setup

[Kees Cook]

On Thu, Feb 29, 2024 at 06:04:02PM +0100, Mickaël Salaün wrote:
> Run all the KUnit tests just before the first userspace code is
> launched.  This makes it it possible to write new tests that check the
> kernel in its final state i.e., with all async __init code called,
> memory and RCU properly set up, and sysctl boot arguments evaluated.
> 
> The initial motivation is to run hardening tests (e.g. memory
> protection, Heki's CR-pinning), which require such security protection
> to be fully setup (e.g. memory marked as read-only).
> 
> Because the suite set could refer to init data, initialize the suite set
> with late_initcall(), before kunit_run_all_tests(), if KUnit is built-in
> and enabled at boot time.  To make it more consistent and easier to
> manage, whatever filters are used or not, always copy test suite entries
> and free them after all tests are run.
> 
> Because of the prepare_namespace() call, we need to have a valid root
> filesystem.  To make it simple, let's use tmpfs with an empty root.
> Teach kunit_kernel.py:LinuxSourceTreeOperations*() about the related
> kernel boot argument, and add this filesystem to the kunit.py's kernel
> build requirements.
> 
> Remove __init and __refdata markers from iov_iter, bitfield, checksum,
> and the example KUnit tests.  Without this change, the kernel tries to
> execute NX-protected pages (because the pages are deallocated).

I defer to the KUnit maintainers, but I'd find this patch easier to
review if the __init and __refdata changes were a separate patch.

But otherwise, I like the goals of this series! It should make adding a
userspace memory region for porting the usercopy tests easier too.

-Kees

> 
> Tested with:
> ./tools/testing/kunit/kunit.py run --alltests
> ./tools/testing/kunit/kunit.py run --alltests --arch x86_64
> 
> Cc: Alan Maguire <alan.maguire@oracle.com>
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Marco Pagani <marpagan@redhat.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  init/main.c                         |  4 +-
>  lib/bitfield_kunit.c                |  8 +--
>  lib/checksum_kunit.c                |  2 +-
>  lib/kunit/executor.c                | 81 +++++++++++++++++++++--------
>  lib/kunit/kunit-example-test.c      |  6 +--
>  lib/kunit_iov_iter.c                | 52 +++++++++---------
>  tools/testing/kunit/kunit_kernel.py |  6 ++-
>  7 files changed, 96 insertions(+), 63 deletions(-)
> 
> diff --git a/init/main.c b/init/main.c
> index e24b0780fdff..b39d74727aad 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1463,6 +1463,8 @@ static int __ref kernel_init(void *unused)
>  
>  	do_sysctl_args();
>  
> +	kunit_run_all_tests();
> +
>  	if (ramdisk_execute_command) {
>  		ret = run_init_process(ramdisk_execute_command);
>  		if (!ret)
> @@ -1550,8 +1552,6 @@ static noinline void __init kernel_init_freeable(void)
>  
>  	do_basic_setup();
>  
> -	kunit_run_all_tests();
> -
>  	wait_for_initramfs();
>  	console_on_rootfs();
>  
> diff --git a/lib/bitfield_kunit.c b/lib/bitfield_kunit.c
> index 1473d8b4bf0f..71e9f2e96496 100644
> --- a/lib/bitfield_kunit.c
> +++ b/lib/bitfield_kunit.c
> @@ -57,7 +57,7 @@
>  		CHECK_ENC_GET_BE(tp, v, field, res);			\
>  	} while (0)
>  
> -static void __init test_bitfields_constants(struct kunit *context)
> +static void test_bitfields_constants(struct kunit *context)
>  {
>  	/*
>  	 * NOTE
> @@ -100,7 +100,7 @@ static void __init test_bitfields_constants(struct kunit *context)
>  				tp##_encode_bits(v, mask) != v << __ffs64(mask));\
>  	} while (0)
>  
> -static void __init test_bitfields_variables(struct kunit *context)
> +static void test_bitfields_variables(struct kunit *context)
>  {
>  	CHECK(u8, 0x0f);
>  	CHECK(u8, 0xf0);
> @@ -126,7 +126,7 @@ static void __init test_bitfields_variables(struct kunit *context)
>  }
>  
>  #ifdef TEST_BITFIELD_COMPILE
> -static void __init test_bitfields_compile(struct kunit *context)
> +static void test_bitfields_compile(struct kunit *context)
>  {
>  	/* these should fail compilation */
>  	CHECK_ENC_GET(16, 16, 0x0f00, 0x1000);
> @@ -137,7 +137,7 @@ static void __init test_bitfields_compile(struct kunit *context)
>  }
>  #endif
>  
> -static struct kunit_case __refdata bitfields_test_cases[] = {
> +static struct kunit_case bitfields_test_cases[] = {
>  	KUNIT_CASE(test_bitfields_constants),
>  	KUNIT_CASE(test_bitfields_variables),
>  	{}
> diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c
> index 225bb7701460..41aaed3a4963 100644
> --- a/lib/checksum_kunit.c
> +++ b/lib/checksum_kunit.c
> @@ -620,7 +620,7 @@ static void test_csum_ipv6_magic(struct kunit *test)
>  #endif /* !CONFIG_NET */
>  }
>  
> -static struct kunit_case __refdata checksum_test_cases[] = {
> +static struct kunit_case checksum_test_cases[] = {
>  	KUNIT_CASE(test_csum_fixed_random_inputs),
>  	KUNIT_CASE(test_csum_all_carry_inputs),
>  	KUNIT_CASE(test_csum_no_carry_inputs),
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 689fff2b2b10..ff3e66ffa739 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -15,6 +15,8 @@ extern struct kunit_suite * const __kunit_suites_end[];
>  extern struct kunit_suite * const __kunit_init_suites_start[];
>  extern struct kunit_suite * const __kunit_init_suites_end[];
>  
> +static struct kunit_suite_set final_suite_set = {};
> +
>  static char *action_param;
>  
>  module_param_named(action, action_param, charp, 0400);
> @@ -233,6 +235,21 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set,
>  		if (!filtered_suite)
>  			continue;
>  
> +		if (filtered_suite == suite_set->start[i]) {
> +			/*
> +			 * To make memory allocation consistent whatever
> +			 * filters are used or not, and to keep
> +			 * kunit_free_suite_set() simple, always copy static
> +			 * data.
> +			 */
> +			filtered_suite = kmemdup(filtered_suite, sizeof(*filtered_suite),
> +					GFP_KERNEL);
> +			if (!filtered_suite) {
> +				*err = -ENOMEM;
> +				goto free_parsed_filters;
> +			}
> +		}
> +
>  		*copy++ = filtered_suite;
>  	}
>  	filtered.start = copy_start;
> @@ -348,7 +365,7 @@ static void kunit_handle_shutdown(void)
>  
>  }
>  
> -int kunit_run_all_tests(void)
> +static int kunit_init_suites(void)
>  {
>  	struct kunit_suite_set suite_set = {NULL, NULL};
>  	struct kunit_suite_set filtered_suite_set = {NULL, NULL};
> @@ -361,6 +378,9 @@ int kunit_run_all_tests(void)
>  	size_t init_num_suites = init_suite_set.end - init_suite_set.start;
>  	int err = 0;
>  
> +	if (!kunit_enabled())
> +		return 0;
> +
>  	if (init_num_suites > 0) {
>  		suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
>  		if (!suite_set.start)
> @@ -368,41 +388,56 @@ int kunit_run_all_tests(void)
>  	} else
>  		suite_set = normal_suite_set;
>  
> -	if (!kunit_enabled()) {
> -		pr_info("kunit: disabled\n");
> +	filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> +			filter_param, filter_action_param, &err);
> +
> +	/* Free original suite set before using filtered suite set */
> +	if (init_num_suites > 0)
> +		kfree(suite_set.start);
> +	suite_set = filtered_suite_set;
> +
> +	if (err) {
> +		pr_err("kunit executor: error filtering suites: %d\n", err);
>  		goto free_out;
>  	}
>  
> -	if (filter_glob_param || filter_param) {
> -		filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> -				filter_param, filter_action_param, &err);
> +	final_suite_set = suite_set;
> +	return 0;
>  
> -		/* Free original suite set before using filtered suite set */
> -		if (init_num_suites > 0)
> -			kfree(suite_set.start);
> -		suite_set = filtered_suite_set;
> +free_out:
> +	kunit_free_suite_set(suite_set);
>  
> -		if (err) {
> -			pr_err("kunit executor: error filtering suites: %d\n", err);
> -			goto free_out;
> -		}
> +out:
> +	kunit_handle_shutdown();
> +	return err;
> +}
> +
> +late_initcall(kunit_init_suites);
> +
> +int kunit_run_all_tests(void)
> +{
> +	int err = 0;
> +
> +	if (!kunit_enabled()) {
> +		pr_info("kunit: disabled\n");
> +		goto out;
>  	}
>  
> +	if (!final_suite_set.start)
> +		goto out;
> +
>  	if (!action_param)
> -		kunit_exec_run_tests(&suite_set, true);
> +		kunit_exec_run_tests(&final_suite_set, true);
>  	else if (strcmp(action_param, "list") == 0)
> -		kunit_exec_list_tests(&suite_set, false);
> +		kunit_exec_list_tests(&final_suite_set, false);
>  	else if (strcmp(action_param, "list_attr") == 0)
> -		kunit_exec_list_tests(&suite_set, true);
> +		kunit_exec_list_tests(&final_suite_set, true);
>  	else
>  		pr_err("kunit executor: unknown action '%s'\n", action_param);
>  
> -free_out:
> -	if (filter_glob_param || filter_param)
> -		kunit_free_suite_set(suite_set);
> -	else if (init_num_suites > 0)
> -		/* Don't use kunit_free_suite_set because suites aren't individually allocated */
> -		kfree(suite_set.start);
> +	kunit_free_suite_set(final_suite_set);
> +	final_suite_set.start = NULL;
> +	final_suite_set.end = NULL;
>  
>  out:
>  	kunit_handle_shutdown();
> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> index 798924f7cc86..248949eb3b16 100644
> --- a/lib/kunit/kunit-example-test.c
> +++ b/lib/kunit/kunit-example-test.c
> @@ -337,7 +337,7 @@ static struct kunit_suite example_test_suite = {
>   */
>  kunit_test_suites(&example_test_suite);
>  
> -static int __init init_add(int x, int y)
> +static int init_add(int x, int y)
>  {
>  	return (x + y);
>  }
> @@ -345,7 +345,7 @@ static int __init init_add(int x, int y)
>  /*
>   * This test should always pass. Can be used to test init suites.
>   */
> -static void __init example_init_test(struct kunit *test)
> +static void example_init_test(struct kunit *test)
>  {
>  	KUNIT_EXPECT_EQ(test, init_add(1, 1), 2);
>  }
> @@ -354,7 +354,7 @@ static void __init example_init_test(struct kunit *test)
>   * The kunit_case struct cannot be marked as __initdata as this will be
>   * used in debugfs to retrieve results after test has run
>   */
> -static struct kunit_case __refdata example_init_test_cases[] = {
> +static struct kunit_case example_init_test_cases[] = {
>  	KUNIT_CASE(example_init_test),
>  	{}
>  };
> diff --git a/lib/kunit_iov_iter.c b/lib/kunit_iov_iter.c
> index 859b67c4d697..a77991a9bffb 100644
> --- a/lib/kunit_iov_iter.c
> +++ b/lib/kunit_iov_iter.c
> @@ -44,9 +44,8 @@ static void iov_kunit_unmap(void *data)
>  	vunmap(data);
>  }
>  
> -static void *__init iov_kunit_create_buffer(struct kunit *test,
> -					    struct page ***ppages,
> -					    size_t npages)
> +static void *iov_kunit_create_buffer(struct kunit *test, struct page ***ppages,
> +				     size_t npages)
>  {
>  	struct page **pages;
>  	unsigned long got;
> @@ -69,11 +68,10 @@ static void *__init iov_kunit_create_buffer(struct kunit *test,
>  	return buffer;
>  }
>  
> -static void __init iov_kunit_load_kvec(struct kunit *test,
> -				       struct iov_iter *iter, int dir,
> -				       struct kvec *kvec, unsigned int kvmax,
> -				       void *buffer, size_t bufsize,
> -				       const struct kvec_test_range *pr)
> +static void iov_kunit_load_kvec(struct kunit *test, struct iov_iter *iter,
> +				int dir, struct kvec *kvec, unsigned int kvmax,
> +				void *buffer, size_t bufsize,
> +				const struct kvec_test_range *pr)
>  {
>  	size_t size = 0;
>  	int i;
> @@ -95,7 +93,7 @@ static void __init iov_kunit_load_kvec(struct kunit *test,
>  /*
>   * Test copying to a ITER_KVEC-type iterator.
>   */
> -static void __init iov_kunit_copy_to_kvec(struct kunit *test)
> +static void iov_kunit_copy_to_kvec(struct kunit *test)
>  {
>  	const struct kvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -145,7 +143,7 @@ static void __init iov_kunit_copy_to_kvec(struct kunit *test)
>  /*
>   * Test copying from a ITER_KVEC-type iterator.
>   */
> -static void __init iov_kunit_copy_from_kvec(struct kunit *test)
> +static void iov_kunit_copy_from_kvec(struct kunit *test)
>  {
>  	const struct kvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -213,12 +211,11 @@ static const struct bvec_test_range bvec_test_ranges[] = {
>  	{ -1, -1, -1 }
>  };
>  
> -static void __init iov_kunit_load_bvec(struct kunit *test,
> -				       struct iov_iter *iter, int dir,
> -				       struct bio_vec *bvec, unsigned int bvmax,
> -				       struct page **pages, size_t npages,
> -				       size_t bufsize,
> -				       const struct bvec_test_range *pr)
> +static void iov_kunit_load_bvec(struct kunit *test, struct iov_iter *iter,
> +				int dir, struct bio_vec *bvec,
> +				unsigned int bvmax, struct page **pages,
> +				size_t npages, size_t bufsize,
> +				const struct bvec_test_range *pr)
>  {
>  	struct page *can_merge = NULL, *page;
>  	size_t size = 0;
> @@ -254,7 +251,7 @@ static void __init iov_kunit_load_bvec(struct kunit *test,
>  /*
>   * Test copying to a ITER_BVEC-type iterator.
>   */
> -static void __init iov_kunit_copy_to_bvec(struct kunit *test)
> +static void iov_kunit_copy_to_bvec(struct kunit *test)
>  {
>  	const struct bvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -308,7 +305,7 @@ static void __init iov_kunit_copy_to_bvec(struct kunit *test)
>  /*
>   * Test copying from a ITER_BVEC-type iterator.
>   */
> -static void __init iov_kunit_copy_from_bvec(struct kunit *test)
> +static void iov_kunit_copy_from_bvec(struct kunit *test)
>  {
>  	const struct bvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -370,10 +367,9 @@ static void iov_kunit_destroy_xarray(void *data)
>  	kfree(xarray);
>  }
>  
> -static void __init iov_kunit_load_xarray(struct kunit *test,
> -					 struct iov_iter *iter, int dir,
> -					 struct xarray *xarray,
> -					 struct page **pages, size_t npages)
> +static void iov_kunit_load_xarray(struct kunit *test, struct iov_iter *iter,
> +				  int dir, struct xarray *xarray,
> +				  struct page **pages, size_t npages)
>  {
>  	size_t size = 0;
>  	int i;
> @@ -401,7 +397,7 @@ static struct xarray *iov_kunit_create_xarray(struct kunit *test)
>  /*
>   * Test copying to a ITER_XARRAY-type iterator.
>   */
> -static void __init iov_kunit_copy_to_xarray(struct kunit *test)
> +static void iov_kunit_copy_to_xarray(struct kunit *test)
>  {
>  	const struct kvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -459,7 +455,7 @@ static void __init iov_kunit_copy_to_xarray(struct kunit *test)
>  /*
>   * Test copying from a ITER_XARRAY-type iterator.
>   */
> -static void __init iov_kunit_copy_from_xarray(struct kunit *test)
> +static void iov_kunit_copy_from_xarray(struct kunit *test)
>  {
>  	const struct kvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -522,7 +518,7 @@ static void __init iov_kunit_copy_from_xarray(struct kunit *test)
>  /*
>   * Test the extraction of ITER_KVEC-type iterators.
>   */
> -static void __init iov_kunit_extract_pages_kvec(struct kunit *test)
> +static void iov_kunit_extract_pages_kvec(struct kunit *test)
>  {
>  	const struct kvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -602,7 +598,7 @@ static void __init iov_kunit_extract_pages_kvec(struct kunit *test)
>  /*
>   * Test the extraction of ITER_BVEC-type iterators.
>   */
> -static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
> +static void iov_kunit_extract_pages_bvec(struct kunit *test)
>  {
>  	const struct bvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -680,7 +676,7 @@ static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
>  /*
>   * Test the extraction of ITER_XARRAY-type iterators.
>   */
> -static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
> +static void iov_kunit_extract_pages_xarray(struct kunit *test)
>  {
>  	const struct kvec_test_range *pr;
>  	struct iov_iter iter;
> @@ -756,7 +752,7 @@ static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
>  	KUNIT_SUCCEED();
>  }
>  
> -static struct kunit_case __refdata iov_kunit_cases[] = {
> +static struct kunit_case iov_kunit_cases[] = {
>  	KUNIT_CASE(iov_kunit_copy_to_kvec),
>  	KUNIT_CASE(iov_kunit_copy_from_kvec),
>  	KUNIT_CASE(iov_kunit_copy_to_bvec),
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 0b6488efed47..e1980ea58118 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -104,12 +104,13 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
>  		self._kconfig = qemu_arch_params.kconfig
>  		self._qemu_arch = qemu_arch_params.qemu_arch
>  		self._kernel_path = qemu_arch_params.kernel_path
> -		self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
> +		self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot rootfstype=tmpfs'
>  		self._extra_qemu_params = qemu_arch_params.extra_qemu_params
>  		self._serial = qemu_arch_params.serial
>  
>  	def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
>  		kconfig = kunit_config.parse_from_string(self._kconfig)
> +		kconfig.add_entry('TMPFS', 'y')
>  		kconfig.merge_in_entries(base_kunitconfig)
>  		return kconfig
>  
> @@ -139,13 +140,14 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
>  
>  	def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
>  		kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
> +		kconfig.add_entry('TMPFS', 'y')
>  		kconfig.merge_in_entries(base_kunitconfig)
>  		return kconfig
>  
>  	def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
>  		"""Runs the Linux UML binary. Must be named 'linux'."""
>  		linux_bin = os.path.join(build_dir, 'linux')
> -		params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt'])
> +		params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt', 'rootfstype=tmpfs'])
>  		return subprocess.Popen([linux_bin] + params,
>  					   stdin=subprocess.PIPE,
>  					   stdout=subprocess.PIPE,
> -- 
> 2.44.0
> 

-- 
Kees Cook

Re: [PATCH v1 1/8] kunit: Run tests when the kernel is fully setup

[kernel test robot]

Hi Mickaël,

kernel test robot noticed the following build warnings:

[auto build test WARNING on d206a76d7d2726f3b096037f2079ce0bd3ba329b]

url:    https://github.com/intel-lab-lkp/linux/commits/Micka-l-Sala-n/kunit-Run-tests-when-the-kernel-is-fully-setup/20240301-011020
base:   d206a76d7d2726f3b096037f2079ce0bd3ba329b
patch link:    https://lore.kernel.org/r/20240229170409.365386-2-mic%40digikod.net
patch subject: [PATCH v1 1/8] kunit: Run tests when the kernel is fully setup
config: arc-allmodconfig (https://download.01.org/0day-ci/archive/20240301/202403011330.telj6BqQ-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240301/202403011330.telj6BqQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403011330.telj6BqQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> lib/kunit/executor.c:18:31: warning: 'final_suite_set' defined but not used [-Wunused-variable]
      18 | static struct kunit_suite_set final_suite_set = {};
         |                               ^~~~~~~~~~~~~~~


vim +/final_suite_set +18 lib/kunit/executor.c

    17	
  > 18	static struct kunit_suite_set final_suite_set = {};
    19	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 1/8] kunit: Run tests when the kernel is fully setup

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 22761 bytes --]

On Fri, 1 Mar 2024 at 01:04, Mickaël Salaün <mic@digikod.net> wrote:
 >
> Run all the KUnit tests just before the first userspace code is
> launched.  This makes it it possible to write new tests that check the
> kernel in its final state i.e., with all async __init code called,
> memory and RCU properly set up, and sysctl boot arguments evaluated.

KUnit has explicit support for running tests which call __init code
(or are themselves marked __init), so I'm not keen to move _all_ tests
here.

That being said, I'm okay with running all of the other tests (ones
not explicitly marked init) after __init.
>
> The initial motivation is to run hardening tests (e.g. memory
> protection, Heki's CR-pinning), which require such security protection
> to be fully setup (e.g. memory marked as read-only).
>
> Because the suite set could refer to init data, initialize the suite set
> with late_initcall(), before kunit_run_all_tests(), if KUnit is built-in
> and enabled at boot time.  To make it more consistent and easier to
> manage, whatever filters are used or not, always copy test suite entries
> and free them after all tests are run.

The goal is to allow init data only for suites explicitly marked as
such, so we should be able to split that along these lines.

And yeah, the filter code is pretty convoluted when it comes to when
they're allocated, and it does do things like check how the suite set
was allocated. So we do need to make any changes carefully.

As Kees suggested, I'd like to see any cleanup here as a separate patch.

>
> Because of the prepare_namespace() call, we need to have a valid root
> filesystem.  To make it simple, let's use tmpfs with an empty root.
> Teach kunit_kernel.py:LinuxSourceTreeOperations*() about the related
> kernel boot argument, and add this filesystem to the kunit.py's kernel
> build requirements.

I think this is a big enough change we'll need to handle it very
carefully: there are a few places where the fact that KUnit doesn't
require a root filesystem is documented, and possibly some other
(non-kunit.py) runners which rely on this.

I don't think that's necessarily a blocker, but we'll definitely want
to document it well, and make sure users are aware before this lands.

>
> Remove __init and __refdata markers from iov_iter, bitfield, checksum,
> and the example KUnit tests.  Without this change, the kernel tries to
> execute NX-protected pages (because the pages are deallocated).

We still want to support these, but we should make sure the suites are
declared with kunit_init_test_suite().

>
> Tested with:
> ./tools/testing/kunit/kunit.py run --alltests
> ./tools/testing/kunit/kunit.py run --alltests --arch x86_64

FYI: This breaks arm64:
./tools/testing/kunit/kunit.py run --raw_output --arch arm64
--cross_compile=aarch64-linux-gnu-

Unable to handle kernel paging request at virtual address ffffffff02016f88
...
Call trace:
 kfree+0x30/0x184
 kunit_run_all_tests+0x88/0x154
 kernel_init+0x6c/0x1e0
 ret_from_fork+0x10/0x20
Code: b25f7be1 aa0003f4 d34cfe73 8b131833 (f9400661)
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b


>
> Cc: Alan Maguire <alan.maguire@oracle.com>
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Marco Pagani <marpagan@redhat.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  init/main.c                         |  4 +-
>  lib/bitfield_kunit.c                |  8 +--
>  lib/checksum_kunit.c                |  2 +-
>  lib/kunit/executor.c                | 81 +++++++++++++++++++++--------
>  lib/kunit/kunit-example-test.c      |  6 +--
>  lib/kunit_iov_iter.c                | 52 +++++++++---------
>  tools/testing/kunit/kunit_kernel.py |  6 ++-
>  7 files changed, 96 insertions(+), 63 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index e24b0780fdff..b39d74727aad 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1463,6 +1463,8 @@ static int __ref kernel_init(void *unused)
>
>         do_sysctl_args();
>
> +       kunit_run_all_tests();
> +
>         if (ramdisk_execute_command) {
>                 ret = run_init_process(ramdisk_execute_command);
>                 if (!ret)
> @@ -1550,8 +1552,6 @@ static noinline void __init kernel_init_freeable(void)
>
>         do_basic_setup();
>
> -       kunit_run_all_tests();
> -
>         wait_for_initramfs();
>         console_on_rootfs();
>
> diff --git a/lib/bitfield_kunit.c b/lib/bitfield_kunit.c
> index 1473d8b4bf0f..71e9f2e96496 100644
> --- a/lib/bitfield_kunit.c
> +++ b/lib/bitfield_kunit.c
> @@ -57,7 +57,7 @@
>                 CHECK_ENC_GET_BE(tp, v, field, res);                    \
>         } while (0)
>
> -static void __init test_bitfields_constants(struct kunit *context)
> +static void test_bitfields_constants(struct kunit *context)
>  {
>         /*
>          * NOTE
> @@ -100,7 +100,7 @@ static void __init test_bitfields_constants(struct kunit *context)
>                                 tp##_encode_bits(v, mask) != v << __ffs64(mask));\
>         } while (0)
>
> -static void __init test_bitfields_variables(struct kunit *context)
> +static void test_bitfields_variables(struct kunit *context)
>  {
>         CHECK(u8, 0x0f);
>         CHECK(u8, 0xf0);
> @@ -126,7 +126,7 @@ static void __init test_bitfields_variables(struct kunit *context)
>  }
>
>  #ifdef TEST_BITFIELD_COMPILE
> -static void __init test_bitfields_compile(struct kunit *context)
> +static void test_bitfields_compile(struct kunit *context)
>  {
>         /* these should fail compilation */
>         CHECK_ENC_GET(16, 16, 0x0f00, 0x1000);
> @@ -137,7 +137,7 @@ static void __init test_bitfields_compile(struct kunit *context)
>  }
>  #endif
>
> -static struct kunit_case __refdata bitfields_test_cases[] = {
> +static struct kunit_case bitfields_test_cases[] = {
>         KUNIT_CASE(test_bitfields_constants),
>         KUNIT_CASE(test_bitfields_variables),
>         {}
> diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c
> index 225bb7701460..41aaed3a4963 100644
> --- a/lib/checksum_kunit.c
> +++ b/lib/checksum_kunit.c
> @@ -620,7 +620,7 @@ static void test_csum_ipv6_magic(struct kunit *test)
>  #endif /* !CONFIG_NET */
>  }
>
> -static struct kunit_case __refdata checksum_test_cases[] = {
> +static struct kunit_case checksum_test_cases[] = {
>         KUNIT_CASE(test_csum_fixed_random_inputs),
>         KUNIT_CASE(test_csum_all_carry_inputs),
>         KUNIT_CASE(test_csum_no_carry_inputs),
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 689fff2b2b10..ff3e66ffa739 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -15,6 +15,8 @@ extern struct kunit_suite * const __kunit_suites_end[];
>  extern struct kunit_suite * const __kunit_init_suites_start[];
>  extern struct kunit_suite * const __kunit_init_suites_end[];
>
> +static struct kunit_suite_set final_suite_set = {};
> +
>  static char *action_param;
>
>  module_param_named(action, action_param, charp, 0400);
> @@ -233,6 +235,21 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set,
>                 if (!filtered_suite)
>                         continue;
>
> +               if (filtered_suite == suite_set->start[i]) {
> +                       /*
> +                        * To make memory allocation consistent whatever
> +                        * filters are used or not, and to keep
> +                        * kunit_free_suite_set() simple, always copy static
> +                        * data.
> +                        */
> +                       filtered_suite = kmemdup(filtered_suite, sizeof(*filtered_suite),
> +                                       GFP_KERNEL);
> +                       if (!filtered_suite) {
> +                               *err = -ENOMEM;
> +                               goto free_parsed_filters;
> +                       }
> +               }
> +
>                 *copy++ = filtered_suite;
>         }
>         filtered.start = copy_start;
> @@ -348,7 +365,7 @@ static void kunit_handle_shutdown(void)
>
>  }
>
> -int kunit_run_all_tests(void)
> +static int kunit_init_suites(void)
>  {
>         struct kunit_suite_set suite_set = {NULL, NULL};
>         struct kunit_suite_set filtered_suite_set = {NULL, NULL};
> @@ -361,6 +378,9 @@ int kunit_run_all_tests(void)
>         size_t init_num_suites = init_suite_set.end - init_suite_set.start;
>         int err = 0;
>
> +       if (!kunit_enabled())
> +               return 0;
> +
>         if (init_num_suites > 0) {
>                 suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
>                 if (!suite_set.start)
> @@ -368,41 +388,56 @@ int kunit_run_all_tests(void)
>         } else
>                 suite_set = normal_suite_set;
>
> -       if (!kunit_enabled()) {
> -               pr_info("kunit: disabled\n");
> +       filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> +                       filter_param, filter_action_param, &err);
> +
> +       /* Free original suite set before using filtered suite set */
> +       if (init_num_suites > 0)
> +               kfree(suite_set.start);
> +       suite_set = filtered_suite_set;
> +
> +       if (err) {
> +               pr_err("kunit executor: error filtering suites: %d\n", err);
>                 goto free_out;
>         }
>
> -       if (filter_glob_param || filter_param) {
> -               filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> -                               filter_param, filter_action_param, &err);
> +       final_suite_set = suite_set;
> +       return 0;
>
> -               /* Free original suite set before using filtered suite set */
> -               if (init_num_suites > 0)
> -                       kfree(suite_set.start);
> -               suite_set = filtered_suite_set;
> +free_out:
> +       kunit_free_suite_set(suite_set);
>
> -               if (err) {
> -                       pr_err("kunit executor: error filtering suites: %d\n", err);
> -                       goto free_out;
> -               }
> +out:
> +       kunit_handle_shutdown();
> +       return err;
> +}
> +
> +late_initcall(kunit_init_suites);
> +
> +int kunit_run_all_tests(void)
> +{
> +       int err = 0;
> +
> +       if (!kunit_enabled()) {
> +               pr_info("kunit: disabled\n");
> +               goto out;
>         }
>
> +       if (!final_suite_set.start)
> +               goto out;
> +
>         if (!action_param)
> -               kunit_exec_run_tests(&suite_set, true);
> +               kunit_exec_run_tests(&final_suite_set, true);
>         else if (strcmp(action_param, "list") == 0)
> -               kunit_exec_list_tests(&suite_set, false);
> +               kunit_exec_list_tests(&final_suite_set, false);
>         else if (strcmp(action_param, "list_attr") == 0)
> -               kunit_exec_list_tests(&suite_set, true);
> +               kunit_exec_list_tests(&final_suite_set, true);
>         else
>                 pr_err("kunit executor: unknown action '%s'\n", action_param);
>
> -free_out:
> -       if (filter_glob_param || filter_param)
> -               kunit_free_suite_set(suite_set);
> -       else if (init_num_suites > 0)
> -               /* Don't use kunit_free_suite_set because suites aren't individually allocated */
> -               kfree(suite_set.start);
> +       kunit_free_suite_set(final_suite_set);
> +       final_suite_set.start = NULL;
> +       final_suite_set.end = NULL;
>
>  out:
>         kunit_handle_shutdown();
> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> index 798924f7cc86..248949eb3b16 100644
> --- a/lib/kunit/kunit-example-test.c
> +++ b/lib/kunit/kunit-example-test.c
> @@ -337,7 +337,7 @@ static struct kunit_suite example_test_suite = {
>   */
>  kunit_test_suites(&example_test_suite);
>
> -static int __init init_add(int x, int y)
> +static int init_add(int x, int y)

The whole point of this function (and the example_init_test) is that
they're marked __init, as proof we can test code which is in the init
section.

So, either we leave this alone, or we remove it entirely. There's no
point just removing __init.

>  {
>         return (x + y);
>  }
> @@ -345,7 +345,7 @@ static int __init init_add(int x, int y)
>  /*
>   * This test should always pass. Can be used to test init suites.
>   */
> -static void __init example_init_test(struct kunit *test)
> +static void example_init_test(struct kunit *test)

As above.

>  {
>         KUNIT_EXPECT_EQ(test, init_add(1, 1), 2);
>  }
> @@ -354,7 +354,7 @@ static void __init example_init_test(struct kunit *test)
>   * The kunit_case struct cannot be marked as __initdata as this will be
>   * used in debugfs to retrieve results after test has run
>   */
> -static struct kunit_case __refdata example_init_test_cases[] = {
> +static struct kunit_case example_init_test_cases[] = {
>         KUNIT_CASE(example_init_test),
>         {}
>  };

As above.

> diff --git a/lib/kunit_iov_iter.c b/lib/kunit_iov_iter.c
> index 859b67c4d697..a77991a9bffb 100644
> --- a/lib/kunit_iov_iter.c
> +++ b/lib/kunit_iov_iter.c
> @@ -44,9 +44,8 @@ static void iov_kunit_unmap(void *data)
>         vunmap(data);
>  }
>
> -static void *__init iov_kunit_create_buffer(struct kunit *test,
> -                                           struct page ***ppages,
> -                                           size_t npages)
> +static void *iov_kunit_create_buffer(struct kunit *test, struct page ***ppages,
> +                                    size_t npages)
>  {
>         struct page **pages;
>         unsigned long got;
> @@ -69,11 +68,10 @@ static void *__init iov_kunit_create_buffer(struct kunit *test,
>         return buffer;
>  }
>
> -static void __init iov_kunit_load_kvec(struct kunit *test,
> -                                      struct iov_iter *iter, int dir,
> -                                      struct kvec *kvec, unsigned int kvmax,
> -                                      void *buffer, size_t bufsize,
> -                                      const struct kvec_test_range *pr)
> +static void iov_kunit_load_kvec(struct kunit *test, struct iov_iter *iter,
> +                               int dir, struct kvec *kvec, unsigned int kvmax,
> +                               void *buffer, size_t bufsize,
> +                               const struct kvec_test_range *pr)
>  {
>         size_t size = 0;
>         int i;
> @@ -95,7 +93,7 @@ static void __init iov_kunit_load_kvec(struct kunit *test,
>  /*
>   * Test copying to a ITER_KVEC-type iterator.
>   */
> -static void __init iov_kunit_copy_to_kvec(struct kunit *test)
> +static void iov_kunit_copy_to_kvec(struct kunit *test)
>  {
>         const struct kvec_test_range *pr;
>         struct iov_iter iter;
> @@ -145,7 +143,7 @@ static void __init iov_kunit_copy_to_kvec(struct kunit *test)
>  /*
>   * Test copying from a ITER_KVEC-type iterator.
>   */
> -static void __init iov_kunit_copy_from_kvec(struct kunit *test)
> +static void iov_kunit_copy_from_kvec(struct kunit *test)
>  {
>         const struct kvec_test_range *pr;
>         struct iov_iter iter;
> @@ -213,12 +211,11 @@ static const struct bvec_test_range bvec_test_ranges[] = {
>         { -1, -1, -1 }
>  };
>
> -static void __init iov_kunit_load_bvec(struct kunit *test,
> -                                      struct iov_iter *iter, int dir,
> -                                      struct bio_vec *bvec, unsigned int bvmax,
> -                                      struct page **pages, size_t npages,
> -                                      size_t bufsize,
> -                                      const struct bvec_test_range *pr)
> +static void iov_kunit_load_bvec(struct kunit *test, struct iov_iter *iter,
> +                               int dir, struct bio_vec *bvec,
> +                               unsigned int bvmax, struct page **pages,
> +                               size_t npages, size_t bufsize,
> +                               const struct bvec_test_range *pr)
>  {
>         struct page *can_merge = NULL, *page;
>         size_t size = 0;
> @@ -254,7 +251,7 @@ static void __init iov_kunit_load_bvec(struct kunit *test,
>  /*
>   * Test copying to a ITER_BVEC-type iterator.
>   */
> -static void __init iov_kunit_copy_to_bvec(struct kunit *test)
> +static void iov_kunit_copy_to_bvec(struct kunit *test)
>  {
>         const struct bvec_test_range *pr;
>         struct iov_iter iter;
> @@ -308,7 +305,7 @@ static void __init iov_kunit_copy_to_bvec(struct kunit *test)
>  /*
>   * Test copying from a ITER_BVEC-type iterator.
>   */
> -static void __init iov_kunit_copy_from_bvec(struct kunit *test)
> +static void iov_kunit_copy_from_bvec(struct kunit *test)
>  {
>         const struct bvec_test_range *pr;
>         struct iov_iter iter;
> @@ -370,10 +367,9 @@ static void iov_kunit_destroy_xarray(void *data)
>         kfree(xarray);
>  }
>
> -static void __init iov_kunit_load_xarray(struct kunit *test,
> -                                        struct iov_iter *iter, int dir,
> -                                        struct xarray *xarray,
> -                                        struct page **pages, size_t npages)
> +static void iov_kunit_load_xarray(struct kunit *test, struct iov_iter *iter,
> +                                 int dir, struct xarray *xarray,
> +                                 struct page **pages, size_t npages)
>  {
>         size_t size = 0;
>         int i;
> @@ -401,7 +397,7 @@ static struct xarray *iov_kunit_create_xarray(struct kunit *test)
>  /*
>   * Test copying to a ITER_XARRAY-type iterator.
>   */
> -static void __init iov_kunit_copy_to_xarray(struct kunit *test)
> +static void iov_kunit_copy_to_xarray(struct kunit *test)
>  {
>         const struct kvec_test_range *pr;
>         struct iov_iter iter;
> @@ -459,7 +455,7 @@ static void __init iov_kunit_copy_to_xarray(struct kunit *test)
>  /*
>   * Test copying from a ITER_XARRAY-type iterator.
>   */
> -static void __init iov_kunit_copy_from_xarray(struct kunit *test)
> +static void iov_kunit_copy_from_xarray(struct kunit *test)
>  {
>         const struct kvec_test_range *pr;
>         struct iov_iter iter;
> @@ -522,7 +518,7 @@ static void __init iov_kunit_copy_from_xarray(struct kunit *test)
>  /*
>   * Test the extraction of ITER_KVEC-type iterators.
>   */
> -static void __init iov_kunit_extract_pages_kvec(struct kunit *test)
> +static void iov_kunit_extract_pages_kvec(struct kunit *test)
>  {
>         const struct kvec_test_range *pr;
>         struct iov_iter iter;
> @@ -602,7 +598,7 @@ static void __init iov_kunit_extract_pages_kvec(struct kunit *test)
>  /*
>   * Test the extraction of ITER_BVEC-type iterators.
>   */
> -static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
> +static void iov_kunit_extract_pages_bvec(struct kunit *test)
>  {
>         const struct bvec_test_range *pr;
>         struct iov_iter iter;
> @@ -680,7 +676,7 @@ static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
>  /*
>   * Test the extraction of ITER_XARRAY-type iterators.
>   */
> -static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
> +static void iov_kunit_extract_pages_xarray(struct kunit *test)
>  {
>         const struct kvec_test_range *pr;
>         struct iov_iter iter;
> @@ -756,7 +752,7 @@ static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
>         KUNIT_SUCCEED();
>  }
>
> -static struct kunit_case __refdata iov_kunit_cases[] = {
> +static struct kunit_case iov_kunit_cases[] = {
>         KUNIT_CASE(iov_kunit_copy_to_kvec),
>         KUNIT_CASE(iov_kunit_copy_from_kvec),
>         KUNIT_CASE(iov_kunit_copy_to_bvec),
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 0b6488efed47..e1980ea58118 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -104,12 +104,13 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
>                 self._kconfig = qemu_arch_params.kconfig
>                 self._qemu_arch = qemu_arch_params.qemu_arch
>                 self._kernel_path = qemu_arch_params.kernel_path
> -               self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
> +               self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot rootfstype=tmpfs'
>                 self._extra_qemu_params = qemu_arch_params.extra_qemu_params
>                 self._serial = qemu_arch_params.serial
>
>         def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
>                 kconfig = kunit_config.parse_from_string(self._kconfig)
> +               kconfig.add_entry('TMPFS', 'y')

Can we add these to config files rather than add them here?


>                 kconfig.merge_in_entries(base_kunitconfig)
>                 return kconfig
>
> @@ -139,13 +140,14 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
>
>         def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
>                 kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
> +               kconfig.add_entry('TMPFS', 'y')
>                 kconfig.merge_in_entries(base_kunitconfig)
>                 return kconfig
>
>         def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
>                 """Runs the Linux UML binary. Must be named 'linux'."""
>                 linux_bin = os.path.join(build_dir, 'linux')
> -               params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt'])
> +               params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt', 'rootfstype=tmpfs'])
>                 return subprocess.Popen([linux_bin] + params,
>                                            stdin=subprocess.PIPE,
>                                            stdout=subprocess.PIPE,
> --
> 2.44.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]

Re: [PATCH v1 1/8] kunit: Run tests when the kernel is fully setup

[Mickaël Salaün]

On Fri, Mar 01, 2024 at 03:14:49PM +0800, David Gow wrote:
> On Fri, 1 Mar 2024 at 01:04, Mickaël Salaün <mic@digikod.net> wrote:
>  >
> > Run all the KUnit tests just before the first userspace code is
> > launched.  This makes it it possible to write new tests that check the
> > kernel in its final state i.e., with all async __init code called,
> > memory and RCU properly set up, and sysctl boot arguments evaluated.
> 
> KUnit has explicit support for running tests which call __init code
> (or are themselves marked __init), so I'm not keen to move _all_ tests
> here.

Makes sense.

> 
> That being said, I'm okay with running all of the other tests (ones
> not explicitly marked init) after __init.

I guess we should have to set of tests, the first one (marked with
__init) to run as it is now, and others set to run when the kernel is
fully setup.  I guess we should be able to identify the init sections
for KUnit tests and rely on that to create the two sets.  I'll work on
that in a separate patch series.

> >
> > The initial motivation is to run hardening tests (e.g. memory
> > protection, Heki's CR-pinning), which require such security protection
> > to be fully setup (e.g. memory marked as read-only).
> >
> > Because the suite set could refer to init data, initialize the suite set
> > with late_initcall(), before kunit_run_all_tests(), if KUnit is built-in
> > and enabled at boot time.  To make it more consistent and easier to
> > manage, whatever filters are used or not, always copy test suite entries
> > and free them after all tests are run.
> 
> The goal is to allow init data only for suites explicitly marked as
> such, so we should be able to split that along these lines.
> 
> And yeah, the filter code is pretty convoluted when it comes to when
> they're allocated, and it does do things like check how the suite set
> was allocated. So we do need to make any changes carefully.
> 
> As Kees suggested, I'd like to see any cleanup here as a separate patch.

Right, I'll send a v2 without this kernel init changes.

> 
> >
> > Because of the prepare_namespace() call, we need to have a valid root
> > filesystem.  To make it simple, let's use tmpfs with an empty root.
> > Teach kunit_kernel.py:LinuxSourceTreeOperations*() about the related
> > kernel boot argument, and add this filesystem to the kunit.py's kernel
> > build requirements.
> 
> I think this is a big enough change we'll need to handle it very
> carefully: there are a few places where the fact that KUnit doesn't
> require a root filesystem is documented, and possibly some other
> (non-kunit.py) runners which rely on this.
> 
> I don't think that's necessarily a blocker, but we'll definitely want
> to document it well, and make sure users are aware before this lands.

Indeed

> 
> >
> > Remove __init and __refdata markers from iov_iter, bitfield, checksum,
> > and the example KUnit tests.  Without this change, the kernel tries to
> > execute NX-protected pages (because the pages are deallocated).
> 
> We still want to support these, but we should make sure the suites are
> declared with kunit_init_test_suite().

OK

> 
> >
> > Tested with:
> > ./tools/testing/kunit/kunit.py run --alltests
> > ./tools/testing/kunit/kunit.py run --alltests --arch x86_64
> 
> FYI: This breaks arm64:
> ./tools/testing/kunit/kunit.py run --raw_output --arch arm64
> --cross_compile=aarch64-linux-gnu-
> 
> Unable to handle kernel paging request at virtual address ffffffff02016f88
> ...
> Call trace:
>  kfree+0x30/0x184
>  kunit_run_all_tests+0x88/0x154
>  kernel_init+0x6c/0x1e0
>  ret_from_fork+0x10/0x20
> Code: b25f7be1 aa0003f4 d34cfe73 8b131833 (f9400661)
> ---[ end trace 0000000000000000 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

I guess it is realted to this patch, so I'll fix that with a future
series focus on these kernel init changes.

> 
> 
> >
> > Cc: Alan Maguire <alan.maguire@oracle.com>
> > Cc: Brendan Higgins <brendanhiggins@google.com>
> > Cc: David Gow <davidgow@google.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Luis Chamberlain <mcgrof@kernel.org>
> > Cc: Marco Pagani <marpagan@redhat.com>
> > Cc: Rae Moar <rmoar@google.com>
> > Cc: Shuah Khan <skhan@linuxfoundation.org>
> > Cc: Stephen Boyd <sboyd@kernel.org>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  init/main.c                         |  4 +-
> >  lib/bitfield_kunit.c                |  8 +--
> >  lib/checksum_kunit.c                |  2 +-
> >  lib/kunit/executor.c                | 81 +++++++++++++++++++++--------
> >  lib/kunit/kunit-example-test.c      |  6 +--
> >  lib/kunit_iov_iter.c                | 52 +++++++++---------
> >  tools/testing/kunit/kunit_kernel.py |  6 ++-
> >  7 files changed, 96 insertions(+), 63 deletions(-)
> >
> > diff --git a/init/main.c b/init/main.c
> > index e24b0780fdff..b39d74727aad 100644
> > --- a/init/main.c
> > +++ b/init/main.c
> > @@ -1463,6 +1463,8 @@ static int __ref kernel_init(void *unused)
> >
> >         do_sysctl_args();
> >
> > +       kunit_run_all_tests();
> > +
> >         if (ramdisk_execute_command) {
> >                 ret = run_init_process(ramdisk_execute_command);
> >                 if (!ret)
> > @@ -1550,8 +1552,6 @@ static noinline void __init kernel_init_freeable(void)
> >
> >         do_basic_setup();
> >
> > -       kunit_run_all_tests();
> > -
> >         wait_for_initramfs();
> >         console_on_rootfs();
> >
> > diff --git a/lib/bitfield_kunit.c b/lib/bitfield_kunit.c
> > index 1473d8b4bf0f..71e9f2e96496 100644
> > --- a/lib/bitfield_kunit.c
> > +++ b/lib/bitfield_kunit.c
> > @@ -57,7 +57,7 @@
> >                 CHECK_ENC_GET_BE(tp, v, field, res);                    \
> >         } while (0)
> >
> > -static void __init test_bitfields_constants(struct kunit *context)
> > +static void test_bitfields_constants(struct kunit *context)
> >  {
> >         /*
> >          * NOTE
> > @@ -100,7 +100,7 @@ static void __init test_bitfields_constants(struct kunit *context)
> >                                 tp##_encode_bits(v, mask) != v << __ffs64(mask));\
> >         } while (0)
> >
> > -static void __init test_bitfields_variables(struct kunit *context)
> > +static void test_bitfields_variables(struct kunit *context)
> >  {
> >         CHECK(u8, 0x0f);
> >         CHECK(u8, 0xf0);
> > @@ -126,7 +126,7 @@ static void __init test_bitfields_variables(struct kunit *context)
> >  }
> >
> >  #ifdef TEST_BITFIELD_COMPILE
> > -static void __init test_bitfields_compile(struct kunit *context)
> > +static void test_bitfields_compile(struct kunit *context)
> >  {
> >         /* these should fail compilation */
> >         CHECK_ENC_GET(16, 16, 0x0f00, 0x1000);
> > @@ -137,7 +137,7 @@ static void __init test_bitfields_compile(struct kunit *context)
> >  }
> >  #endif
> >
> > -static struct kunit_case __refdata bitfields_test_cases[] = {
> > +static struct kunit_case bitfields_test_cases[] = {
> >         KUNIT_CASE(test_bitfields_constants),
> >         KUNIT_CASE(test_bitfields_variables),
> >         {}
> > diff --git a/lib/checksum_kunit.c b/lib/checksum_kunit.c
> > index 225bb7701460..41aaed3a4963 100644
> > --- a/lib/checksum_kunit.c
> > +++ b/lib/checksum_kunit.c
> > @@ -620,7 +620,7 @@ static void test_csum_ipv6_magic(struct kunit *test)
> >  #endif /* !CONFIG_NET */
> >  }
> >
> > -static struct kunit_case __refdata checksum_test_cases[] = {
> > +static struct kunit_case checksum_test_cases[] = {
> >         KUNIT_CASE(test_csum_fixed_random_inputs),
> >         KUNIT_CASE(test_csum_all_carry_inputs),
> >         KUNIT_CASE(test_csum_no_carry_inputs),
> > diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> > index 689fff2b2b10..ff3e66ffa739 100644
> > --- a/lib/kunit/executor.c
> > +++ b/lib/kunit/executor.c
> > @@ -15,6 +15,8 @@ extern struct kunit_suite * const __kunit_suites_end[];
> >  extern struct kunit_suite * const __kunit_init_suites_start[];
> >  extern struct kunit_suite * const __kunit_init_suites_end[];
> >
> > +static struct kunit_suite_set final_suite_set = {};
> > +
> >  static char *action_param;
> >
> >  module_param_named(action, action_param, charp, 0400);
> > @@ -233,6 +235,21 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set,
> >                 if (!filtered_suite)
> >                         continue;
> >
> > +               if (filtered_suite == suite_set->start[i]) {
> > +                       /*
> > +                        * To make memory allocation consistent whatever
> > +                        * filters are used or not, and to keep
> > +                        * kunit_free_suite_set() simple, always copy static
> > +                        * data.
> > +                        */
> > +                       filtered_suite = kmemdup(filtered_suite, sizeof(*filtered_suite),
> > +                                       GFP_KERNEL);
> > +                       if (!filtered_suite) {
> > +                               *err = -ENOMEM;
> > +                               goto free_parsed_filters;
> > +                       }
> > +               }
> > +
> >                 *copy++ = filtered_suite;
> >         }
> >         filtered.start = copy_start;
> > @@ -348,7 +365,7 @@ static void kunit_handle_shutdown(void)
> >
> >  }
> >
> > -int kunit_run_all_tests(void)
> > +static int kunit_init_suites(void)
> >  {
> >         struct kunit_suite_set suite_set = {NULL, NULL};
> >         struct kunit_suite_set filtered_suite_set = {NULL, NULL};
> > @@ -361,6 +378,9 @@ int kunit_run_all_tests(void)
> >         size_t init_num_suites = init_suite_set.end - init_suite_set.start;
> >         int err = 0;
> >
> > +       if (!kunit_enabled())
> > +               return 0;
> > +
> >         if (init_num_suites > 0) {
> >                 suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
> >                 if (!suite_set.start)
> > @@ -368,41 +388,56 @@ int kunit_run_all_tests(void)
> >         } else
> >                 suite_set = normal_suite_set;
> >
> > -       if (!kunit_enabled()) {
> > -               pr_info("kunit: disabled\n");
> > +       filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> > +                       filter_param, filter_action_param, &err);
> > +
> > +       /* Free original suite set before using filtered suite set */
> > +       if (init_num_suites > 0)
> > +               kfree(suite_set.start);
> > +       suite_set = filtered_suite_set;
> > +
> > +       if (err) {
> > +               pr_err("kunit executor: error filtering suites: %d\n", err);
> >                 goto free_out;
> >         }
> >
> > -       if (filter_glob_param || filter_param) {
> > -               filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> > -                               filter_param, filter_action_param, &err);
> > +       final_suite_set = suite_set;
> > +       return 0;
> >
> > -               /* Free original suite set before using filtered suite set */
> > -               if (init_num_suites > 0)
> > -                       kfree(suite_set.start);
> > -               suite_set = filtered_suite_set;
> > +free_out:
> > +       kunit_free_suite_set(suite_set);
> >
> > -               if (err) {
> > -                       pr_err("kunit executor: error filtering suites: %d\n", err);
> > -                       goto free_out;
> > -               }
> > +out:
> > +       kunit_handle_shutdown();
> > +       return err;
> > +}
> > +
> > +late_initcall(kunit_init_suites);
> > +
> > +int kunit_run_all_tests(void)
> > +{
> > +       int err = 0;
> > +
> > +       if (!kunit_enabled()) {
> > +               pr_info("kunit: disabled\n");
> > +               goto out;
> >         }
> >
> > +       if (!final_suite_set.start)
> > +               goto out;
> > +
> >         if (!action_param)
> > -               kunit_exec_run_tests(&suite_set, true);
> > +               kunit_exec_run_tests(&final_suite_set, true);
> >         else if (strcmp(action_param, "list") == 0)
> > -               kunit_exec_list_tests(&suite_set, false);
> > +               kunit_exec_list_tests(&final_suite_set, false);
> >         else if (strcmp(action_param, "list_attr") == 0)
> > -               kunit_exec_list_tests(&suite_set, true);
> > +               kunit_exec_list_tests(&final_suite_set, true);
> >         else
> >                 pr_err("kunit executor: unknown action '%s'\n", action_param);
> >
> > -free_out:
> > -       if (filter_glob_param || filter_param)
> > -               kunit_free_suite_set(suite_set);
> > -       else if (init_num_suites > 0)
> > -               /* Don't use kunit_free_suite_set because suites aren't individually allocated */
> > -               kfree(suite_set.start);
> > +       kunit_free_suite_set(final_suite_set);
> > +       final_suite_set.start = NULL;
> > +       final_suite_set.end = NULL;
> >
> >  out:
> >         kunit_handle_shutdown();
> > diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> > index 798924f7cc86..248949eb3b16 100644
> > --- a/lib/kunit/kunit-example-test.c
> > +++ b/lib/kunit/kunit-example-test.c
> > @@ -337,7 +337,7 @@ static struct kunit_suite example_test_suite = {
> >   */
> >  kunit_test_suites(&example_test_suite);
> >
> > -static int __init init_add(int x, int y)
> > +static int init_add(int x, int y)
> 
> The whole point of this function (and the example_init_test) is that
> they're marked __init, as proof we can test code which is in the init
> section.
> 
> So, either we leave this alone, or we remove it entirely. There's no
> point just removing __init.

Right

> 
> >  {
> >         return (x + y);
> >  }
> > @@ -345,7 +345,7 @@ static int __init init_add(int x, int y)
> >  /*
> >   * This test should always pass. Can be used to test init suites.
> >   */
> > -static void __init example_init_test(struct kunit *test)
> > +static void example_init_test(struct kunit *test)
> 
> As above.
> 
> >  {
> >         KUNIT_EXPECT_EQ(test, init_add(1, 1), 2);
> >  }
> > @@ -354,7 +354,7 @@ static void __init example_init_test(struct kunit *test)
> >   * The kunit_case struct cannot be marked as __initdata as this will be
> >   * used in debugfs to retrieve results after test has run
> >   */
> > -static struct kunit_case __refdata example_init_test_cases[] = {
> > +static struct kunit_case example_init_test_cases[] = {
> >         KUNIT_CASE(example_init_test),
> >         {}
> >  };
> 
> As above.
> 
> > diff --git a/lib/kunit_iov_iter.c b/lib/kunit_iov_iter.c
> > index 859b67c4d697..a77991a9bffb 100644
> > --- a/lib/kunit_iov_iter.c
> > +++ b/lib/kunit_iov_iter.c
> > @@ -44,9 +44,8 @@ static void iov_kunit_unmap(void *data)
> >         vunmap(data);
> >  }
> >
> > -static void *__init iov_kunit_create_buffer(struct kunit *test,
> > -                                           struct page ***ppages,
> > -                                           size_t npages)
> > +static void *iov_kunit_create_buffer(struct kunit *test, struct page ***ppages,
> > +                                    size_t npages)
> >  {
> >         struct page **pages;
> >         unsigned long got;
> > @@ -69,11 +68,10 @@ static void *__init iov_kunit_create_buffer(struct kunit *test,
> >         return buffer;
> >  }
> >
> > -static void __init iov_kunit_load_kvec(struct kunit *test,
> > -                                      struct iov_iter *iter, int dir,
> > -                                      struct kvec *kvec, unsigned int kvmax,
> > -                                      void *buffer, size_t bufsize,
> > -                                      const struct kvec_test_range *pr)
> > +static void iov_kunit_load_kvec(struct kunit *test, struct iov_iter *iter,
> > +                               int dir, struct kvec *kvec, unsigned int kvmax,
> > +                               void *buffer, size_t bufsize,
> > +                               const struct kvec_test_range *pr)
> >  {
> >         size_t size = 0;
> >         int i;
> > @@ -95,7 +93,7 @@ static void __init iov_kunit_load_kvec(struct kunit *test,
> >  /*
> >   * Test copying to a ITER_KVEC-type iterator.
> >   */
> > -static void __init iov_kunit_copy_to_kvec(struct kunit *test)
> > +static void iov_kunit_copy_to_kvec(struct kunit *test)
> >  {
> >         const struct kvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -145,7 +143,7 @@ static void __init iov_kunit_copy_to_kvec(struct kunit *test)
> >  /*
> >   * Test copying from a ITER_KVEC-type iterator.
> >   */
> > -static void __init iov_kunit_copy_from_kvec(struct kunit *test)
> > +static void iov_kunit_copy_from_kvec(struct kunit *test)
> >  {
> >         const struct kvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -213,12 +211,11 @@ static const struct bvec_test_range bvec_test_ranges[] = {
> >         { -1, -1, -1 }
> >  };
> >
> > -static void __init iov_kunit_load_bvec(struct kunit *test,
> > -                                      struct iov_iter *iter, int dir,
> > -                                      struct bio_vec *bvec, unsigned int bvmax,
> > -                                      struct page **pages, size_t npages,
> > -                                      size_t bufsize,
> > -                                      const struct bvec_test_range *pr)
> > +static void iov_kunit_load_bvec(struct kunit *test, struct iov_iter *iter,
> > +                               int dir, struct bio_vec *bvec,
> > +                               unsigned int bvmax, struct page **pages,
> > +                               size_t npages, size_t bufsize,
> > +                               const struct bvec_test_range *pr)
> >  {
> >         struct page *can_merge = NULL, *page;
> >         size_t size = 0;
> > @@ -254,7 +251,7 @@ static void __init iov_kunit_load_bvec(struct kunit *test,
> >  /*
> >   * Test copying to a ITER_BVEC-type iterator.
> >   */
> > -static void __init iov_kunit_copy_to_bvec(struct kunit *test)
> > +static void iov_kunit_copy_to_bvec(struct kunit *test)
> >  {
> >         const struct bvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -308,7 +305,7 @@ static void __init iov_kunit_copy_to_bvec(struct kunit *test)
> >  /*
> >   * Test copying from a ITER_BVEC-type iterator.
> >   */
> > -static void __init iov_kunit_copy_from_bvec(struct kunit *test)
> > +static void iov_kunit_copy_from_bvec(struct kunit *test)
> >  {
> >         const struct bvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -370,10 +367,9 @@ static void iov_kunit_destroy_xarray(void *data)
> >         kfree(xarray);
> >  }
> >
> > -static void __init iov_kunit_load_xarray(struct kunit *test,
> > -                                        struct iov_iter *iter, int dir,
> > -                                        struct xarray *xarray,
> > -                                        struct page **pages, size_t npages)
> > +static void iov_kunit_load_xarray(struct kunit *test, struct iov_iter *iter,
> > +                                 int dir, struct xarray *xarray,
> > +                                 struct page **pages, size_t npages)
> >  {
> >         size_t size = 0;
> >         int i;
> > @@ -401,7 +397,7 @@ static struct xarray *iov_kunit_create_xarray(struct kunit *test)
> >  /*
> >   * Test copying to a ITER_XARRAY-type iterator.
> >   */
> > -static void __init iov_kunit_copy_to_xarray(struct kunit *test)
> > +static void iov_kunit_copy_to_xarray(struct kunit *test)
> >  {
> >         const struct kvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -459,7 +455,7 @@ static void __init iov_kunit_copy_to_xarray(struct kunit *test)
> >  /*
> >   * Test copying from a ITER_XARRAY-type iterator.
> >   */
> > -static void __init iov_kunit_copy_from_xarray(struct kunit *test)
> > +static void iov_kunit_copy_from_xarray(struct kunit *test)
> >  {
> >         const struct kvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -522,7 +518,7 @@ static void __init iov_kunit_copy_from_xarray(struct kunit *test)
> >  /*
> >   * Test the extraction of ITER_KVEC-type iterators.
> >   */
> > -static void __init iov_kunit_extract_pages_kvec(struct kunit *test)
> > +static void iov_kunit_extract_pages_kvec(struct kunit *test)
> >  {
> >         const struct kvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -602,7 +598,7 @@ static void __init iov_kunit_extract_pages_kvec(struct kunit *test)
> >  /*
> >   * Test the extraction of ITER_BVEC-type iterators.
> >   */
> > -static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
> > +static void iov_kunit_extract_pages_bvec(struct kunit *test)
> >  {
> >         const struct bvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -680,7 +676,7 @@ static void __init iov_kunit_extract_pages_bvec(struct kunit *test)
> >  /*
> >   * Test the extraction of ITER_XARRAY-type iterators.
> >   */
> > -static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
> > +static void iov_kunit_extract_pages_xarray(struct kunit *test)
> >  {
> >         const struct kvec_test_range *pr;
> >         struct iov_iter iter;
> > @@ -756,7 +752,7 @@ static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
> >         KUNIT_SUCCEED();
> >  }
> >
> > -static struct kunit_case __refdata iov_kunit_cases[] = {
> > +static struct kunit_case iov_kunit_cases[] = {
> >         KUNIT_CASE(iov_kunit_copy_to_kvec),
> >         KUNIT_CASE(iov_kunit_copy_from_kvec),
> >         KUNIT_CASE(iov_kunit_copy_to_bvec),
> > diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> > index 0b6488efed47..e1980ea58118 100644
> > --- a/tools/testing/kunit/kunit_kernel.py
> > +++ b/tools/testing/kunit/kunit_kernel.py
> > @@ -104,12 +104,13 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
> >                 self._kconfig = qemu_arch_params.kconfig
> >                 self._qemu_arch = qemu_arch_params.qemu_arch
> >                 self._kernel_path = qemu_arch_params.kernel_path
> > -               self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
> > +               self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot rootfstype=tmpfs'
> >                 self._extra_qemu_params = qemu_arch_params.extra_qemu_params
> >                 self._serial = qemu_arch_params.serial
> >
> >         def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> >                 kconfig = kunit_config.parse_from_string(self._kconfig)
> > +               kconfig.add_entry('TMPFS', 'y')
> 
> Can we add these to config files rather than add them here?

I initially tried that but this makes this change more complex because
of the way UML and non-UML+arch configs are managed.

> 
> 
> >                 kconfig.merge_in_entries(base_kunitconfig)
> >                 return kconfig
> >
> > @@ -139,13 +140,14 @@ class LinuxSourceTreeOperationsUml(LinuxSourceTreeOperations):
> >
> >         def make_arch_config(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
> >                 kconfig = kunit_config.parse_file(UML_KCONFIG_PATH)
> > +               kconfig.add_entry('TMPFS', 'y')
> >                 kconfig.merge_in_entries(base_kunitconfig)
> >                 return kconfig
> >
> >         def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
> >                 """Runs the Linux UML binary. Must be named 'linux'."""
> >                 linux_bin = os.path.join(build_dir, 'linux')
> > -               params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt'])
> > +               params.extend(['mem=1G', 'console=tty', 'kunit_shutdown=halt', 'rootfstype=tmpfs'])
> >                 return subprocess.Popen([linux_bin] + params,
> >                                            stdin=subprocess.PIPE,
> >                                            stdout=subprocess.PIPE,
> > --
> > 2.44.0
> >

Re: [PATCH v1 1/8] kunit: Run tests when the kernel is fully setup

[kernel test robot]

Hi Mickaël,

kernel test robot noticed the following build warnings:

[auto build test WARNING on d206a76d7d2726f3b096037f2079ce0bd3ba329b]

url:    https://github.com/intel-lab-lkp/linux/commits/Micka-l-Sala-n/kunit-Run-tests-when-the-kernel-is-fully-setup/20240301-011020
base:   d206a76d7d2726f3b096037f2079ce0bd3ba329b
patch link:    https://lore.kernel.org/r/20240229170409.365386-2-mic%40digikod.net
patch subject: [PATCH v1 1/8] kunit: Run tests when the kernel is fully setup
config: s390-defconfig (https://download.01.org/0day-ci/archive/20240301/202403011856.cJe6do38-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 325f51237252e6dab8e4e1ea1fa7acbb4faee1cd)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240301/202403011856.cJe6do38-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403011856.cJe6do38-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from lib/kunit/executor.c:4:
   In file included from include/kunit/test.h:24:
   In file included from include/linux/module.h:19:
   In file included from include/linux/elf.h:6:
   In file included from arch/s390/include/asm/elf.h:173:
   In file included from arch/s390/include/asm/mmu_context.h:11:
   In file included from arch/s390/include/asm/pgalloc.h:18:
   In file included from include/linux/mm.h:2188:
   include/linux/vmstat.h:508:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     508 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     509 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:515:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     515 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     516 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     522 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:527:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     527 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     528 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:536:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     536 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     537 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> lib/kunit/executor.c:18:31: warning: unused variable 'final_suite_set' [-Wunused-variable]
      18 | static struct kunit_suite_set final_suite_set = {};
         |                               ^~~~~~~~~~~~~~~
   6 warnings generated.


vim +/final_suite_set +18 lib/kunit/executor.c

    17	
  > 18	static struct kunit_suite_set final_suite_set = {};
    19	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v1 2/8] kunit: Handle thread creation error

[Mickaël Salaün]

Previously, if a thread creation failed (e.g. -ENOMEM), the function was
called (kunit_catch_run_case or kunit_catch_run_case_cleanup) without
marking the test as failed.  Instead, fill try_result with the error
code returned by kthread_run(), which will mark the test as failed and
print "internal error occurred...".

Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 lib/kunit/try-catch.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
index f7825991d576..a5cb2ef70a25 100644
--- a/lib/kunit/try-catch.c
+++ b/lib/kunit/try-catch.c
@@ -69,6 +69,7 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 				  try_catch,
 				  "kunit_try_catch_thread");
 	if (IS_ERR(task_struct)) {
+		try_catch->try_result = PTR_ERR(task_struct);
 		try_catch->catch(try_catch->context);
 		return;
 	}
-- 
2.44.0

Re: [PATCH v1 2/8] kunit: Handle thread creation error

[Kees Cook]

On Thu, Feb 29, 2024 at 06:04:03PM +0100, Mickaël Salaün wrote:
> Previously, if a thread creation failed (e.g. -ENOMEM), the function was
> called (kunit_catch_run_case or kunit_catch_run_case_cleanup) without
> marking the test as failed.  Instead, fill try_result with the error
> code returned by kthread_run(), which will mark the test as failed and
> print "internal error occurred...".
> 
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH v1 3/8] kunit: Fix kthread reference

[Mickaël Salaün]

There is a race condition when a kthread finishes after the deadline and
before the call to kthread_stop(), which may lead to use after free.

Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 lib/kunit/try-catch.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
index a5cb2ef70a25..73f5007f20ea 100644
--- a/lib/kunit/try-catch.c
+++ b/lib/kunit/try-catch.c
@@ -11,6 +11,7 @@
 #include <linux/completion.h>
 #include <linux/kernel.h>
 #include <linux/kthread.h>
+#include <linux/sched/task.h>
 
 #include "try-catch-impl.h"
 
@@ -65,14 +66,15 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 	try_catch->context = context;
 	try_catch->try_completion = &try_completion;
 	try_catch->try_result = 0;
-	task_struct = kthread_run(kunit_generic_run_threadfn_adapter,
-				  try_catch,
-				  "kunit_try_catch_thread");
+	task_struct = kthread_create(kunit_generic_run_threadfn_adapter,
+				     try_catch, "kunit_try_catch_thread");
 	if (IS_ERR(task_struct)) {
 		try_catch->try_result = PTR_ERR(task_struct);
 		try_catch->catch(try_catch->context);
 		return;
 	}
+	get_task_struct(task_struct);
+	wake_up_process(task_struct);
 
 	time_remaining = wait_for_completion_timeout(&try_completion,
 						     kunit_test_timeout());
@@ -82,6 +84,7 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 		kthread_stop(task_struct);
 	}
 
+	put_task_struct(task_struct);
 	exit_code = try_catch->try_result;
 
 	if (!exit_code)
-- 
2.44.0

Re: [PATCH v1 3/8] kunit: Fix kthread reference

[Kees Cook]

On Thu, Feb 29, 2024 at 06:04:04PM +0100, Mickaël Salaün wrote:
> There is a race condition when a kthread finishes after the deadline and
> before the call to kthread_stop(), which may lead to use after free.
> 
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH v1 4/8] kunit: Fix timeout message

[Mickaël Salaün]

The exit code is always checked, so let's properly handle the -ETIMEDOUT
error code.

Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 lib/kunit/try-catch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
index 73f5007f20ea..cab8b24b5d5a 100644
--- a/lib/kunit/try-catch.c
+++ b/lib/kunit/try-catch.c
@@ -79,7 +79,6 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 	time_remaining = wait_for_completion_timeout(&try_completion,
 						     kunit_test_timeout());
 	if (time_remaining == 0) {
-		kunit_err(test, "try timed out\n");
 		try_catch->try_result = -ETIMEDOUT;
 		kthread_stop(task_struct);
 	}
@@ -94,6 +93,8 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 		try_catch->try_result = 0;
 	else if (exit_code == -EINTR)
 		kunit_err(test, "wake_up_process() was never called\n");
+	else if (exit_code == -ETIMEDOUT)
+		kunit_err(test, "try timed out\n");
 	else if (exit_code)
 		kunit_err(test, "Unknown error: %d\n", exit_code);
 
-- 
2.44.0

Re: [PATCH v1 4/8] kunit: Fix timeout message

[Kees Cook]

On Thu, Feb 29, 2024 at 06:04:05PM +0100, Mickaël Salaün wrote:
> The exit code is always checked, so let's properly handle the -ETIMEDOUT
> error code.
> 
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH v1 5/8] kunit: Handle test faults

[Mickaël Salaün]

Previously, when a kernel test thread crashed (e.g. NULL pointer
dereference, general protection fault), the KUnit test hanged for 30
seconds and exited with a timeout error.

Fix this issue by waiting on task_struct->vfork_done instead of the
custom kunit_try_catch.try_completion, and track the execution state by
initially setting try_result with -EFAULT and only setting it to 0 if
the test passed.

Fix kunit_generic_run_threadfn_adapter() signature by returning 0
instead of calling kthread_complete_and_exit().  Because thread's exit
code is never checked, always set it to 0 to make it clear.

Fix the -EINTR error message, which couldn't be reached until now.

This is tested with a following patch.

Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 include/kunit/try-catch.h |  3 ---
 lib/kunit/try-catch.c     | 14 +++++++-------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
index c507dd43119d..7c966a1adbd3 100644
--- a/include/kunit/try-catch.h
+++ b/include/kunit/try-catch.h
@@ -14,13 +14,11 @@
 
 typedef void (*kunit_try_catch_func_t)(void *);
 
-struct completion;
 struct kunit;
 
 /**
  * struct kunit_try_catch - provides a generic way to run code which might fail.
  * @test: The test case that is currently being executed.
- * @try_completion: Completion that the control thread waits on while test runs.
  * @try_result: Contains any errno obtained while running test case.
  * @try: The function, the test case, to attempt to run.
  * @catch: The function called if @try bails out.
@@ -46,7 +44,6 @@ struct kunit;
 struct kunit_try_catch {
 	/* private: internal use only. */
 	struct kunit *test;
-	struct completion *try_completion;
 	int try_result;
 	kunit_try_catch_func_t try;
 	kunit_try_catch_func_t catch;
diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
index cab8b24b5d5a..c6ee4db0b3bd 100644
--- a/lib/kunit/try-catch.c
+++ b/lib/kunit/try-catch.c
@@ -18,7 +18,7 @@
 void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch)
 {
 	try_catch->try_result = -EFAULT;
-	kthread_complete_and_exit(try_catch->try_completion, -EFAULT);
+	kthread_exit(0);
 }
 EXPORT_SYMBOL_GPL(kunit_try_catch_throw);
 
@@ -26,9 +26,12 @@ static int kunit_generic_run_threadfn_adapter(void *data)
 {
 	struct kunit_try_catch *try_catch = data;
 
+	try_catch->try_result = -EINTR;
 	try_catch->try(try_catch->context);
+	if (try_catch->try_result == -EINTR)
+		try_catch->try_result = 0;
 
-	kthread_complete_and_exit(try_catch->try_completion, 0);
+	return 0;
 }
 
 static unsigned long kunit_test_timeout(void)
@@ -58,13 +61,11 @@ static unsigned long kunit_test_timeout(void)
 
 void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 {
-	DECLARE_COMPLETION_ONSTACK(try_completion);
 	struct kunit *test = try_catch->test;
 	struct task_struct *task_struct;
 	int exit_code, time_remaining;
 
 	try_catch->context = context;
-	try_catch->try_completion = &try_completion;
 	try_catch->try_result = 0;
 	task_struct = kthread_create(kunit_generic_run_threadfn_adapter,
 				     try_catch, "kunit_try_catch_thread");
@@ -75,8 +76,7 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 	}
 	get_task_struct(task_struct);
 	wake_up_process(task_struct);
-
-	time_remaining = wait_for_completion_timeout(&try_completion,
+	time_remaining = wait_for_completion_timeout(task_struct->vfork_done,
 						     kunit_test_timeout());
 	if (time_remaining == 0) {
 		try_catch->try_result = -ETIMEDOUT;
@@ -92,7 +92,7 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 	if (exit_code == -EFAULT)
 		try_catch->try_result = 0;
 	else if (exit_code == -EINTR)
-		kunit_err(test, "wake_up_process() was never called\n");
+		kunit_err(test, "try faulted\n");
 	else if (exit_code == -ETIMEDOUT)
 		kunit_err(test, "try timed out\n");
 	else if (exit_code)
-- 
2.44.0

Re: [PATCH v1 5/8] kunit: Handle test faults

[Kees Cook]

On Thu, Feb 29, 2024 at 06:04:06PM +0100, Mickaël Salaün wrote:
> Previously, when a kernel test thread crashed (e.g. NULL pointer
> dereference, general protection fault), the KUnit test hanged for 30
> seconds and exited with a timeout error.
> 
> Fix this issue by waiting on task_struct->vfork_done instead of the
> custom kunit_try_catch.try_completion, and track the execution state by
> initially setting try_result with -EFAULT and only setting it to 0 if
> the test passed.
> 
> Fix kunit_generic_run_threadfn_adapter() signature by returning 0
> instead of calling kthread_complete_and_exit().  Because thread's exit
> code is never checked, always set it to 0 to make it clear.
> 
> Fix the -EINTR error message, which couldn't be reached until now.
> 
> This is tested with a following patch.
> 
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>

I assume we can start checking for "intentional" faults now?

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

Re: [PATCH v1 5/8] kunit: Handle test faults

[Mickaël Salaün]

On Thu, Feb 29, 2024 at 10:24:19AM -0800, Kees Cook wrote:
> On Thu, Feb 29, 2024 at 06:04:06PM +0100, Mickaël Salaün wrote:
> > Previously, when a kernel test thread crashed (e.g. NULL pointer
> > dereference, general protection fault), the KUnit test hanged for 30
> > seconds and exited with a timeout error.
> > 
> > Fix this issue by waiting on task_struct->vfork_done instead of the
> > custom kunit_try_catch.try_completion, and track the execution state by
> > initially setting try_result with -EFAULT and only setting it to 0 if
> > the test passed.
> > 
> > Fix kunit_generic_run_threadfn_adapter() signature by returning 0
> > instead of calling kthread_complete_and_exit().  Because thread's exit
> > code is never checked, always set it to 0 to make it clear.
> > 
> > Fix the -EINTR error message, which couldn't be reached until now.
> > 
> > This is tested with a following patch.
> > 
> > Cc: Brendan Higgins <brendanhiggins@google.com>
> > Cc: David Gow <davidgow@google.com>
> > Cc: Rae Moar <rmoar@google.com>
> > Cc: Shuah Khan <skhan@linuxfoundation.org>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> 
> I assume we can start checking for "intentional" faults now?

Yes, but adding dedicated exception handling for such faults would
probably be cleaner.

At least we can now easily write tests as I did with the last patch. The
only potential issue is that the kernel will still print the related
warning in logs, but I think it's OK for tests (and maybe something we'd
like to test too by the way).

> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 
> -- 
> Kees Cook
> 

[PATCH v1 6/8] kunit: Fix KUNIT_SUCCESS() calls in iov_iter tests

[Mickaël Salaün]

Fix KUNIT_SUCCESS() calls to pass a test argument.

This is a no-op for now because this macro does nothing, but it will be
required for the next commit.

Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 lib/kunit_iov_iter.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/kunit_iov_iter.c b/lib/kunit_iov_iter.c
index a77991a9bffb..b586aa19e45d 100644
--- a/lib/kunit_iov_iter.c
+++ b/lib/kunit_iov_iter.c
@@ -137,7 +137,7 @@ static void iov_kunit_copy_to_kvec(struct kunit *test)
 			return;
 	}
 
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 /*
@@ -192,7 +192,7 @@ static void iov_kunit_copy_from_kvec(struct kunit *test)
 			return;
 	}
 
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 struct bvec_test_range {
@@ -299,7 +299,7 @@ static void iov_kunit_copy_to_bvec(struct kunit *test)
 			return;
 	}
 
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 /*
@@ -356,7 +356,7 @@ static void iov_kunit_copy_from_bvec(struct kunit *test)
 			return;
 	}
 
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 static void iov_kunit_destroy_xarray(void *data)
@@ -449,7 +449,7 @@ static void iov_kunit_copy_to_xarray(struct kunit *test)
 			return;
 	}
 
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 /*
@@ -512,7 +512,7 @@ static void iov_kunit_copy_from_xarray(struct kunit *test)
 			return;
 	}
 
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 /*
@@ -592,7 +592,7 @@ static void iov_kunit_extract_pages_kvec(struct kunit *test)
 stop:
 	KUNIT_EXPECT_EQ(test, size, 0);
 	KUNIT_EXPECT_EQ(test, iter.count, 0);
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 /*
@@ -670,7 +670,7 @@ static void iov_kunit_extract_pages_bvec(struct kunit *test)
 stop:
 	KUNIT_EXPECT_EQ(test, size, 0);
 	KUNIT_EXPECT_EQ(test, iter.count, 0);
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 /*
@@ -749,7 +749,7 @@ static void iov_kunit_extract_pages_xarray(struct kunit *test)
 	}
 
 stop:
-	KUNIT_SUCCEED();
+	KUNIT_SUCCEED(test);
 }
 
 static struct kunit_case iov_kunit_cases[] = {
-- 
2.44.0

Re: [PATCH v1 6/8] kunit: Fix KUNIT_SUCCESS() calls in iov_iter tests

[Kees Cook]

On Thu, Feb 29, 2024 at 06:04:07PM +0100, Mickaël Salaün wrote:
> Fix KUNIT_SUCCESS() calls to pass a test argument.
> 
> This is a no-op for now because this macro does nothing, but it will be
> required for the next commit.
> 
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH v1 7/8] kunit: Print last test location on fault

[Mickaël Salaün]

This helps identify the location of test faults.

Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 include/kunit/test.h  | 24 +++++++++++++++++++++---
 lib/kunit/try-catch.c | 10 +++++++---
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index fcb4a4940ace..f3aa66eb0087 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -301,6 +301,8 @@ struct kunit {
 	struct list_head resources; /* Protected by lock. */
 
 	char status_comment[KUNIT_STATUS_COMMENT_SIZE];
+	/* Saves the last seen test. Useful to help with faults. */
+	struct kunit_loc last_seen;
 };
 
 static inline void kunit_set_failure(struct kunit *test)
@@ -567,6 +569,15 @@ void __printf(2, 3) kunit_log_append(struct string_stream *log, const char *fmt,
 #define kunit_err(test, fmt, ...) \
 	kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__)
 
+/*
+ * Must be called at the beginning of each KUNIT_*_ASSERTION().
+ * Cf. KUNIT_CURRENT_LOC.
+ */
+#define _KUNIT_SAVE_LOC(test) do {					       \
+	WRITE_ONCE(test->last_seen.file, __FILE__);			       \
+	WRITE_ONCE(test->last_seen.line, __LINE__);			       \
+} while (0)
+
 /**
  * KUNIT_SUCCEED() - A no-op expectation. Only exists for code clarity.
  * @test: The test context object.
@@ -575,7 +586,7 @@ void __printf(2, 3) kunit_log_append(struct string_stream *log, const char *fmt,
  * words, it does nothing and only exists for code clarity. See
  * KUNIT_EXPECT_TRUE() for more information.
  */
-#define KUNIT_SUCCEED(test) do {} while (0)
+#define KUNIT_SUCCEED(test) _KUNIT_SAVE_LOC(test)
 
 void __noreturn __kunit_abort(struct kunit *test);
 
@@ -601,14 +612,16 @@ void __kunit_do_failed_assertion(struct kunit *test,
 } while (0)
 
 
-#define KUNIT_FAIL_ASSERTION(test, assert_type, fmt, ...)		       \
+#define KUNIT_FAIL_ASSERTION(test, assert_type, fmt, ...) do {		       \
+	_KUNIT_SAVE_LOC(test);						       \
 	_KUNIT_FAILED(test,						       \
 		      assert_type,					       \
 		      kunit_fail_assert,				       \
 		      kunit_fail_assert_format,				       \
 		      {},						       \
 		      fmt,						       \
-		      ##__VA_ARGS__)
+		      ##__VA_ARGS__);					       \
+} while (0)
 
 /**
  * KUNIT_FAIL() - Always causes a test to fail when evaluated.
@@ -637,6 +650,7 @@ void __kunit_do_failed_assertion(struct kunit *test,
 			      fmt,					       \
 			      ...)					       \
 do {									       \
+	_KUNIT_SAVE_LOC(test);						       \
 	if (likely(!!(condition_) == !!expected_true_))			       \
 		break;							       \
 									       \
@@ -698,6 +712,7 @@ do {									       \
 		.right_text = #right,					       \
 	};								       \
 									       \
+	_KUNIT_SAVE_LOC(test);						       \
 	if (likely(__left op __right))					       \
 		break;							       \
 									       \
@@ -758,6 +773,7 @@ do {									       \
 		.right_text = #right,					       \
 	};								       \
 									       \
+	_KUNIT_SAVE_LOC(test);						       \
 	if (likely((__left) && (__right) && (strcmp(__left, __right) op 0)))   \
 		break;							       \
 									       \
@@ -791,6 +807,7 @@ do {									       \
 		.right_text = #right,					       \
 	};								       \
 									       \
+	_KUNIT_SAVE_LOC(test);						       \
 	if (likely(__left && __right))					       \
 		if (likely(memcmp(__left, __right, __size) op 0))	       \
 			break;						       \
@@ -815,6 +832,7 @@ do {									       \
 do {									       \
 	const typeof(ptr) __ptr = (ptr);				       \
 									       \
+	_KUNIT_SAVE_LOC(test);						       \
 	if (!IS_ERR_OR_NULL(__ptr))					       \
 		break;							       \
 									       \
diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
index c6ee4db0b3bd..2ec21c6918f3 100644
--- a/lib/kunit/try-catch.c
+++ b/lib/kunit/try-catch.c
@@ -91,9 +91,13 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
 
 	if (exit_code == -EFAULT)
 		try_catch->try_result = 0;
-	else if (exit_code == -EINTR)
-		kunit_err(test, "try faulted\n");
-	else if (exit_code == -ETIMEDOUT)
+	else if (exit_code == -EINTR) {
+		if (test->last_seen.file)
+			kunit_err(test, "try faulted after %s:%d\n",
+				  test->last_seen.file, test->last_seen.line);
+		else
+			kunit_err(test, "try faulted before the first test\n");
+	} else if (exit_code == -ETIMEDOUT)
 		kunit_err(test, "try timed out\n");
 	else if (exit_code)
 		kunit_err(test, "Unknown error: %d\n", exit_code);
-- 
2.44.0

Re: [PATCH v1 7/8] kunit: Print last test location on fault

[Kees Cook]

On Thu, Feb 29, 2024 at 06:04:08PM +0100, Mickaël Salaün wrote:
> This helps identify the location of test faults.
> 
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>

Much more detailed error!

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

[PATCH v1 8/8] kunit: Add tests for faults

[Mickaël Salaün]

The first test checks NULL pointer dereference and make sure it would
result as a failed test.

The second and third tests check that read-only data is indeed read-only
and trying to modify it would result as a failed test.

This kunit_x86_fault test suite is marked as skipped when run on a
non-x86 native architecture.  It is then skipped on UML because such
test would result to a kernel panic.

Tested with:
./tools/testing/kunit/kunit.py run --arch x86_64 kunit_x86_fault

Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Cc: Shuah Khan <skhan@linuxfoundation.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 lib/kunit/kunit-test.c | 115 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 114 insertions(+), 1 deletion(-)

diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
index f7980ef236a3..57d8eff00c66 100644
--- a/lib/kunit/kunit-test.c
+++ b/lib/kunit/kunit-test.c
@@ -10,6 +10,7 @@
 #include <kunit/test-bug.h>
 
 #include <linux/device.h>
+#include <linux/init.h>
 #include <kunit/device.h>
 
 #include "string-stream.h"
@@ -109,6 +110,117 @@ static struct kunit_suite kunit_try_catch_test_suite = {
 	.test_cases = kunit_try_catch_test_cases,
 };
 
+#ifdef CONFIG_X86
+
+static void kunit_test_null_dereference(void *data)
+{
+	struct kunit *test = data;
+	int *null = NULL;
+
+	*null = 0;
+
+	KUNIT_FAIL(test, "This line should never be reached\n");
+}
+
+static void kunit_test_fault_null_dereference(struct kunit *test)
+{
+	struct kunit_try_catch_test_context *ctx = test->priv;
+	struct kunit_try_catch *try_catch = ctx->try_catch;
+
+	kunit_try_catch_init(try_catch,
+			     test,
+			     kunit_test_null_dereference,
+			     kunit_test_catch);
+	kunit_try_catch_run(try_catch, test);
+
+	KUNIT_EXPECT_EQ(test, try_catch->try_result, -EINTR);
+	KUNIT_EXPECT_TRUE(test, ctx->function_called);
+}
+
+#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX)
+
+const int test_const = 1;
+
+static void kunit_test_const(void *data)
+{
+	struct kunit *test = data;
+	/* Bypasses compiler check. */
+	int *ptr = (int *)&test_const;
+
+	KUNIT_EXPECT_EQ(test, test_const, 1);
+	*ptr = 2;
+
+	KUNIT_FAIL(test, "This line should never be reached\n");
+}
+
+static void kunit_test_fault_const(struct kunit *test)
+{
+	struct kunit_try_catch_test_context *ctx = test->priv;
+	struct kunit_try_catch *try_catch = ctx->try_catch;
+
+	kunit_try_catch_init(try_catch, test, kunit_test_const,
+			     kunit_test_catch);
+	kunit_try_catch_run(try_catch, test);
+
+	KUNIT_EXPECT_EQ(test, test_const, 1);
+	KUNIT_EXPECT_EQ(test, try_catch->try_result, -EINTR);
+	KUNIT_EXPECT_TRUE(test, ctx->function_called);
+}
+
+static int test_rodata __ro_after_init = 1;
+
+static void kunit_test_rodata(void *data)
+{
+	struct kunit *test = data;
+
+	KUNIT_EXPECT_EQ(test, test_rodata, 1);
+	test_rodata = 2;
+
+	KUNIT_FAIL(test, "This line should never be reached\n");
+}
+
+static void kunit_test_fault_rodata(struct kunit *test)
+{
+	struct kunit_try_catch_test_context *ctx = test->priv;
+	struct kunit_try_catch *try_catch = ctx->try_catch;
+
+	if (!rodata_enabled)
+		kunit_skip(test, "Strict RWX is not enabled");
+
+	kunit_try_catch_init(try_catch, test, kunit_test_rodata,
+			     kunit_test_catch);
+	kunit_try_catch_run(try_catch, test);
+
+	KUNIT_EXPECT_EQ(test, test_rodata, 1);
+	KUNIT_EXPECT_EQ(test, try_catch->try_result, -EINTR);
+	KUNIT_EXPECT_TRUE(test, ctx->function_called);
+}
+
+#else /* defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) */
+
+static void kunit_test_fault_rodata(struct kunit *test)
+{
+	kunit_skip(test, "Strict RWX is not supported");
+}
+
+#endif /* defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) */
+#endif /* CONFIG_X86 */
+
+static struct kunit_case kunit_x86_fault_test_cases[] = {
+#ifdef CONFIG_X86
+	KUNIT_CASE(kunit_test_fault_null_dereference),
+	KUNIT_CASE(kunit_test_fault_const),
+	KUNIT_CASE(kunit_test_fault_rodata),
+#endif /* CONFIG_X86 */
+	{}
+};
+
+static struct kunit_suite kunit_x86_fault_test_suite = {
+	.name = "kunit_x86_fault",
+	.init = kunit_try_catch_test_init,
+	.test_cases = kunit_x86_fault_test_cases,
+};
+
 /*
  * Context for testing test managed resources
  * is_resource_initialized is used to test arbitrary resources
@@ -826,6 +938,7 @@ static struct kunit_suite kunit_current_test_suite = {
 
 kunit_test_suites(&kunit_try_catch_test_suite, &kunit_resource_test_suite,
 		  &kunit_log_test_suite, &kunit_status_test_suite,
-		  &kunit_current_test_suite, &kunit_device_test_suite);
+		  &kunit_current_test_suite, &kunit_device_test_suite,
+		  &kunit_x86_fault_test_suite);
 
 MODULE_LICENSE("GPL v2");
-- 
2.44.0

Re: [PATCH v1 8/8] kunit: Add tests for faults

[Kees Cook]

On Thu, Feb 29, 2024 at 06:04:09PM +0100, Mickaël Salaün wrote:
> The first test checks NULL pointer dereference and make sure it would
> result as a failed test.
> 
> The second and third tests check that read-only data is indeed read-only
> and trying to modify it would result as a failed test.
> 
> This kunit_x86_fault test suite is marked as skipped when run on a
> non-x86 native architecture.  It is then skipped on UML because such
> test would result to a kernel panic.
> 
> Tested with:
> ./tools/testing/kunit/kunit.py run --arch x86_64 kunit_x86_fault
> 
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: Shuah Khan <skhan@linuxfoundation.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>

If we can add some way to collect WARN/BUG output for examination, I
could rewrite most of LKDTM in KUnit! I really like this!

> ---
>  lib/kunit/kunit-test.c | 115 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 114 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
> index f7980ef236a3..57d8eff00c66 100644
> --- a/lib/kunit/kunit-test.c
> +++ b/lib/kunit/kunit-test.c
> @@ -10,6 +10,7 @@
>  #include <kunit/test-bug.h>
>  
>  #include <linux/device.h>
> +#include <linux/init.h>
>  #include <kunit/device.h>
>  
>  #include "string-stream.h"
> @@ -109,6 +110,117 @@ static struct kunit_suite kunit_try_catch_test_suite = {
>  	.test_cases = kunit_try_catch_test_cases,
>  };
>  
> +#ifdef CONFIG_X86

Why is this x86 specific?

-- 
Kees Cook

Re: [PATCH v1 8/8] kunit: Add tests for faults

[Mickaël Salaün]

On Thu, Feb 29, 2024 at 10:28:18AM -0800, Kees Cook wrote:
> On Thu, Feb 29, 2024 at 06:04:09PM +0100, Mickaël Salaün wrote:
> > The first test checks NULL pointer dereference and make sure it would
> > result as a failed test.
> > 
> > The second and third tests check that read-only data is indeed read-only
> > and trying to modify it would result as a failed test.
> > 
> > This kunit_x86_fault test suite is marked as skipped when run on a
> > non-x86 native architecture.  It is then skipped on UML because such
> > test would result to a kernel panic.
> > 
> > Tested with:
> > ./tools/testing/kunit/kunit.py run --arch x86_64 kunit_x86_fault
> > 
> > Cc: Brendan Higgins <brendanhiggins@google.com>
> > Cc: David Gow <davidgow@google.com>
> > Cc: Rae Moar <rmoar@google.com>
> > Cc: Shuah Khan <skhan@linuxfoundation.org>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> 
> If we can add some way to collect WARN/BUG output for examination, I
> could rewrite most of LKDTM in KUnit! I really like this!

Thanks!  About the WARN/BUG examination, I guess the easier way would be
to do in in user space by extending kunit_parser.py.

> 
> > ---
> >  lib/kunit/kunit-test.c | 115 ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 114 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
> > index f7980ef236a3..57d8eff00c66 100644
> > --- a/lib/kunit/kunit-test.c
> > +++ b/lib/kunit/kunit-test.c
> > @@ -10,6 +10,7 @@
> >  #include <kunit/test-bug.h>
> >  
> >  #include <linux/device.h>
> > +#include <linux/init.h>
> >  #include <kunit/device.h>
> >  
> >  #include "string-stream.h"
> > @@ -109,6 +110,117 @@ static struct kunit_suite kunit_try_catch_test_suite = {
> >  	.test_cases = kunit_try_catch_test_cases,
> >  };
> >  
> > +#ifdef CONFIG_X86
> 
> Why is this x86 specific?

Because I didn't test on other architecture, and it looks it crashed on
arm64. :)

I'll test on arm64 and change this condition with !CONFIG_UML.

> 
> -- 
> Kees Cook
> 

Re: [PATCH v1 8/8] kunit: Add tests for faults

[kernel test robot]

Hi Mickaël,

kernel test robot noticed the following build warnings:

[auto build test WARNING on d206a76d7d2726f3b096037f2079ce0bd3ba329b]

url:    https://github.com/intel-lab-lkp/linux/commits/Micka-l-Sala-n/kunit-Run-tests-when-the-kernel-is-fully-setup/20240301-011020
base:   d206a76d7d2726f3b096037f2079ce0bd3ba329b
patch link:    https://lore.kernel.org/r/20240229170409.365386-9-mic%40digikod.net
patch subject: [PATCH v1 8/8] kunit: Add tests for faults
config: x86_64-randconfig-122-20240301 (https://download.01.org/0day-ci/archive/20240302/202403020418.NnNnFElm-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240302/202403020418.NnNnFElm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403020418.NnNnFElm-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> lib/kunit/kunit-test.c:142:11: sparse: sparse: symbol 'test_const' was not declared. Should it be static?

vim +/test_const +142 lib/kunit/kunit-test.c

   141	
 > 142	const int test_const = 1;
   143	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 0/8] Run KUnit tests late and handle faults

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 3211 bytes --]

On Fri, 1 Mar 2024 at 01:04, Mickaël Salaün <mic@digikod.net> wrote:
>
> Hi,
>

Thanks very much. I think there's a lot going on in this series, and
it'd probably be easier to address if it were broken up a bit more.

To take things one at a time:

> This patch series moves KUnit test execution at the very end of kernel
> initialization, just before launching the init process.  This opens the
> way to test any kernel code in its normal state (i.e. fully
> initialized).

I like the general idea here, but there are a few things to keep in mind:
- We can already do this with tests built as modules.
- We have explicit support for testing __init code, so if we want to
keep that (and I think we do), we'll need to make sure that there
remains a way to run tests before __init.
- Behaviour changes here will need to be documented and tested well
across all tests and architectures, so it's not something I'd want to
land quickly.
- The requirement to have a root filesystem set up is another thing
we'll want to handle carefully.
- As-is, the patch seems to break arm64.

>
> This patch series also teaches KUnit to handle kthread faults as errors,
> and it brings a few related fixes and improvements.

These seem very good overall. I want to look at the last location
stuff in a bit more detail, but otherwise this is okay.

Personally, I'd like to see this split out into a separate series,
partly because I don't want to delay it while we sort the other parts
of this series out, and partly because I have some other changes to
the thread context stuff I think we need to make.

>
> New tests check NULL pointer dereference and read-only memory, which
> wasn't possible before.

These look interesting, but I don't like that they are listed as x86-specific.

>
> This is useful to test current kernel self-protection mechanisms or
> future ones such as Heki: https://github.com/heki-linux
>
> Regards,

Thanks again. I'll do a more detailed review of the individual patches
next week, but I'm excited to see this overall.

Cheers,
-- David


>
> Mickaël Salaün (8):
>   kunit: Run tests when the kernel is fully setup
>   kunit: Handle thread creation error
>   kunit: Fix kthread reference
>   kunit: Fix timeout message
>   kunit: Handle test faults
>   kunit: Fix KUNIT_SUCCESS() calls in iov_iter tests
>   kunit: Print last test location on fault
>   kunit: Add tests for faults
>
>  include/kunit/test.h                |  24 +++++-
>  include/kunit/try-catch.h           |   3 -
>  init/main.c                         |   4 +-
>  lib/bitfield_kunit.c                |   8 +-
>  lib/checksum_kunit.c                |   2 +-
>  lib/kunit/executor.c                |  81 ++++++++++++++------
>  lib/kunit/kunit-example-test.c      |   6 +-
>  lib/kunit/kunit-test.c              | 115 +++++++++++++++++++++++++++-
>  lib/kunit/try-catch.c               |  33 +++++---
>  lib/kunit_iov_iter.c                |  70 ++++++++---------
>  tools/testing/kunit/kunit_kernel.py |   6 +-
>  11 files changed, 261 insertions(+), 91 deletions(-)
>
>
> base-commit: d206a76d7d2726f3b096037f2079ce0bd3ba329b
> --
> 2.44.0
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]

Re: [PATCH v1 0/8] Run KUnit tests late and handle faults

[Mickaël Salaün]

On Fri, Mar 01, 2024 at 03:15:08PM +0800, David Gow wrote:
> On Fri, 1 Mar 2024 at 01:04, Mickaël Salaün <mic@digikod.net> wrote:
> >
> > Hi,
> >
> 
> Thanks very much. I think there's a lot going on in this series, and
> it'd probably be easier to address if it were broken up a bit more.
> 
> To take things one at a time:
> 
> > This patch series moves KUnit test execution at the very end of kernel
> > initialization, just before launching the init process.  This opens the
> > way to test any kernel code in its normal state (i.e. fully
> > initialized).
> 
> I like the general idea here, but there are a few things to keep in mind:
> - We can already do this with tests built as modules.
> - We have explicit support for testing __init code, so if we want to
> keep that (and I think we do), we'll need to make sure that there
> remains a way to run tests before __init.
> - Behaviour changes here will need to be documented and tested well
> across all tests and architectures, so it's not something I'd want to
> land quickly.
> - The requirement to have a root filesystem set up is another thing
> we'll want to handle carefully.
> - As-is, the patch seems to break arm64.

Fair, I'll remove this patch from the next series.

> 
> >
> > This patch series also teaches KUnit to handle kthread faults as errors,
> > and it brings a few related fixes and improvements.
> 
> These seem very good overall. I want to look at the last location
> stuff in a bit more detail, but otherwise this is okay.

Thanks!

> 
> Personally, I'd like to see this split out into a separate series,
> partly because I don't want to delay it while we sort the other parts
> of this series out, and partly because I have some other changes to
> the thread context stuff I think we need to make.

I'll do that today.

> 
> >
> > New tests check NULL pointer dereference and read-only memory, which
> > wasn't possible before.
> 
> These look interesting, but I don't like that they are listed as x86-specific.

I was reluctant to make it more broadly available because I only tested
on x86...

> 
> >
> > This is useful to test current kernel self-protection mechanisms or
> > future ones such as Heki: https://github.com/heki-linux
> >
> > Regards,
> 
> Thanks again. I'll do a more detailed review of the individual patches
> next week, but I'm excited to see this overall.

Good, you'll review the v2 then.

> 
> Cheers,
> -- David
> 
> 
> >
> > Mickaël Salaün (8):
> >   kunit: Run tests when the kernel is fully setup
> >   kunit: Handle thread creation error
> >   kunit: Fix kthread reference
> >   kunit: Fix timeout message
> >   kunit: Handle test faults
> >   kunit: Fix KUNIT_SUCCESS() calls in iov_iter tests
> >   kunit: Print last test location on fault
> >   kunit: Add tests for faults
> >
> >  include/kunit/test.h                |  24 +++++-
> >  include/kunit/try-catch.h           |   3 -
> >  init/main.c                         |   4 +-
> >  lib/bitfield_kunit.c                |   8 +-
> >  lib/checksum_kunit.c                |   2 +-
> >  lib/kunit/executor.c                |  81 ++++++++++++++------
> >  lib/kunit/kunit-example-test.c      |   6 +-
> >  lib/kunit/kunit-test.c              | 115 +++++++++++++++++++++++++++-
> >  lib/kunit/try-catch.c               |  33 +++++---
> >  lib/kunit_iov_iter.c                |  70 ++++++++---------
> >  tools/testing/kunit/kunit_kernel.py |   6 +-
> >  11 files changed, 261 insertions(+), 91 deletions(-)
> >
> >
> > base-commit: d206a76d7d2726f3b096037f2079ce0bd3ba329b
> > --
> > 2.44.0
> >