linux-kernel - Re: [PATCH 3/4] kselftest_module.h: add struct rnd

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 30 Oct 2020 17:23:08 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Shuah Khan <shuah@...nel.org>, Kees Cook <keescook@...omium.org>,
        Willy Tarreau <w@....eu>, linux-kselftest@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Arpitha Raghunandan <98.arpi@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Brendan Higgins <brendanhiggins@...gle.com>
Subject: Re: [PATCH 3/4] kselftest_module.h: add struct rnd_state and seed
 parameter

On Sun 2020-10-25 22:48:41, Rasmus Villemoes wrote:
> Some test suites make use of random numbers to increase the test
> coverage when the test suite gets run on different machines and
> increase the chance of some corner case bug being discovered - and I'm
> planning on extending some existing ones in that direction as
> well. However, should a bug be found this way, it's important that the
> exact same series of tests can be repeated to verify the bug is
> fixed. That means the random numbers must be obtained
> deterministically from a generator private to the test module.
> 
> To avoid adding boilerplate to various test modules, put some logic
> into kselftest_module.h: If the module declares that it will use
> random numbers, add a "seed" module parameter. If not explicitly given
> when the module is loaded (or via kernel command line), obtain a
> random one. In either case, print the seed used, and repeat that
> information if there was at least one test failing.
> 
> Signed-off-by: Rasmus Villemoes <linux@...musvillemoes.dk>
> ---
>  tools/testing/selftests/kselftest_module.h | 35 ++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kselftest_module.h b/tools/testing/selftests/kselftest_module.h
> index c81c0b0c054befaf665b..43f3ca58fcd550b8ac83 100644
> --- a/tools/testing/selftests/kselftest_module.h
> +++ b/tools/testing/selftests/kselftest_module.h
> @@ -3,14 +3,31 @@
>  #define __KSELFTEST_MODULE_H
>  
>  #include <linux/module.h>
> +#include <linux/prandom.h>
> +#include <linux/random.h>
>  
>  /*
>   * Test framework for writing test modules to be loaded by kselftest.
>   * See Documentation/dev-tools/kselftest.rst for an example test module.
>   */
>  
> +/*
> + * If the test module makes use of random numbers, define KSTM_RANDOM
> + * to 1 before including this header. Then a module parameter "seed"
> + * will be defined. If not given, a random one will be obtained. In
> + * either case, the used seed is reported, so the exact same series of
> + * tests can be repeated by loading the module with that seed
> + * given.
> + */
> +
> +#ifndef KSTM_RANDOM
> +#define KSTM_RANDOM 0
> +#endif
> +
>  static unsigned int total_tests __initdata;
>  static unsigned int failed_tests __initdata;
> +static struct rnd_state rnd_state __initdata;
> +static u64 seed __initdata;
>  
>  #define KSTM_CHECK_ZERO(x) do {						\
>  	total_tests++;							\
> @@ -22,11 +39,13 @@ static unsigned int failed_tests __initdata;
>  
>  static inline int kstm_report(unsigned int total_tests, unsigned int failed_tests)
>  {
> -	if (failed_tests == 0)
> +	if (failed_tests == 0) {
>  		pr_info("all %u tests passed\n", total_tests);
> -	else
> +	} else {
>  		pr_warn("failed %u out of %u tests\n", failed_tests, total_tests);
> -
> +		if (KSTM_RANDOM)
> +			pr_info("random seed used was 0x%016llx\n", seed);

I have a bit mixed feelings about this. It is genial and dirty hack at the
same time ;-) Well, it is basically the same approach as with
IS_ENABLED(CONFIG_bla_bla).

Reviewed-by: Petr Mladek <pmladek@...e.com>

Best Regards,
Petr