2024-03-19 10:55:57

by Mickaël Salaün

[permalink] [raw]
Subject: [PATCH v3 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: Rae Moar <[email protected]>
Cc: Shuah Khan <[email protected]>
Reviewed-by: David Gow <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---

Changes since v2:
* Add David's Reviewed-by.

Changes since v1:
* Remove 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-04-19 22:34:03

by Guenter Roeck

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

Hi,

On Tue, Mar 19, 2024 at 11:48:57AM +0100, Micka?l Sala?n 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
>

What is the rationale for adding those tests unconditionally whenever
CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system
because it concludes that it is pointless to continue testing
after the "Unable to handle kernel NULL pointer dereference" backtrace.
At the same time, it is all or nothing, meaning I can not disable
it but still run other kunit tests.

Guenter

2024-04-19 23:38:22

by Guenter Roeck

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

On Fri, Apr 19, 2024 at 03:33:49PM -0700, Guenter Roeck wrote:
> Hi,
>
> On Tue, Mar 19, 2024 at 11:48:57AM +0100, Micka?l Sala?n 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
> >
>
> What is the rationale for adding those tests unconditionally whenever
> CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system
> because it concludes that it is pointless to continue testing
> after the "Unable to handle kernel NULL pointer dereference" backtrace.
> At the same time, it is all or nothing, meaning I can not disable
> it but still run other kunit tests.
>

Oh, never mind. I just disabled CONFIG_KUNIT_TEST in my test bed
to "solve" the problem. I'll take that as one of those "unintended
consequences" items: Instead of more tests, there are fewer.

Guenter

2024-04-22 13:15:56

by Mickaël Salaün

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

On Fri, Apr 19, 2024 at 04:38:01PM -0700, Guenter Roeck wrote:
> On Fri, Apr 19, 2024 at 03:33:49PM -0700, Guenter Roeck wrote:
> > Hi,
> >
> > On Tue, Mar 19, 2024 at 11:48:57AM +0100, Mickaël Salaün 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
> > >
> >
> > What is the rationale for adding those tests unconditionally whenever
> > CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system
> > because it concludes that it is pointless to continue testing
> > after the "Unable to handle kernel NULL pointer dereference" backtrace.
> > At the same time, it is all or nothing, meaning I can not disable
> > it but still run other kunit tests.
> >

CONFIG_KUNIT_TEST is to test KUnit itself. Why does this messes up your
test system, and what is your test system? Is it related to the kernel
warning and then the message you previously sent?
https://lore.kernel.org/r/[email protected]
It seems David has a solution to suppress such warning.

>
> Oh, never mind. I just disabled CONFIG_KUNIT_TEST in my test bed
> to "solve" the problem. I'll take that as one of those "unintended
> consequences" items: Instead of more tests, there are fewer.
>
> Guenter
>

2024-04-22 13:36:22

by Guenter Roeck

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

On 4/22/24 06:08, Mickaël Salaün wrote:
> On Fri, Apr 19, 2024 at 04:38:01PM -0700, Guenter Roeck wrote:
>> On Fri, Apr 19, 2024 at 03:33:49PM -0700, Guenter Roeck wrote:
>>> Hi,
>>>
>>> On Tue, Mar 19, 2024 at 11:48:57AM +0100, Mickaël Salaün 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
>>>>
>>>
>>> What is the rationale for adding those tests unconditionally whenever
>>> CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system
>>> because it concludes that it is pointless to continue testing
>>> after the "Unable to handle kernel NULL pointer dereference" backtrace.
>>> At the same time, it is all or nothing, meaning I can not disable
>>> it but still run other kunit tests.
>>>
>
> CONFIG_KUNIT_TEST is to test KUnit itself. Why does this messes up your
> test system, and what is your test system? Is it related to the kernel
> warning and then the message you previously sent?

It is not a warning, it is a BUG which terminates the affected kernel thread.
NULL pointer dereferences are normally fatal, which is why I abort tests
if one is encountered. I am not going to start introducing code into my
scripts to ignore such warnings (or BUG messages) on a case by case basis;
this would be unmaintainable.

> https://lore.kernel.org/r/[email protected]
> It seems David has a solution to suppress such warning.
>

I don't think so. My series tried to suppress warning backtraces, not BUG
messages. BUG messages can not easily be suppressed since the reaction is
architecture specific and typically fatal.

As I said below, never mind, I just disabled CONFIG_KUNIT_TEST in my testing.

Guenter

>>
>> Oh, never mind. I just disabled CONFIG_KUNIT_TEST in my test bed
>> to "solve" the problem. I'll take that as one of those "unintended
>> consequences" items: Instead of more tests, there are fewer.
>>
>> Guenter
>>