2024-03-01 19:43:48

by Mickaël Salaün

[permalink] [raw]
Subject: [PATCH v2 7/7] kunit: Add tests for fault

Add a test case to check NULL pointer dereference and make sure it would
result as a failed test.

The full kunit_fault test suite is marked as skipped when run on UML
because it would result to a kernel panic.

Tested with:
/tools/testing/kunit/kunit.py run --arch x86_64 kunit_fault
/tools/testing/kunit/kunit.py run --arch arm64 \
--cross_compile=aarch64-linux-gnu- kunit_fault

Cc: Brendan Higgins <[email protected]>
Cc: David Gow <[email protected]>
Cc: Rae Moar <[email protected]>
Cc: Shuah Khan <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---

Changes since v1:
* Removed the rodata and const test cases for now.
* Replace CONFIG_X86 check with !CONFIG_UML, and remove the "_x86"
references.
---
lib/kunit/kunit-test.c | 45 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
index f7980ef236a3..0fdca5fffaec 100644
--- a/lib/kunit/kunit-test.c
+++ b/lib/kunit/kunit-test.c
@@ -109,6 +109,48 @@ static struct kunit_suite kunit_try_catch_test_suite = {
.test_cases = kunit_try_catch_test_cases,
};

+#ifndef CONFIG_UML
+
+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);
+}
+
+#endif /* !CONFIG_UML */
+
+static struct kunit_case kunit_fault_test_cases[] = {
+#ifndef CONFIG_UML
+ KUNIT_CASE(kunit_test_fault_null_dereference),
+#endif /* !CONFIG_UML */
+ {}
+};
+
+static struct kunit_suite kunit_fault_test_suite = {
+ .name = "kunit_fault",
+ .init = kunit_try_catch_test_init,
+ .test_cases = kunit_fault_test_cases,
+};
+
/*
* Context for testing test managed resources
* is_resource_initialized is used to test arbitrary resources
@@ -826,6 +868,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_fault_test_suite);

MODULE_LICENSE("GPL v2");
--
2.44.0



2024-03-12 04:57:08

by David Gow

[permalink] [raw]
Subject: Re: [PATCH v2 7/7] kunit: Add tests for fault

On Sat, 2 Mar 2024 at 03:40, Mickaël Salaün <[email protected]> wrote:
>
> Add a test case to check NULL pointer dereference and make sure it would
> result as a failed test.
>
> The full kunit_fault test suite is marked as skipped when run on UML
> because it would result to a kernel panic.
>
> Tested with:
> ./tools/testing/kunit/kunit.py run --arch x86_64 kunit_fault
> ./tools/testing/kunit/kunit.py run --arch arm64 \
> --cross_compile=aarch64-linux-gnu- kunit_fault
>
> Cc: Brendan Higgins <[email protected]>
> Cc: David Gow <[email protected]>
> Cc: Rae Moar <[email protected]>
> Cc: Shuah Khan <[email protected]>
> Signed-off-by: Mickaël Salaün <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> ---
>
> Changes since v1:
> * Removed the rodata and const test cases for now.
> * Replace CONFIG_X86 check with !CONFIG_UML, and remove the "_x86"
> references.
> ---

I think UML _should_ be able to handle this with signal handlers, but
I tested it and agree that it's broken, so disabling for now makes
sense.

In general, I'd prefer to have an empty test which SKIP()s here, but
since the suite is empty, KUnit does that anyway, so this is fine.

Reviewed-by: David Gow <[email protected]>

Cheers,
-- David


> lib/kunit/kunit-test.c | 45 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
> index f7980ef236a3..0fdca5fffaec 100644
> --- a/lib/kunit/kunit-test.c
> +++ b/lib/kunit/kunit-test.c
> @@ -109,6 +109,48 @@ static struct kunit_suite kunit_try_catch_test_suite = {
> .test_cases = kunit_try_catch_test_cases,
> };
>
> +#ifndef CONFIG_UML
> +
> +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);
> +}
> +
> +#endif /* !CONFIG_UML */
> +
> +static struct kunit_case kunit_fault_test_cases[] = {
> +#ifndef CONFIG_UML
> + KUNIT_CASE(kunit_test_fault_null_dereference),
> +#endif /* !CONFIG_UML */
> + {}
> +};
> +
> +static struct kunit_suite kunit_fault_test_suite = {
> + .name = "kunit_fault",
> + .init = kunit_try_catch_test_init,
> + .test_cases = kunit_fault_test_cases,
> +};
> +
> /*
> * Context for testing test managed resources
> * is_resource_initialized is used to test arbitrary resources
> @@ -826,6 +868,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_fault_test_suite);
>
> MODULE_LICENSE("GPL v2");
> --
> 2.44.0
>


Attachments:
smime.p7s (3.92 kB)
S/MIME Cryptographic Signature