[PATCH] siphash: Convert selftest to KUnit

[PATCH] siphash: Convert selftest to KUnit

[Kees Cook]

Convert the siphash self-test to KUnit so it will be included in "all
KUnit tests" coverage, and can be run individually still:

$ ./tools/testing/kunit/kunit.py run siphash
...
[02:58:45] Starting KUnit Kernel (1/1)...
[02:58:45] ============================================================
[02:58:45] =================== siphash (1 subtest) ====================
[02:58:45] [PASSED] siphash_test
[02:58:45] ===================== [PASSED] siphash =====================
[02:58:45] ============================================================
[02:58:45] Testing complete. Ran 1 tests: passed: 1
[02:58:45] Elapsed time: 21.421s total, 4.306s configuring, 16.947s building, 0.148s running

Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc: David Gow <davidgow@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 MAINTAINERS                             |   2 +-
 lib/Kconfig.debug                       |  20 +--
 lib/Makefile                            |   2 +-
 lib/{test_siphash.c => siphash_kunit.c} | 165 ++++++++++--------------
 4 files changed, 83 insertions(+), 106 deletions(-)
 rename lib/{test_siphash.c => siphash_kunit.c} (60%)

diff --git a/MAINTAINERS b/MAINTAINERS
index cf0f18502372..037466b9a027 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18862,7 +18862,7 @@ M:	Jason A. Donenfeld <Jason@zx2c4.com>
 S:	Maintained
 F:	include/linux/siphash.h
 F:	lib/siphash.c
-F:	lib/test_siphash.c
+F:	lib/siphash_kunit.c
 
 SIS 190 ETHERNET DRIVER
 M:	Francois Romieu <romieu@fr.zoreil.com>
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 3fc7abffc7aa..65593675cd5a 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2247,15 +2247,6 @@ config TEST_RHASHTABLE
 
 	  If unsure, say N.
 
-config TEST_SIPHASH
-	tristate "Perform selftest on siphash functions"
-	help
-	  Enable this option to test the kernel's siphash (<linux/siphash.h>) hash
-	  functions on boot (or module load).
-
-	  This is intended to help people writing architecture-specific
-	  optimized versions.  If unsure, say N.
-
 config TEST_IDA
 	tristate "Perform selftest on IDA functions"
 
@@ -2583,6 +2574,17 @@ config HW_BREAKPOINT_KUNIT_TEST
 
 	  If unsure, say N.
 
+config SIPHASH_KUNIT_TEST
+	tristate "Perform selftest on siphash functions" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Enable this option to test the kernel's siphash (<linux/siphash.h>) hash
+	  functions on boot (or module load).
+
+	  This is intended to help people writing architecture-specific
+	  optimized versions.  If unsure, say N.
+
 config TEST_UDELAY
 	tristate "udelay test driver"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index 161d6a724ff7..bca02ac1adf8 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -62,7 +62,6 @@ obj-$(CONFIG_TEST_BITOPS) += test_bitops.o
 CFLAGS_test_bitops.o += -Werror
 obj-$(CONFIG_CPUMASK_KUNIT_TEST) += cpumask_kunit.o
 obj-$(CONFIG_TEST_SYSCTL) += test_sysctl.o
-obj-$(CONFIG_TEST_SIPHASH) += test_siphash.o
 obj-$(CONFIG_HASH_KUNIT_TEST) += test_hash.o
 obj-$(CONFIG_TEST_IDA) += test_ida.o
 obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o
@@ -380,6 +379,7 @@ obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
 CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
 obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
 obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
+obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
 
diff --git a/lib/test_siphash.c b/lib/siphash_kunit.c
similarity index 60%
rename from lib/test_siphash.c
rename to lib/siphash_kunit.c
index a96788d0141d..a3c697e8be35 100644
--- a/lib/test_siphash.c
+++ b/lib/siphash_kunit.c
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <kunit/test.h>
 #include <linux/siphash.h>
 #include <linux/kernel.h>
 #include <linux/string.h>
@@ -109,114 +110,88 @@ static const u32 test_vectors_hsiphash[64] = {
 };
 #endif
 
-static int __init siphash_test_init(void)
+#define chk(hash, vector, fmt...)			\
+	KUNIT_EXPECT_EQ_MSG(test, hash, vector, fmt)
+
+static void siphash_test(struct kunit *test)
 {
 	u8 in[64] __aligned(SIPHASH_ALIGNMENT);
 	u8 in_unaligned[65] __aligned(SIPHASH_ALIGNMENT);
 	u8 i;
-	int ret = 0;
 
 	for (i = 0; i < 64; ++i) {
 		in[i] = i;
 		in_unaligned[i + 1] = i;
-		if (siphash(in, i, &test_key_siphash) !=
-						test_vectors_siphash[i]) {
-			pr_info("siphash self-test aligned %u: FAIL\n", i + 1);
-			ret = -EINVAL;
-		}
-		if (siphash(in_unaligned + 1, i, &test_key_siphash) !=
-						test_vectors_siphash[i]) {
-			pr_info("siphash self-test unaligned %u: FAIL\n", i + 1);
-			ret = -EINVAL;
-		}
-		if (hsiphash(in, i, &test_key_hsiphash) !=
-						test_vectors_hsiphash[i]) {
-			pr_info("hsiphash self-test aligned %u: FAIL\n", i + 1);
-			ret = -EINVAL;
-		}
-		if (hsiphash(in_unaligned + 1, i, &test_key_hsiphash) !=
-						test_vectors_hsiphash[i]) {
-			pr_info("hsiphash self-test unaligned %u: FAIL\n", i + 1);
-			ret = -EINVAL;
-		}
-	}
-	if (siphash_1u64(0x0706050403020100ULL, &test_key_siphash) !=
-						test_vectors_siphash[8]) {
-		pr_info("siphash self-test 1u64: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (siphash_2u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL,
-			 &test_key_siphash) != test_vectors_siphash[16]) {
-		pr_info("siphash self-test 2u64: FAIL\n");
-		ret = -EINVAL;
+		chk(siphash(in, i, &test_key_siphash),
+		    test_vectors_siphash[i],
+		    "siphash self-test aligned %u: FAIL", i + 1);
+		chk(siphash(in_unaligned + 1, i, &test_key_siphash),
+		    test_vectors_siphash[i],
+		    "siphash self-test unaligned %u: FAIL", i + 1);
+		chk(hsiphash(in, i, &test_key_hsiphash),
+		    test_vectors_hsiphash[i],
+		    "hsiphash self-test aligned %u: FAIL", i + 1);
+		chk(hsiphash(in_unaligned + 1, i, &test_key_hsiphash),
+		    test_vectors_hsiphash[i],
+		    "hsiphash self-test unaligned %u: FAIL", i + 1);
 	}
-	if (siphash_3u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL,
-			 0x1716151413121110ULL, &test_key_siphash) !=
-						test_vectors_siphash[24]) {
-		pr_info("siphash self-test 3u64: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (siphash_4u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL,
+	chk(siphash_1u64(0x0706050403020100ULL, &test_key_siphash),
+	    test_vectors_siphash[8],
+	    "siphash self-test 1u64: FAIL");
+	chk(siphash_2u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL,
+			 &test_key_siphash),
+	    test_vectors_siphash[16],
+	    "siphash self-test 2u64: FAIL");
+	chk(siphash_3u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL,
+			 0x1716151413121110ULL, &test_key_siphash),
+	    test_vectors_siphash[24],
+	    "siphash self-test 3u64: FAIL");
+	chk(siphash_4u64(0x0706050403020100ULL, 0x0f0e0d0c0b0a0908ULL,
 			 0x1716151413121110ULL, 0x1f1e1d1c1b1a1918ULL,
-			 &test_key_siphash) != test_vectors_siphash[32]) {
-		pr_info("siphash self-test 4u64: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (siphash_1u32(0x03020100U, &test_key_siphash) !=
-						test_vectors_siphash[4]) {
-		pr_info("siphash self-test 1u32: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (siphash_2u32(0x03020100U, 0x07060504U, &test_key_siphash) !=
-						test_vectors_siphash[8]) {
-		pr_info("siphash self-test 2u32: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (siphash_3u32(0x03020100U, 0x07060504U,
-			 0x0b0a0908U, &test_key_siphash) !=
-						test_vectors_siphash[12]) {
-		pr_info("siphash self-test 3u32: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (siphash_4u32(0x03020100U, 0x07060504U,
-			 0x0b0a0908U, 0x0f0e0d0cU, &test_key_siphash) !=
-						test_vectors_siphash[16]) {
-		pr_info("siphash self-test 4u32: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (hsiphash_1u32(0x03020100U, &test_key_hsiphash) !=
-						test_vectors_hsiphash[4]) {
-		pr_info("hsiphash self-test 1u32: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (hsiphash_2u32(0x03020100U, 0x07060504U, &test_key_hsiphash) !=
-						test_vectors_hsiphash[8]) {
-		pr_info("hsiphash self-test 2u32: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (hsiphash_3u32(0x03020100U, 0x07060504U,
-			  0x0b0a0908U, &test_key_hsiphash) !=
-						test_vectors_hsiphash[12]) {
-		pr_info("hsiphash self-test 3u32: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (hsiphash_4u32(0x03020100U, 0x07060504U,
-			  0x0b0a0908U, 0x0f0e0d0cU, &test_key_hsiphash) !=
-						test_vectors_hsiphash[16]) {
-		pr_info("hsiphash self-test 4u32: FAIL\n");
-		ret = -EINVAL;
-	}
-	if (!ret)
-		pr_info("self-tests: pass\n");
-	return ret;
+			 &test_key_siphash),
+	    test_vectors_siphash[32],
+	    "siphash self-test 4u64: FAIL");
+	chk(siphash_1u32(0x03020100U, &test_key_siphash),
+	    test_vectors_siphash[4],
+	    "siphash self-test 1u32: FAIL");
+	chk(siphash_2u32(0x03020100U, 0x07060504U, &test_key_siphash),
+	    test_vectors_siphash[8],
+	    "siphash self-test 2u32: FAIL");
+	chk(siphash_3u32(0x03020100U, 0x07060504U,
+			 0x0b0a0908U, &test_key_siphash),
+	    test_vectors_siphash[12],
+	    "siphash self-test 3u32: FAIL");
+	chk(siphash_4u32(0x03020100U, 0x07060504U,
+			 0x0b0a0908U, 0x0f0e0d0cU, &test_key_siphash),
+	    test_vectors_siphash[16],
+	    "siphash self-test 4u32: FAIL");
+	chk(hsiphash_1u32(0x03020100U, &test_key_hsiphash),
+	    test_vectors_hsiphash[4],
+	    "hsiphash self-test 1u32: FAIL");
+	chk(hsiphash_2u32(0x03020100U, 0x07060504U, &test_key_hsiphash),
+	    test_vectors_hsiphash[8],
+	    "hsiphash self-test 2u32: FAIL");
+	chk(hsiphash_3u32(0x03020100U, 0x07060504U,
+			  0x0b0a0908U, &test_key_hsiphash),
+	    test_vectors_hsiphash[12],
+	    "hsiphash self-test 3u32: FAIL");
+	chk(hsiphash_4u32(0x03020100U, 0x07060504U,
+			  0x0b0a0908U, 0x0f0e0d0cU, &test_key_hsiphash),
+	    test_vectors_hsiphash[16],
+	    "hsiphash self-test 4u32: FAIL");
 }
 
-static void __exit siphash_test_exit(void)
-{
-}
+static struct kunit_case siphash_test_cases[] = {
+	KUNIT_CASE(siphash_test),
+	{}
+};
+
+static struct kunit_suite siphash_test_suite = {
+	.name = "siphash",
+	.test_cases = siphash_test_cases,
+};
 
-module_init(siphash_test_init);
-module_exit(siphash_test_exit);
+kunit_test_suite(siphash_test_suite);
 
 MODULE_AUTHOR("Jason A. Donenfeld <Jason@zx2c4.com>");
 MODULE_LICENSE("Dual BSD/GPL");
-- 
2.34.1

Re: [PATCH] siphash: Convert selftest to KUnit

[Jason A. Donenfeld]

On Tue, Oct 18, 2022 at 03:05:46AM -0700, Kees Cook wrote:
> Convert the siphash self-test to KUnit so it will be included in "all
> KUnit tests" coverage, and can be run individually still:
> 
> $ ./tools/testing/kunit/kunit.py run siphash
> ...
> [02:58:45] Starting KUnit Kernel (1/1)...
> [02:58:45] ============================================================
> [02:58:45] =================== siphash (1 subtest) ====================
> [02:58:45] [PASSED] siphash_test
> [02:58:45] ===================== [PASSED] siphash =====================
> [02:58:45] ============================================================
> [02:58:45] Testing complete. Ran 1 tests: passed: 1
> [02:58:45] Elapsed time: 21.421s total, 4.306s configuring, 16.947s building, 0.148s running
> 
> Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
> Cc: David Gow <davidgow@google.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>

I'll queue this up. Thanks for the conversion. Appears to work well.

Jason

Re: [PATCH] siphash: Convert selftest to KUnit

[Kees Cook]

On Tue, Oct 18, 2022 at 11:00:26AM -0600, Jason A. Donenfeld wrote:
> On Tue, Oct 18, 2022 at 03:05:46AM -0700, Kees Cook wrote:
> > Convert the siphash self-test to KUnit so it will be included in "all
> > KUnit tests" coverage, and can be run individually still:
> > 
> > $ ./tools/testing/kunit/kunit.py run siphash
> > ...
> > [02:58:45] Starting KUnit Kernel (1/1)...
> > [02:58:45] ============================================================
> > [02:58:45] =================== siphash (1 subtest) ====================
> > [02:58:45] [PASSED] siphash_test
> > [02:58:45] ===================== [PASSED] siphash =====================
> > [02:58:45] ============================================================
> > [02:58:45] Testing complete. Ran 1 tests: passed: 1
> > [02:58:45] Elapsed time: 21.421s total, 4.306s configuring, 16.947s building, 0.148s running
> > 
> > Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
> > Cc: David Gow <davidgow@google.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> 
> I'll queue this up. Thanks for the conversion. Appears to work well.

Cool! Thanks for looking it over. If we want to avoid some tree
collisions, I could carry it in my tree? I've got some other conversions
in progress. Though maybe this begs the question of "how should kunit
tests be ordered in the Kconfig and Makefile?" so that collisions are
obvious about how to order...

-- 
Kees Cook

Re: [PATCH] siphash: Convert selftest to KUnit

[Jason A. Donenfeld]

On Tue, Oct 18, 2022 at 11:13 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Oct 18, 2022 at 11:00:26AM -0600, Jason A. Donenfeld wrote:
> > On Tue, Oct 18, 2022 at 03:05:46AM -0700, Kees Cook wrote:
> > > Convert the siphash self-test to KUnit so it will be included in "all
> > > KUnit tests" coverage, and can be run individually still:
> > >
> > > $ ./tools/testing/kunit/kunit.py run siphash
> > > ...
> > > [02:58:45] Starting KUnit Kernel (1/1)...
> > > [02:58:45] ============================================================
> > > [02:58:45] =================== siphash (1 subtest) ====================
> > > [02:58:45] [PASSED] siphash_test
> > > [02:58:45] ===================== [PASSED] siphash =====================
> > > [02:58:45] ============================================================
> > > [02:58:45] Testing complete. Ran 1 tests: passed: 1
> > > [02:58:45] Elapsed time: 21.421s total, 4.306s configuring, 16.947s building, 0.148s running
> > >
> > > Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
> > > Cc: David Gow <davidgow@google.com>
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> >
> > I'll queue this up. Thanks for the conversion. Appears to work well.
>
> Cool! Thanks for looking it over. If we want to avoid some tree
> collisions, I could carry it in my tree? I've got some other conversions
> in progress. Though maybe this begs the question of "how should kunit
> tests be ordered in the Kconfig and Makefile?" so that collisions are
> obvious about how to order...

No problem:

Acked-by: Jason A. Donenfeld <Jason@zx2c4.com>

Please use a lowercase "convert" in the commit subject, like the other
siphash commits.

Jason

Re: [PATCH] siphash: Convert selftest to KUnit

[David Gow]

On Tue, Oct 18, 2022 at 6:05 PM Kees Cook <keescook@chromium.org> wrote:
>
> Convert the siphash self-test to KUnit so it will be included in "all
> KUnit tests" coverage, and can be run individually still:
>
> $ ./tools/testing/kunit/kunit.py run siphash
> ...
> [02:58:45] Starting KUnit Kernel (1/1)...
> [02:58:45] ============================================================
> [02:58:45] =================== siphash (1 subtest) ====================
> [02:58:45] [PASSED] siphash_test
> [02:58:45] ===================== [PASSED] siphash =====================
> [02:58:45] ============================================================
> [02:58:45] Testing complete. Ran 1 tests: passed: 1
> [02:58:45] Elapsed time: 21.421s total, 4.306s configuring, 16.947s building, 0.148s running
>
> Cc: "Jason A. Donenfeld" <Jason@zx2c4.com>
> Cc: David Gow <davidgow@google.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---

Works fine here, and looks good from a KUnit point-of-view.

Tested-by: David Gow <davidgow@google.com>

Cheers,
-- David