linux-kernel - Re: [PATCH] kunit: test: Move fault tests behind KUNIT_FAULT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+GJov4SLN-=V0RrqkgwG6WN-DUwcsmtpH-fEzJ4+NyOfaDgow@mail.gmail.com>
Date: Tue, 23 Apr 2024 17:12:59 -0400
From: Rae Moar <rmoar@...gle.com>
To: David Gow <davidgow@...gle.com>
Cc: Mickaël Salaün <mic@...ikod.net>, 
	Shuah Khan <skhan@...uxfoundation.org>, Guenter Roeck <linux@...ck-us.net>, 
	kunit-dev@...glegroups.com, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kunit: test: Move fault tests behind KUNIT_FAULT_TEST
 Kconfig option

On Tue, Apr 23, 2024 at 5:08 AM David Gow <davidgow@...gle.com> wrote:
>
> The NULL dereference tests in kunit_fault deliberately trigger a kernel
> BUG(), and therefore print the associated stack trace, even when the
> test passes. This is both annoying (as it bloats the test output), and
> can confuse some test harnesses, which assume any BUG() is a failure.
>
> Allow these tests to be specifically disabled (without disabling all
> of KUnit's other tests), by placing them behind the
> CONFIG_KUNIT_FAULT_TEST Kconfig option. This is enabled by default, but
> can be set to 'n' to disable the test. An empty 'kunit_fault' suite is
> left behind, which will automatically be marked 'skipped'.
>
> As the fault tests already were disabled under UML (as they weren't
> compatible with its fault handling), we can simply adapt those
> conditions, and add a dependency on !UML for our new option.
>
> Suggested-by: Guenter Roeck <linux@...ck-us.net>
> Link: https://lore.kernel.org/all/928249cc-e027-4f7f-b43f-502f99a1ea63@roeck-us.net/
> Fixes: 82b0beff3497 ("kunit: Add tests for fault")
> Signed-off-by: David Gow <davidgow@...gle.com>

Hello!

This looks good to me. Very useful!

Reviewed-by: Rae Moar <rmoar@...gle.com>

Thanks!
-Rae

> ---
>  lib/kunit/Kconfig      | 11 +++++++++++
>  lib/kunit/kunit-test.c |  8 ++++----
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index 68a6daec0aef..34d7242d526d 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -24,6 +24,17 @@ config KUNIT_DEBUGFS
>           test suite, which allow users to see results of the last test suite
>           run that occurred.
>
> +config KUNIT_FAULT_TEST
> +       bool "Enable KUnit tests which print BUG stacktraces"
> +       depends on KUNIT_TEST
> +       depends on !UML
> +       default y
> +       help
> +         Enables fault handling tests for the KUnit framework. These tests may
> +         trigger a kernel BUG(), and the associated stack trace, even when they
> +         pass. If this conflicts with your test infrastrcture (or is confusing
> +         or annoying), they can be disabled by setting this to N.
> +
>  config KUNIT_TEST
>         tristate "KUnit test for KUnit" if !KUNIT_ALL_TESTS
>         default KUNIT_ALL_TESTS
> diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c
> index 0fdca5fffaec..e3412e0ca399 100644
> --- a/lib/kunit/kunit-test.c
> +++ b/lib/kunit/kunit-test.c
> @@ -109,7 +109,7 @@ static struct kunit_suite kunit_try_catch_test_suite = {
>         .test_cases = kunit_try_catch_test_cases,
>  };
>
> -#ifndef CONFIG_UML
> +#if IS_ENABLED(CONFIG_KUNIT_FAULT_TEST)
>
>  static void kunit_test_null_dereference(void *data)
>  {
> @@ -136,12 +136,12 @@ static void kunit_test_fault_null_dereference(struct kunit *test)
>         KUNIT_EXPECT_TRUE(test, ctx->function_called);
>  }
>
> -#endif /* !CONFIG_UML */
> +#endif /* CONFIG_KUNIT_FAULT_TEST */
>
>  static struct kunit_case kunit_fault_test_cases[] = {
> -#ifndef CONFIG_UML
> +#if IS_ENABLED(CONFIG_KUNIT_FAULT_TEST)
>         KUNIT_CASE(kunit_test_fault_null_dereference),
> -#endif /* !CONFIG_UML */
> +#endif /* CONFIG_KUNIT_FAULT_TEST */
>         {}
>  };
>
> --
> 2.44.0.769.g3c40516874-goog
>