* [PATCH] lib: stackinit: Convert to KUnit
@ 2022-02-24 5:51 Kees Cook
2022-02-24 19:43 ` Daniel Latypov
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Kees Cook @ 2022-02-24 5:51 UTC (permalink / raw)
To: David Gow
Cc: Kees Cook, Daniel Latypov, Arnd Bergmann, Nathan Chancellor,
Nick Desaulniers, linux-kernel, KUnit Development, llvm,
linux-hardening
Convert to running under Kunit (and retain being able to run stand-alone
too). Building under Clang (or GCC 12) with CONFIG_INIT_STACK_ALL_ZERO=y,
this now passes as expected:
$ ./tools/testing/kunit/kunit.py config --make_option LLVM=1
$ ./tools/testing/kunit/kunit.py run overflow --make_option LLVM=1 \
--kconfig_add CONFIG_INIT_STACK_ALL_ZERO=y
...
[09:47:28] ================= stackinit (65 subtests) ==================
[09:47:28] [PASSED] test_u8_zero
[09:47:28] [PASSED] test_u16_zero
[09:47:28] [PASSED] test_u32_zero
[09:47:28] [PASSED] test_u64_zero
[09:47:28] [PASSED] test_char_array_zero
[09:47:28] [PASSED] test_small_hole_zero
[09:47:28] [PASSED] test_big_hole_zero
[09:47:28] [PASSED] test_trailing_hole_zero
[09:47:28] [PASSED] test_packed_zero
[09:47:28] [PASSED] test_small_hole_dynamic_partial
[09:47:28] [PASSED] test_big_hole_dynamic_partial
[09:47:28] [PASSED] test_trailing_hole_dynamic_partial
[09:47:28] [PASSED] test_packed_dynamic_partial
[09:47:28] [PASSED] test_small_hole_assigned_dynamic_partial
[09:47:28] [PASSED] test_big_hole_assigned_dynamic_partial
[09:47:28] [PASSED] test_trailing_hole_assigned_dynamic_partial
[09:47:28] [PASSED] test_packed_assigned_dynamic_partial
[09:47:28] [PASSED] test_small_hole_static_partial
[09:47:28] [PASSED] test_big_hole_static_partial
[09:47:28] [PASSED] test_trailing_hole_static_partial
[09:47:28] [PASSED] test_packed_static_partial
[09:47:28] [PASSED] test_small_hole_static_all
[09:47:28] [PASSED] test_big_hole_static_all
[09:47:28] [PASSED] test_trailing_hole_static_all
[09:47:28] [PASSED] test_packed_static_all
[09:47:28] [PASSED] test_small_hole_dynamic_all
[09:47:28] [PASSED] test_big_hole_dynamic_all
[09:47:28] [PASSED] test_trailing_hole_dynamic_all
[09:47:28] [PASSED] test_packed_dynamic_all
[09:47:28] [PASSED] test_small_hole_runtime_partial
[09:47:28] [PASSED] test_big_hole_runtime_partial
[09:47:28] [PASSED] test_trailing_hole_runtime_partial
[09:47:28] [PASSED] test_packed_runtime_partial
[09:47:28] [PASSED] test_small_hole_runtime_all
[09:47:28] [PASSED] test_big_hole_runtime_all
[09:47:28] [PASSED] test_trailing_hole_runtime_all
[09:47:28] [PASSED] test_packed_runtime_all
[09:47:28] [PASSED] test_small_hole_assigned_static_partial
[09:47:28] [PASSED] test_big_hole_assigned_static_partial
[09:47:28] [PASSED] test_trailing_hole_assigned_static_partial
[09:47:28] [PASSED] test_packed_assigned_static_partial
[09:47:28] [PASSED] test_small_hole_assigned_static_all
[09:47:28] [PASSED] test_big_hole_assigned_static_all
[09:47:28] [PASSED] test_trailing_hole_assigned_static_all
[09:47:28] [PASSED] test_packed_assigned_static_all
[09:47:28] [PASSED] test_small_hole_assigned_dynamic_all
[09:47:28] [PASSED] test_big_hole_assigned_dynamic_all
[09:47:28] [PASSED] test_trailing_hole_assigned_dynamic_all
[09:47:28] [PASSED] test_packed_assigned_dynamic_all
[09:47:28] [SKIPPED] test_small_hole_assigned_copy
[09:47:28] [SKIPPED] test_big_hole_assigned_copy
[09:47:28] [SKIPPED] test_trailing_hole_assigned_copy
[09:47:28] [PASSED] test_packed_assigned_copy
[09:47:28] [PASSED] test_u8_none
[09:47:28] [PASSED] test_u16_none
[09:47:28] [PASSED] test_u32_none
[09:47:28] [PASSED] test_u64_none
[09:47:28] [PASSED] test_char_array_none
[09:47:28] [SKIPPED] test_switch_1_none
[09:47:28] [SKIPPED] test_switch_2_none
[09:47:28] [PASSED] test_small_hole_none
[09:47:28] [PASSED] test_big_hole_none
[09:47:28] [PASSED] test_trailing_hole_none
[09:47:28] [PASSED] test_packed_none
[09:47:28] [PASSED] test_user
[09:47:28] ==================== [PASSED] stackinit ====================
[09:47:28] ============================================================
[09:47:28] Testing complete. Passed: 60, Failed: 0, Crashed: 0, Skipped: 5, Errors: 0
[09:47:28] Elapsed time: 4.192s total, 0.001s configuring, 4.070s building, 0.102s running
Cc: David Gow <davidgow@google.com>
Cc: Daniel Latypov <dlatypov@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
lib/Kconfig.debug | 26 +-
lib/Makefile | 4 +-
lib/{test_stackinit.c => stackinit_kunit.c} | 249 ++++++++++++--------
3 files changed, 168 insertions(+), 111 deletions(-)
rename lib/{test_stackinit.c => stackinit_kunit.c} (73%)
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 14d90d03bc8d..ea4415275563 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2511,6 +2511,21 @@ config OVERFLOW_KUNIT_TEST
If unsure, say N.
+config STACKINIT_KUNIT_TEST
+ tristate "Test level of stack variable initialization" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Test if the kernel is zero-initializing stack variables and
+ padding. Coverage is controlled by compiler flags,
+ CONFIG_GCC_PLUGIN_STRUCTLEAK, CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF,
+ or CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
+
+ For more information on KUnit and unit tests in general please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+ If unsure, say N.
+
config TEST_UDELAY
tristate "udelay test driver"
help
@@ -2602,17 +2617,6 @@ config TEST_OBJAGG
Enable this option to test object aggregation manager on boot
(or module load).
-
-config TEST_STACKINIT
- tristate "Test level of stack variable initialization"
- help
- Test if the kernel is zero-initializing stack variables and
- padding. Coverage is controlled by compiler flags,
- CONFIG_GCC_PLUGIN_STRUCTLEAK, CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF,
- or CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
-
- If unsure, say N.
-
config TEST_MEMINIT
tristate "Test heap/page initialization"
help
diff --git a/lib/Makefile b/lib/Makefile
index fdfcbfaff32f..353bc09ce38d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -93,8 +93,6 @@ obj-$(CONFIG_TEST_KMOD) += test_kmod.o
obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += test_debug_virtual.o
obj-$(CONFIG_TEST_MEMCAT_P) += test_memcat_p.o
obj-$(CONFIG_TEST_OBJAGG) += test_objagg.o
-CFLAGS_test_stackinit.o += $(call cc-disable-warning, switch-unreachable)
-obj-$(CONFIG_TEST_STACKINIT) += test_stackinit.o
obj-$(CONFIG_TEST_BLACKHOLE_DEV) += test_blackhole_dev.o
obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
@@ -363,6 +361,8 @@ obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
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_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
diff --git a/lib/test_stackinit.c b/lib/stackinit_kunit.c
similarity index 73%
rename from lib/test_stackinit.c
rename to lib/stackinit_kunit.c
index a3c74e6a21ff..72c7d4cb8ed2 100644
--- a/lib/test_stackinit.c
+++ b/lib/stackinit_kunit.c
@@ -2,14 +2,23 @@
/*
* Test cases for compiler-based stack variable zeroing via
* -ftrivial-auto-var-init={zero,pattern} or CONFIG_GCC_PLUGIN_STRUCTLEAK*.
+ * For example, see:
+ * https://www.kernel.org/doc/html/latest/dev-tools/kunit/kunit-tool.html#configuring-building-and-running-tests
+ * ./tools/testing/kunit/kunit.py config --make_option LLVM=1
+ * ./tools/testing/kunit/kunit.py run overflow [--raw_output] \
+ * --make_option LLVM=1 \
+ * --kconfig_add CONFIG_INIT_STACK_ALL_ZERO=y
*
* External build example:
* clang -O2 -Wall -ftrivial-auto-var-init=pattern \
- * -o test_stackinit test_stackinit.c
+ * -o stackinit_kunit stackinit_kunit.c
+ * ./stackinit_kunit
+ *
*/
#ifdef __KERNEL__
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <kunit/test.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/module.h>
@@ -18,21 +27,55 @@
#else
/* Userspace headers. */
+#define _GNU_SOURCE
#include <stdio.h>
#include <stdint.h>
+#include <stdlib.h>
#include <string.h>
#include <stdbool.h>
#include <errno.h>
#include <sys/types.h>
/* Linux kernel-ism stubs for stand-alone userspace build. */
-#define KBUILD_MODNAME "stackinit"
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#define pr_err(fmt, ...) fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_warn(fmt, ...) fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_info(fmt, ...) fprintf(stdout, pr_fmt(fmt), ##__VA_ARGS__)
-#define __init /**/
-#define __exit /**/
+#define TEST_PASS 0
+#define TEST_SKIP 1
+#define TEST_FAIL 2
+struct kunit {
+ int status;
+ char *msg;
+};
+struct kunit_case {
+ void (*run_case)(struct kunit *test);
+ const char *name;
+};
+struct kunit_suite {
+ const char *name;
+ const struct kunit_case *test_cases;
+};
+#define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
+
+#define KUNIT_ASSERT_TRUE_MSG(test, expr, fmt, ...) \
+do { \
+ if (!(expr)) { \
+ if (test->status != TEST_SKIP) \
+ test->status = TEST_FAIL; \
+ if (test->msg) \
+ free(test->msg); \
+ asprintf(&test->msg, fmt, ##__VA_ARGS__); \
+ } \
+} while (0)
+
+#define KUNIT_ASSERT_EQ_MSG(test, left, right, fmt, ...) \
+ KUNIT_ASSERT_TRUE_MSG(test, (left) == (right), fmt, ##__VA_ARGS__)
+
+#define kunit_skip(test, fmt, ...) \
+do { \
+ test->status = TEST_SKIP; \
+ if (test->msg) \
+ free(test->msg); \
+ asprintf(&test->msg, fmt, ##__VA_ARGS__); \
+} while (0)
+
#define __user /**/
#define noinline __attribute__((__noinline__))
#define __aligned(x) __attribute__((__aligned__(x)))
@@ -59,16 +102,44 @@ typedef uint16_t u16;
typedef uint32_t u32;
typedef uint64_t u64;
-#define module_init(func) static int (*do_init)(void) = func
-#define module_exit(func) static void (*do_exit)(void) = func
-#define MODULE_LICENSE(str) int main(void) { \
- int rc; \
- /* License: str */ \
- rc = do_init(); \
- if (rc == 0) \
- do_exit(); \
- return rc; \
- }
+#define MODULE_LICENSE(str) /* */
+
+int do_kunit_test_suite(struct kunit_suite *suite)
+{
+ const struct kunit_case *test_case;
+ int rc = 0;
+
+ for (test_case = suite->test_cases; test_case->run_case; test_case++) {
+ struct kunit test = { };
+
+ test_case->run_case(&test);
+ switch (test.status) {
+ default:
+ case TEST_FAIL:
+ fprintf(stderr, "FAIL: %s%s%s", test_case->name,
+ test.msg ? ": " : "",
+ test.msg ?: "\n");
+ rc = 1;
+ break;
+ case TEST_SKIP:
+ fprintf(stdout, "XFAIL: %s%s%s", test_case->name,
+ test.msg ? ": " : "",
+ test.msg ?: "\n");
+ break;
+ case TEST_PASS:
+ fprintf(stdout, "ok: %s\n", test_case->name);
+ break;
+ }
+ if (test.msg)
+ free(test.msg);
+ }
+ return rc;
+}
+
+#define kunit_test_suite(suite) \
+int main(void) { \
+ return do_kunit_test_suite(&(suite)); \
+}
#endif /* __KERNEL__ */
@@ -201,7 +272,7 @@ static bool range_contains(char *haystack_start, size_t haystack_size,
*/
#define DEFINE_TEST_DRIVER(name, var_type, which, xfail) \
/* Returns 0 on success, 1 on failure. */ \
-static noinline __init int test_ ## name (void) \
+static noinline void test_ ## name (struct kunit *test) \
{ \
var_type zero INIT_CLONE_ ## which; \
int ignored; \
@@ -220,10 +291,8 @@ static noinline __init int test_ ## name (void) \
/* Verify all bytes overwritten with 0xFF. */ \
for (sum = 0, i = 0; i < target_size; i++) \
sum += (check_buf[i] != 0xFF); \
- if (sum) { \
- pr_err(#name ": leaf fill was not 0xFF!?\n"); \
- return 1; \
- } \
+ KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
+ "leaf fill was not 0xFF!?\n"); \
/* Clear entire check buffer for later bit tests. */ \
memset(check_buf, 0x00, sizeof(check_buf)); \
/* Extract stack-defined variable contents. */ \
@@ -231,32 +300,29 @@ static noinline __init int test_ ## name (void) \
FETCH_ARG_ ## which(zero)); \
\
/* Validate that compiler lined up fill and target. */ \
- if (!range_contains(fill_start, fill_size, \
- target_start, target_size)) { \
- pr_err(#name ": stack fill missed target!?\n"); \
- pr_err(#name ": fill %zu wide\n", fill_size); \
- pr_err(#name ": target offset by %d\n", \
- (int)((ssize_t)(uintptr_t)fill_start - \
- (ssize_t)(uintptr_t)target_start)); \
- return 1; \
- } \
+ KUNIT_ASSERT_TRUE_MSG(test, \
+ range_contains(fill_start, fill_size, \
+ target_start, target_size), \
+ "stack fill missed target!? " \
+ "(fill %zu wide, target offset by %d)\n", \
+ fill_size, \
+ (int)((ssize_t)(uintptr_t)fill_start - \
+ (ssize_t)(uintptr_t)target_start)); \
\
/* Look for any bytes still 0xFF in check region. */ \
for (sum = 0, i = 0; i < target_size; i++) \
sum += (check_buf[i] == 0xFF); \
\
- if (sum == 0) { \
- pr_info(#name " ok\n"); \
- return 0; \
- } else { \
- pr_warn(#name " %sFAIL (uninit bytes: %d)\n", \
- (xfail) ? "X" : "", sum); \
- return (xfail) ? 0 : 1; \
- } \
+ if (sum != 0 && xfail) \
+ kunit_skip(test, \
+ "XFAIL uninit bytes: %d\n", \
+ sum); \
+ KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
+ "uninit bytes: %d\n", sum); \
}
#define DEFINE_TEST(name, var_type, which, init_level, xfail) \
/* no-op to force compiler into ignoring "uninitialized" vars */\
-static noinline __init DO_NOTHING_TYPE_ ## which(var_type) \
+static noinline DO_NOTHING_TYPE_ ## which(var_type) \
do_nothing_ ## name(var_type *ptr) \
{ \
/* Will always be true, but compiler doesn't know. */ \
@@ -265,9 +331,8 @@ do_nothing_ ## name(var_type *ptr) \
else \
return DO_NOTHING_RETURN_ ## which(ptr + 1); \
} \
-static noinline __init int leaf_ ## name(unsigned long sp, \
- bool fill, \
- var_type *arg) \
+static noinline int leaf_ ## name(unsigned long sp, bool fill, \
+ var_type *arg) \
{ \
char buf[VAR_BUFFER]; \
var_type var \
@@ -398,7 +463,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
* This is intentionally unreachable. To silence the
* warning, build with -Wno-switch-unreachable
*/
- uint64_t var;
+ uint64_t var[10];
case 1:
target_start = &var;
@@ -423,19 +488,19 @@ static int noinline __leaf_switch_none(int path, bool fill)
memcpy(check_buf, target_start, target_size);
break;
default:
- var = 5;
- return var & forced_mask;
+ var[1] = 5;
+ return var[1] & forced_mask;
}
return 0;
}
-static noinline __init int leaf_switch_1_none(unsigned long sp, bool fill,
+static noinline int leaf_switch_1_none(unsigned long sp, bool fill,
uint64_t *arg)
{
return __leaf_switch_none(1, fill);
}
-static noinline __init int leaf_switch_2_none(unsigned long sp, bool fill,
+static noinline int leaf_switch_2_none(unsigned long sp, bool fill,
uint64_t *arg)
{
return __leaf_switch_none(2, fill);
@@ -450,65 +515,53 @@ static noinline __init int leaf_switch_2_none(unsigned long sp, bool fill,
DEFINE_TEST_DRIVER(switch_1_none, uint64_t, SCALAR, XFAIL);
DEFINE_TEST_DRIVER(switch_2_none, uint64_t, SCALAR, XFAIL);
-static int __init test_stackinit_init(void)
-{
- unsigned int failures = 0;
-
-#define test_scalars(init) do { \
- failures += test_u8_ ## init (); \
- failures += test_u16_ ## init (); \
- failures += test_u32_ ## init (); \
- failures += test_u64_ ## init (); \
- failures += test_char_array_ ## init (); \
- } while (0)
+#define KUNIT_test_scalars(init) \
+ KUNIT_CASE(test_u8_ ## init), \
+ KUNIT_CASE(test_u16_ ## init), \
+ KUNIT_CASE(test_u32_ ## init), \
+ KUNIT_CASE(test_u64_ ## init), \
+ KUNIT_CASE(test_char_array_ ## init)
-#define test_structs(init) do { \
- failures += test_small_hole_ ## init (); \
- failures += test_big_hole_ ## init (); \
- failures += test_trailing_hole_ ## init (); \
- failures += test_packed_ ## init (); \
- } while (0)
+#define KUNIT_test_structs(init) \
+ KUNIT_CASE(test_small_hole_ ## init), \
+ KUNIT_CASE(test_big_hole_ ## init), \
+ KUNIT_CASE(test_trailing_hole_ ## init),\
+ KUNIT_CASE(test_packed_ ## init) \
+static struct kunit_case stackinit_test_cases[] = {
/* These are explicitly initialized and should always pass. */
- test_scalars(zero);
- test_structs(zero);
+ KUNIT_test_scalars(zero),
+ KUNIT_test_structs(zero),
/* Padding here appears to be accidentally always initialized? */
- test_structs(dynamic_partial);
- test_structs(assigned_dynamic_partial);
+ KUNIT_test_structs(dynamic_partial),
+ KUNIT_test_structs(assigned_dynamic_partial),
/* Padding initialization depends on compiler behaviors. */
- test_structs(static_partial);
- test_structs(static_all);
- test_structs(dynamic_all);
- test_structs(runtime_partial);
- test_structs(runtime_all);
- test_structs(assigned_static_partial);
- test_structs(assigned_static_all);
- test_structs(assigned_dynamic_all);
+ KUNIT_test_structs(static_partial),
+ KUNIT_test_structs(static_all),
+ KUNIT_test_structs(dynamic_all),
+ KUNIT_test_structs(runtime_partial),
+ KUNIT_test_structs(runtime_all),
+ KUNIT_test_structs(assigned_static_partial),
+ KUNIT_test_structs(assigned_static_all),
+ KUNIT_test_structs(assigned_dynamic_all),
/* Everything fails this since it effectively performs a memcpy(). */
- test_structs(assigned_copy);
-
+ KUNIT_test_structs(assigned_copy),
/* STRUCTLEAK_BYREF_ALL should cover everything from here down. */
- test_scalars(none);
- failures += test_switch_1_none();
- failures += test_switch_2_none();
-
+ KUNIT_test_scalars(none),
+ KUNIT_CASE(test_switch_1_none),
+ KUNIT_CASE(test_switch_2_none),
/* STRUCTLEAK_BYREF should cover from here down. */
- test_structs(none);
-
+ KUNIT_test_structs(none),
/* STRUCTLEAK will only cover this. */
- failures += test_user();
-
- if (failures == 0)
- pr_info("all tests passed!\n");
- else
- pr_err("failures: %u\n", failures);
+ KUNIT_CASE(test_user),
+ {}
+};
- return failures ? -EINVAL : 0;
-}
-module_init(test_stackinit_init);
+static struct kunit_suite stackinit_test_suite = {
+ .name = "stackinit",
+ .test_cases = stackinit_test_cases,
+};
-static void __exit test_stackinit_exit(void)
-{ }
-module_exit(test_stackinit_exit);
+kunit_test_suite(stackinit_test_suite);
MODULE_LICENSE("GPL");
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] lib: stackinit: Convert to KUnit
2022-02-24 5:51 [PATCH] lib: stackinit: Convert to KUnit Kees Cook
@ 2022-02-24 19:43 ` Daniel Latypov
2022-02-25 1:17 ` Kees Cook
2022-02-25 6:53 ` David Gow
2022-03-22 14:30 ` Geert Uytterhoeven
2 siblings, 1 reply; 6+ messages in thread
From: Daniel Latypov @ 2022-02-24 19:43 UTC (permalink / raw)
To: Kees Cook
Cc: David Gow, Arnd Bergmann, Nathan Chancellor, Nick Desaulniers,
linux-kernel, KUnit Development, llvm, linux-hardening
On Wed, Feb 23, 2022 at 9:51 PM Kees Cook <keescook@chromium.org> wrote:
>
<snip>
> /* Userspace headers. */
> +#define _GNU_SOURCE
> #include <stdio.h>
> #include <stdint.h>
> +#include <stdlib.h>
> #include <string.h>
> #include <stdbool.h>
> #include <errno.h>
> #include <sys/types.h>
>
> /* Linux kernel-ism stubs for stand-alone userspace build. */
This is neat and esp. so that it works.
But may I ask, what's the value of using this vs UML?
Given this has changed into mainly just a KUnit-compatibility layer,
it feels like it can maybe live as a standalone file, if there's ever
interest in doing this for other tests.
It feels like something that will never quite be "supported", but I
find it neat enough I'd have fun sending some patches to make it more
realistic.
> -#define KBUILD_MODNAME "stackinit"
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -#define pr_err(fmt, ...) fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__)
> -#define pr_warn(fmt, ...) fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__)
> -#define pr_info(fmt, ...) fprintf(stdout, pr_fmt(fmt), ##__VA_ARGS__)
> -#define __init /**/
> -#define __exit /**/
> +#define TEST_PASS 0
> +#define TEST_SKIP 1
> +#define TEST_FAIL 2
> +struct kunit {
> + int status;
> + char *msg;
> +};
> +struct kunit_case {
> + void (*run_case)(struct kunit *test);
> + const char *name;
> +};
> +struct kunit_suite {
> + const char *name;
> + const struct kunit_case *test_cases;
> +};
> +#define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
> +
> +#define KUNIT_ASSERT_TRUE_MSG(test, expr, fmt, ...) \
> +do { \
> + if (!(expr)) { \
> + if (test->status != TEST_SKIP) \
> + test->status = TEST_FAIL; \
> + if (test->msg) \
> + free(test->msg); \
> + asprintf(&test->msg, fmt, ##__VA_ARGS__); \
> + } \
> +} while (0)
This looks more like KUNIT_EXPECT_TRUE_MSG(), since this macro won't
abort the test if the expectation fails.
Looking at the code, it looks like we do want the ability to abort.
Perhaps we can do what Googletest does in C++ and just add in a `return;`?
It has some annoying implications, like using them from helper
functions doesn't work as one would expect.
But people seem to be doing fine with that tradeoff in C++ land.
> +
> +#define KUNIT_ASSERT_EQ_MSG(test, left, right, fmt, ...) \
> + KUNIT_ASSERT_TRUE_MSG(test, (left) == (right), fmt, ##__VA_ARGS__)
Very optional:
It might be nice to show the expressions automatically on failure.
We could implement that via something like
KUNIT_ASSERT_TRUE_MSG(test, (left) == (right), #left " != " #right ":
" fmt, ##__VA_ARGS__);
E.g.
KUNIT_ASSERT_EQ_MSG(test, 2+2, 5, "math is broken")
=> 2+2 != 5: math is broken
But I can see that being a bit too complicated to want to do here.
And the failure messages we had before are already decent at giving context.
> +
> +#define kunit_skip(test, fmt, ...) \
> +do { \
> + test->status = TEST_SKIP; \
> + if (test->msg) \
> + free(test->msg); \
> + asprintf(&test->msg, fmt, ##__VA_ARGS__); \
> +} while (0)
Similarly, this has no control flow implications, so the current
semantics match kunit_mark_skipped().
But looking at the code, I think we want to abort early here too.
> +
> #define __user /**/
> #define noinline __attribute__((__noinline__))
> #define __aligned(x) __attribute__((__aligned__(x)))
> @@ -59,16 +102,44 @@ typedef uint16_t u16;
> typedef uint32_t u32;
> typedef uint64_t u64;
>
> -#define module_init(func) static int (*do_init)(void) = func
> -#define module_exit(func) static void (*do_exit)(void) = func
> -#define MODULE_LICENSE(str) int main(void) { \
> - int rc; \
> - /* License: str */ \
> - rc = do_init(); \
> - if (rc == 0) \
> - do_exit(); \
> - return rc; \
> - }
> +#define MODULE_LICENSE(str) /* */
> +
> +int do_kunit_test_suite(struct kunit_suite *suite)
> +{
> + const struct kunit_case *test_case;
> + int rc = 0;
> +
> + for (test_case = suite->test_cases; test_case->run_case; test_case++) {
> + struct kunit test = { };
> +
> + test_case->run_case(&test);
> + switch (test.status) {
> + default:
> + case TEST_FAIL:
> + fprintf(stderr, "FAIL: %s%s%s", test_case->name,
> + test.msg ? ": " : "",
> + test.msg ?: "\n");
> + rc = 1;
> + break;
> + case TEST_SKIP:
> + fprintf(stdout, "XFAIL: %s%s%s", test_case->name,
> + test.msg ? ": " : "",
> + test.msg ?: "\n");
> + break;
> + case TEST_PASS:
> + fprintf(stdout, "ok: %s\n", test_case->name);
> + break;
> + }
> + if (test.msg)
> + free(test.msg);
> + }
> + return rc;
> +}
> +
> +#define kunit_test_suite(suite) \
> +int main(void) { \
> + return do_kunit_test_suite(&(suite)); \
> +}
very optional:
emulating kunit_test_suites() might be more future-proof here, if we
ever want to setup more suites.
There's little reason to do so right now given the lack of init and
exit support.
Like the stuff above, it wouldn't be hard to do, but I can see it not
being worth the extra code, i.e.
#define kunit_test_suites(suites...) \
int main(void) {
static struct kunit_suite *suites =[] = { __VA_ARGS__ };
int i, ret = 0;
for (i = 0; i < ARRAY_SIZE(suites); ++i)
ret += do_kunit_test_suite(suites[i]);
return ret;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lib: stackinit: Convert to KUnit
2022-02-24 19:43 ` Daniel Latypov
@ 2022-02-25 1:17 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2022-02-25 1:17 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Arnd Bergmann, Nathan Chancellor, Nick Desaulniers,
linux-kernel, KUnit Development, llvm, linux-hardening
On Thu, Feb 24, 2022 at 11:43:40AM -0800, Daniel Latypov wrote:
> On Wed, Feb 23, 2022 at 9:51 PM Kees Cook <keescook@chromium.org> wrote:
> >
>
> <snip>
>
>
> > /* Userspace headers. */
> > +#define _GNU_SOURCE
> > #include <stdio.h>
> > #include <stdint.h>
> > +#include <stdlib.h>
> > #include <string.h>
> > #include <stdbool.h>
> > #include <errno.h>
> > #include <sys/types.h>
> >
> > /* Linux kernel-ism stubs for stand-alone userspace build. */
>
> This is neat and esp. so that it works.
> But may I ask, what's the value of using this vs UML?
Mainly it's been for giving a single stand-alone file for testing to
compiler devs, packagers, and distro maintainers instead of asking them
to pull down the entire kernel, etc, etc. :)
> Given this has changed into mainly just a KUnit-compatibility layer,
> it feels like it can maybe live as a standalone file, if there's ever
> interest in doing this for other tests.
That's a terrifying and lovely idea!
> It feels like something that will never quite be "supported", but I
> find it neat enough I'd have fun sending some patches to make it more
> realistic.
Right, and as you found, I took some short-cuts that were specific to
how this code used KUnit. :P
I'll ponder this and go through your other suggestions. Thanks!
-Kees
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lib: stackinit: Convert to KUnit
2022-02-24 5:51 [PATCH] lib: stackinit: Convert to KUnit Kees Cook
2022-02-24 19:43 ` Daniel Latypov
@ 2022-02-25 6:53 ` David Gow
2022-03-22 14:30 ` Geert Uytterhoeven
2 siblings, 0 replies; 6+ messages in thread
From: David Gow @ 2022-02-25 6:53 UTC (permalink / raw)
To: Kees Cook
Cc: Daniel Latypov, Arnd Bergmann, Nathan Chancellor,
Nick Desaulniers, Linux Kernel Mailing List, KUnit Development,
llvm, linux-hardening
[-- Attachment #1: Type: text/plain, Size: 26731 bytes --]
On Thu, Feb 24, 2022 at 1:51 PM Kees Cook <keescook@chromium.org> wrote:
>
> Convert to running under Kunit (and retain being able to run stand-alone
> too). Building under Clang (or GCC 12) with CONFIG_INIT_STACK_ALL_ZERO=y,
> this now passes as expected:
>
> $ ./tools/testing/kunit/kunit.py config --make_option LLVM=1
> $ ./tools/testing/kunit/kunit.py run overflow --make_option LLVM=1 \
> --kconfig_add CONFIG_INIT_STACK_ALL_ZERO=y
Should this be "...kunit.py run 'stackinit'...", not overflow?
> ...
> [09:47:28] ================= stackinit (65 subtests) ==================
> [09:47:28] [PASSED] test_u8_zero
> [09:47:28] [PASSED] test_u16_zero
> [09:47:28] [PASSED] test_u32_zero
> [09:47:28] [PASSED] test_u64_zero
> [09:47:28] [PASSED] test_char_array_zero
> [09:47:28] [PASSED] test_small_hole_zero
> [09:47:28] [PASSED] test_big_hole_zero
> [09:47:28] [PASSED] test_trailing_hole_zero
> [09:47:28] [PASSED] test_packed_zero
> [09:47:28] [PASSED] test_small_hole_dynamic_partial
> [09:47:28] [PASSED] test_big_hole_dynamic_partial
> [09:47:28] [PASSED] test_trailing_hole_dynamic_partial
> [09:47:28] [PASSED] test_packed_dynamic_partial
> [09:47:28] [PASSED] test_small_hole_assigned_dynamic_partial
> [09:47:28] [PASSED] test_big_hole_assigned_dynamic_partial
> [09:47:28] [PASSED] test_trailing_hole_assigned_dynamic_partial
> [09:47:28] [PASSED] test_packed_assigned_dynamic_partial
> [09:47:28] [PASSED] test_small_hole_static_partial
> [09:47:28] [PASSED] test_big_hole_static_partial
> [09:47:28] [PASSED] test_trailing_hole_static_partial
> [09:47:28] [PASSED] test_packed_static_partial
> [09:47:28] [PASSED] test_small_hole_static_all
> [09:47:28] [PASSED] test_big_hole_static_all
> [09:47:28] [PASSED] test_trailing_hole_static_all
> [09:47:28] [PASSED] test_packed_static_all
> [09:47:28] [PASSED] test_small_hole_dynamic_all
> [09:47:28] [PASSED] test_big_hole_dynamic_all
> [09:47:28] [PASSED] test_trailing_hole_dynamic_all
> [09:47:28] [PASSED] test_packed_dynamic_all
> [09:47:28] [PASSED] test_small_hole_runtime_partial
> [09:47:28] [PASSED] test_big_hole_runtime_partial
> [09:47:28] [PASSED] test_trailing_hole_runtime_partial
> [09:47:28] [PASSED] test_packed_runtime_partial
> [09:47:28] [PASSED] test_small_hole_runtime_all
> [09:47:28] [PASSED] test_big_hole_runtime_all
> [09:47:28] [PASSED] test_trailing_hole_runtime_all
> [09:47:28] [PASSED] test_packed_runtime_all
> [09:47:28] [PASSED] test_small_hole_assigned_static_partial
> [09:47:28] [PASSED] test_big_hole_assigned_static_partial
> [09:47:28] [PASSED] test_trailing_hole_assigned_static_partial
> [09:47:28] [PASSED] test_packed_assigned_static_partial
> [09:47:28] [PASSED] test_small_hole_assigned_static_all
> [09:47:28] [PASSED] test_big_hole_assigned_static_all
> [09:47:28] [PASSED] test_trailing_hole_assigned_static_all
> [09:47:28] [PASSED] test_packed_assigned_static_all
> [09:47:28] [PASSED] test_small_hole_assigned_dynamic_all
> [09:47:28] [PASSED] test_big_hole_assigned_dynamic_all
> [09:47:28] [PASSED] test_trailing_hole_assigned_dynamic_all
> [09:47:28] [PASSED] test_packed_assigned_dynamic_all
> [09:47:28] [SKIPPED] test_small_hole_assigned_copy
> [09:47:28] [SKIPPED] test_big_hole_assigned_copy
> [09:47:28] [SKIPPED] test_trailing_hole_assigned_copy
> [09:47:28] [PASSED] test_packed_assigned_copy
> [09:47:28] [PASSED] test_u8_none
> [09:47:28] [PASSED] test_u16_none
> [09:47:28] [PASSED] test_u32_none
> [09:47:28] [PASSED] test_u64_none
> [09:47:28] [PASSED] test_char_array_none
> [09:47:28] [SKIPPED] test_switch_1_none
> [09:47:28] [SKIPPED] test_switch_2_none
> [09:47:28] [PASSED] test_small_hole_none
> [09:47:28] [PASSED] test_big_hole_none
> [09:47:28] [PASSED] test_trailing_hole_none
> [09:47:28] [PASSED] test_packed_none
> [09:47:28] [PASSED] test_user
> [09:47:28] ==================== [PASSED] stackinit ====================
> [09:47:28] ============================================================
> [09:47:28] Testing complete. Passed: 60, Failed: 0, Crashed: 0, Skipped: 5, Errors: 0
> [09:47:28] Elapsed time: 4.192s total, 0.001s configuring, 4.070s building, 0.102s running
>
> Cc: David Gow <davidgow@google.com>
> Cc: Daniel Latypov <dlatypov@google.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
This looks pretty good as a KUnit test overall, even if it does some
unusual (but cool!) things with the userland version.
A few comments inline.
Cheers,
-- David
> lib/Kconfig.debug | 26 +-
> lib/Makefile | 4 +-
> lib/{test_stackinit.c => stackinit_kunit.c} | 249 ++++++++++++--------
> 3 files changed, 168 insertions(+), 111 deletions(-)
> rename lib/{test_stackinit.c => stackinit_kunit.c} (73%)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 14d90d03bc8d..ea4415275563 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2511,6 +2511,21 @@ config OVERFLOW_KUNIT_TEST
>
> If unsure, say N.
>
> +config STACKINIT_KUNIT_TEST
> + tristate "Test level of stack variable initialization" if !KUNIT_ALL_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
We probably don't want to enable this test by default (with
KUNIT_ALL_TESTS) if we don't have CONFIG_INIT_STACK_ALL_ZERO=y, as
that'll result in a number of failures for any "all tests"
configuration.
Does it make sense to either:
1. Make this test depend on INIT_STACK_ALL_ZERO?
2. Just change the default, so that KUNIT_ALL_TESTS doesn't trigger it
(or doesn't trigger it unless INIT_STACK_ALL_ZERO is also enabled), so
people can force it to build/run and see the failures?
3. Make it run, but mark some or all tests as SKIPPED if
INIT_STACK_ALL_ZERO is not set (as a failure here is expected)?
> + help
> + Test if the kernel is zero-initializing stack variables and
> + padding. Coverage is controlled by compiler flags,
> + CONFIG_GCC_PLUGIN_STRUCTLEAK, CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF,
> + or CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
> +
> + For more information on KUnit and unit tests in general please refer
> + to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> + If unsure, say N.
> +
> config TEST_UDELAY
> tristate "udelay test driver"
> help
> @@ -2602,17 +2617,6 @@ config TEST_OBJAGG
> Enable this option to test object aggregation manager on boot
> (or module load).
>
> -
> -config TEST_STACKINIT
> - tristate "Test level of stack variable initialization"
> - help
> - Test if the kernel is zero-initializing stack variables and
> - padding. Coverage is controlled by compiler flags,
> - CONFIG_GCC_PLUGIN_STRUCTLEAK, CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF,
> - or CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
> -
> - If unsure, say N.
> -
> config TEST_MEMINIT
> tristate "Test heap/page initialization"
> help
> diff --git a/lib/Makefile b/lib/Makefile
> index fdfcbfaff32f..353bc09ce38d 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -93,8 +93,6 @@ obj-$(CONFIG_TEST_KMOD) += test_kmod.o
> obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += test_debug_virtual.o
> obj-$(CONFIG_TEST_MEMCAT_P) += test_memcat_p.o
> obj-$(CONFIG_TEST_OBJAGG) += test_objagg.o
> -CFLAGS_test_stackinit.o += $(call cc-disable-warning, switch-unreachable)
> -obj-$(CONFIG_TEST_STACKINIT) += test_stackinit.o
> obj-$(CONFIG_TEST_BLACKHOLE_DEV) += test_blackhole_dev.o
> obj-$(CONFIG_TEST_MEMINIT) += test_meminit.o
> obj-$(CONFIG_TEST_LOCKUP) += test_lockup.o
> @@ -363,6 +361,8 @@ obj-$(CONFIG_CMDLINE_KUNIT_TEST) += cmdline_kunit.o
> obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
> obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
> obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
FYI: Due to this context line, this patch only applies cleanly if the
overflow test port patch is applied first.
> +CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
> +obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
>
> obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
>
> diff --git a/lib/test_stackinit.c b/lib/stackinit_kunit.c
> similarity index 73%
> rename from lib/test_stackinit.c
> rename to lib/stackinit_kunit.c
> index a3c74e6a21ff..72c7d4cb8ed2 100644
> --- a/lib/test_stackinit.c
> +++ b/lib/stackinit_kunit.c
> @@ -2,14 +2,23 @@
> /*
> * Test cases for compiler-based stack variable zeroing via
> * -ftrivial-auto-var-init={zero,pattern} or CONFIG_GCC_PLUGIN_STRUCTLEAK*.
> + * For example, see:
> + * https://www.kernel.org/doc/html/latest/dev-tools/kunit/kunit-tool.html#configuring-building-and-running-tests
> + * ./tools/testing/kunit/kunit.py config --make_option LLVM=1
> + * ./tools/testing/kunit/kunit.py run overflow [--raw_output] \
As above, this should probably be 'stackinit', not 'overflow'.
> + * --make_option LLVM=1 \
> + * --kconfig_add CONFIG_INIT_STACK_ALL_ZERO=y
> *
> * External build example:
> * clang -O2 -Wall -ftrivial-auto-var-init=pattern \
> - * -o test_stackinit test_stackinit.c
> + * -o stackinit_kunit stackinit_kunit.c
> + * ./stackinit_kunit
> + *
> */
> #ifdef __KERNEL__
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <kunit/test.h>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> @@ -18,21 +27,55 @@
> #else
>
> /* Userspace headers. */
> +#define _GNU_SOURCE
> #include <stdio.h>
> #include <stdint.h>
> +#include <stdlib.h>
> #include <string.h>
> #include <stdbool.h>
> #include <errno.h>
> #include <sys/types.h>
>
> /* Linux kernel-ism stubs for stand-alone userspace build. */
> -#define KBUILD_MODNAME "stackinit"
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -#define pr_err(fmt, ...) fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__)
> -#define pr_warn(fmt, ...) fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__)
> -#define pr_info(fmt, ...) fprintf(stdout, pr_fmt(fmt), ##__VA_ARGS__)
> -#define __init /**/
> -#define __exit /**/
> +#define TEST_PASS 0
> +#define TEST_SKIP 1
> +#define TEST_FAIL 2
> +struct kunit {
> + int status;
> + char *msg;
> +};
> +struct kunit_case {
> + void (*run_case)(struct kunit *test);
> + const char *name;
> +};
> +struct kunit_suite {
> + const char *name;
> + const struct kunit_case *test_cases;
> +};
> +#define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
> +
> +#define KUNIT_ASSERT_TRUE_MSG(test, expr, fmt, ...) \
> +do { \
> + if (!(expr)) { \
> + if (test->status != TEST_SKIP) \
> + test->status = TEST_FAIL; \
> + if (test->msg) \
> + free(test->msg); \
> + asprintf(&test->msg, fmt, ##__VA_ARGS__); \
> + } \
> +} while (0)
> +
> +#define KUNIT_ASSERT_EQ_MSG(test, left, right, fmt, ...) \
> + KUNIT_ASSERT_TRUE_MSG(test, (left) == (right), fmt, ##__VA_ARGS__)
> +
> +#define kunit_skip(test, fmt, ...) \
> +do { \
> + test->status = TEST_SKIP; \
> + if (test->msg) \
> + free(test->msg); \
> + asprintf(&test->msg, fmt, ##__VA_ARGS__); \
> +} while (0)
> +
> #define __user /**/
> #define noinline __attribute__((__noinline__))
> #define __aligned(x) __attribute__((__aligned__(x)))
> @@ -59,16 +102,44 @@ typedef uint16_t u16;
> typedef uint32_t u32;
> typedef uint64_t u64;
>
> -#define module_init(func) static int (*do_init)(void) = func
> -#define module_exit(func) static void (*do_exit)(void) = func
> -#define MODULE_LICENSE(str) int main(void) { \
> - int rc; \
> - /* License: str */ \
> - rc = do_init(); \
> - if (rc == 0) \
> - do_exit(); \
> - return rc; \
> - }
> +#define MODULE_LICENSE(str) /* */
> +
> +int do_kunit_test_suite(struct kunit_suite *suite)
> +{
> + const struct kunit_case *test_case;
> + int rc = 0;
> +
> + for (test_case = suite->test_cases; test_case->run_case; test_case++) {
> + struct kunit test = { };
> +
> + test_case->run_case(&test);
> + switch (test.status) {
> + default:
> + case TEST_FAIL:
> + fprintf(stderr, "FAIL: %s%s%s", test_case->name,
> + test.msg ? ": " : "",
> + test.msg ?: "\n");
> + rc = 1;
> + break;
> + case TEST_SKIP:
> + fprintf(stdout, "XFAIL: %s%s%s", test_case->name,
> + test.msg ? ": " : "",
> + test.msg ?: "\n");
> + break;
> + case TEST_PASS:
> + fprintf(stdout, "ok: %s\n", test_case->name);
> + break;
> + }
> + if (test.msg)
> + free(test.msg);
> + }
> + return rc;
> +}
This whole userspace mini-kunit thing is very neat. Daniel makes some
good points about how it could be extended and made to match KUnit
more (though I don't think we need to make a perfect userland KUnit
for just this test). The one other thing I'll ask is if it makes sense
to have this output something more similar to KTAP. If the target of
the userspace version isn't kernel developers, I don't think it's
necessary, but if there's no reason _not_ to, it'd be nice for it to
be consistent with the kernel version.
> +
> +#define kunit_test_suite(suite) \
> +int main(void) { \
> + return do_kunit_test_suite(&(suite)); \
> +}
>
> #endif /* __KERNEL__ */
>
> @@ -201,7 +272,7 @@ static bool range_contains(char *haystack_start, size_t haystack_size,
> */
> #define DEFINE_TEST_DRIVER(name, var_type, which, xfail) \
> /* Returns 0 on success, 1 on failure. */ \
> -static noinline __init int test_ ## name (void) \
> +static noinline void test_ ## name (struct kunit *test) \
> { \
> var_type zero INIT_CLONE_ ## which; \
> int ignored; \
> @@ -220,10 +291,8 @@ static noinline __init int test_ ## name (void) \
> /* Verify all bytes overwritten with 0xFF. */ \
> for (sum = 0, i = 0; i < target_size; i++) \
> sum += (check_buf[i] != 0xFF); \
> - if (sum) { \
> - pr_err(#name ": leaf fill was not 0xFF!?\n"); \
> - return 1; \
> - } \
> + KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
> + "leaf fill was not 0xFF!?\n"); \
As Daniel noted, this is a behaviour change for the userspace version,
as its implementation of KUNIT_ASSERT_EQ_MSG() doesn't exit the test
early. So earlier errors will be overwritten with later ones, akin to
KUNIT_EXPECT_EQ_MSG().
In general, KUnit tests will often use the EXPECT variant where
possible, as KUnit will print all failed expectations out, not just
the last one, so ASSERT is usually reserved for things like memory
allocation failures where continuing is no longer safe. That being
said, it can get a bit spammy, so I'm happy with it either way.
> /* Clear entire check buffer for later bit tests. */ \
> memset(check_buf, 0x00, sizeof(check_buf)); \
> /* Extract stack-defined variable contents. */ \
> @@ -231,32 +300,29 @@ static noinline __init int test_ ## name (void) \
> FETCH_ARG_ ## which(zero)); \
> \
> /* Validate that compiler lined up fill and target. */ \
> - if (!range_contains(fill_start, fill_size, \
> - target_start, target_size)) { \
> - pr_err(#name ": stack fill missed target!?\n"); \
> - pr_err(#name ": fill %zu wide\n", fill_size); \
> - pr_err(#name ": target offset by %d\n", \
> - (int)((ssize_t)(uintptr_t)fill_start - \
> - (ssize_t)(uintptr_t)target_start)); \
> - return 1; \
> - } \
> + KUNIT_ASSERT_TRUE_MSG(test, \
> + range_contains(fill_start, fill_size, \
> + target_start, target_size), \
> + "stack fill missed target!? " \
> + "(fill %zu wide, target offset by %d)\n", \
> + fill_size, \
> + (int)((ssize_t)(uintptr_t)fill_start - \
> + (ssize_t)(uintptr_t)target_start)); \
> \
> /* Look for any bytes still 0xFF in check region. */ \
> for (sum = 0, i = 0; i < target_size; i++) \
> sum += (check_buf[i] == 0xFF); \
> \
> - if (sum == 0) { \
> - pr_info(#name " ok\n"); \
> - return 0; \
> - } else { \
> - pr_warn(#name " %sFAIL (uninit bytes: %d)\n", \
> - (xfail) ? "X" : "", sum); \
> - return (xfail) ? 0 : 1; \
> - } \
> + if (sum != 0 && xfail) \
> + kunit_skip(test, \
> + "XFAIL uninit bytes: %d\n", \
> + sum); \
> + KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
> + "uninit bytes: %d\n", sum); \
> }
> #define DEFINE_TEST(name, var_type, which, init_level, xfail) \
> /* no-op to force compiler into ignoring "uninitialized" vars */\
> -static noinline __init DO_NOTHING_TYPE_ ## which(var_type) \
> +static noinline DO_NOTHING_TYPE_ ## which(var_type) \
> do_nothing_ ## name(var_type *ptr) \
> { \
> /* Will always be true, but compiler doesn't know. */ \
> @@ -265,9 +331,8 @@ do_nothing_ ## name(var_type *ptr) \
> else \
> return DO_NOTHING_RETURN_ ## which(ptr + 1); \
> } \
> -static noinline __init int leaf_ ## name(unsigned long sp, \
> - bool fill, \
> - var_type *arg) \
> +static noinline int leaf_ ## name(unsigned long sp, bool fill, \
> + var_type *arg) \
> { \
> char buf[VAR_BUFFER]; \
> var_type var \
> @@ -398,7 +463,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
> * This is intentionally unreachable. To silence the
> * warning, build with -Wno-switch-unreachable
> */
> - uint64_t var;
> + uint64_t var[10];
>
> case 1:
> target_start = &var;
> @@ -423,19 +488,19 @@ static int noinline __leaf_switch_none(int path, bool fill)
> memcpy(check_buf, target_start, target_size);
> break;
> default:
> - var = 5;
> - return var & forced_mask;
> + var[1] = 5;
> + return var[1] & forced_mask;
> }
> return 0;
> }
>
> -static noinline __init int leaf_switch_1_none(unsigned long sp, bool fill,
> +static noinline int leaf_switch_1_none(unsigned long sp, bool fill,
> uint64_t *arg)
> {
> return __leaf_switch_none(1, fill);
> }
>
> -static noinline __init int leaf_switch_2_none(unsigned long sp, bool fill,
> +static noinline int leaf_switch_2_none(unsigned long sp, bool fill,
> uint64_t *arg)
> {
> return __leaf_switch_none(2, fill);
> @@ -450,65 +515,53 @@ static noinline __init int leaf_switch_2_none(unsigned long sp, bool fill,
> DEFINE_TEST_DRIVER(switch_1_none, uint64_t, SCALAR, XFAIL);
> DEFINE_TEST_DRIVER(switch_2_none, uint64_t, SCALAR, XFAIL);
>
> -static int __init test_stackinit_init(void)
> -{
> - unsigned int failures = 0;
> -
> -#define test_scalars(init) do { \
> - failures += test_u8_ ## init (); \
> - failures += test_u16_ ## init (); \
> - failures += test_u32_ ## init (); \
> - failures += test_u64_ ## init (); \
> - failures += test_char_array_ ## init (); \
> - } while (0)
> +#define KUNIT_test_scalars(init) \
> + KUNIT_CASE(test_u8_ ## init), \
> + KUNIT_CASE(test_u16_ ## init), \
> + KUNIT_CASE(test_u32_ ## init), \
> + KUNIT_CASE(test_u64_ ## init), \
> + KUNIT_CASE(test_char_array_ ## init)
>
> -#define test_structs(init) do { \
> - failures += test_small_hole_ ## init (); \
> - failures += test_big_hole_ ## init (); \
> - failures += test_trailing_hole_ ## init (); \
> - failures += test_packed_ ## init (); \
> - } while (0)
> +#define KUNIT_test_structs(init) \
> + KUNIT_CASE(test_small_hole_ ## init), \
> + KUNIT_CASE(test_big_hole_ ## init), \
> + KUNIT_CASE(test_trailing_hole_ ## init),\
> + KUNIT_CASE(test_packed_ ## init) \
>
> +static struct kunit_case stackinit_test_cases[] = {
> /* These are explicitly initialized and should always pass. */
> - test_scalars(zero);
> - test_structs(zero);
> + KUNIT_test_scalars(zero),
> + KUNIT_test_structs(zero),
> /* Padding here appears to be accidentally always initialized? */
> - test_structs(dynamic_partial);
> - test_structs(assigned_dynamic_partial);
> + KUNIT_test_structs(dynamic_partial),
> + KUNIT_test_structs(assigned_dynamic_partial),
> /* Padding initialization depends on compiler behaviors. */
> - test_structs(static_partial);
> - test_structs(static_all);
> - test_structs(dynamic_all);
> - test_structs(runtime_partial);
> - test_structs(runtime_all);
> - test_structs(assigned_static_partial);
> - test_structs(assigned_static_all);
> - test_structs(assigned_dynamic_all);
> + KUNIT_test_structs(static_partial),
> + KUNIT_test_structs(static_all),
> + KUNIT_test_structs(dynamic_all),
> + KUNIT_test_structs(runtime_partial),
> + KUNIT_test_structs(runtime_all),
> + KUNIT_test_structs(assigned_static_partial),
> + KUNIT_test_structs(assigned_static_all),
> + KUNIT_test_structs(assigned_dynamic_all),
> /* Everything fails this since it effectively performs a memcpy(). */
> - test_structs(assigned_copy);
> -
> + KUNIT_test_structs(assigned_copy),
> /* STRUCTLEAK_BYREF_ALL should cover everything from here down. */
> - test_scalars(none);
> - failures += test_switch_1_none();
> - failures += test_switch_2_none();
> -
> + KUNIT_test_scalars(none),
> + KUNIT_CASE(test_switch_1_none),
> + KUNIT_CASE(test_switch_2_none),
> /* STRUCTLEAK_BYREF should cover from here down. */
> - test_structs(none);
> -
> + KUNIT_test_structs(none),
> /* STRUCTLEAK will only cover this. */
> - failures += test_user();
> -
> - if (failures == 0)
> - pr_info("all tests passed!\n");
> - else
> - pr_err("failures: %u\n", failures);
> + KUNIT_CASE(test_user),
> + {}
> +};
>
> - return failures ? -EINVAL : 0;
> -}
> -module_init(test_stackinit_init);
> +static struct kunit_suite stackinit_test_suite = {
> + .name = "stackinit",
> + .test_cases = stackinit_test_cases,
> +};
>
> -static void __exit test_stackinit_exit(void)
> -{ }
> -module_exit(test_stackinit_exit);
> +kunit_test_suite(stackinit_test_suite);
>
> MODULE_LICENSE("GPL");
> --
> 2.30.2
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lib: stackinit: Convert to KUnit
2022-02-24 5:51 [PATCH] lib: stackinit: Convert to KUnit Kees Cook
2022-02-24 19:43 ` Daniel Latypov
2022-02-25 6:53 ` David Gow
@ 2022-03-22 14:30 ` Geert Uytterhoeven
2022-03-23 15:46 ` Kees Cook
2 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2022-03-22 14:30 UTC (permalink / raw)
To: Kees Cook
Cc: David Gow, Daniel Latypov, Arnd Bergmann, Nathan Chancellor,
Nick Desaulniers, Linux Kernel Mailing List, KUnit Development,
llvm, linux-hardening, linux-m68k
Hi Kees,
On Thu, Feb 24, 2022 at 9:12 AM Kees Cook <keescook@chromium.org> wrote:
> Convert to running under Kunit (and retain being able to run stand-alone
> too). Building under Clang (or GCC 12) with CONFIG_INIT_STACK_ALL_ZERO=y,
> this now passes as expected:
>
> $ ./tools/testing/kunit/kunit.py config --make_option LLVM=1
> $ ./tools/testing/kunit/kunit.py run overflow --make_option LLVM=1 \
> --kconfig_add CONFIG_INIT_STACK_ALL_ZERO=y
> ...
> Signed-off-by: Kees Cook <keescook@chromium.org>
Thanks for your patch, which is now commit 02788ebcf521fe78 ("lib:
stackinit: Convert to KUnit") upstream.
Out of curiosity, I gave this a try on m68k, and it still seems to
fail the same way of before[1]:
# Subtest: stackinit
1..65
# test_u8_zero: ASSERTION FAILED at lib/stackinit_kunit.c:333
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 1 wide, target offset by 16)
not ok 1 - test_u8_zero
# test_u16_zero: ASSERTION FAILED at lib/stackinit_kunit.c:333
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 2 wide, target offset by 16)
not ok 2 - test_u16_zero
# test_u32_zero: ASSERTION FAILED at lib/stackinit_kunit.c:333
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 4 wide, target offset by 16)
not ok 3 - test_u32_zero
# test_u64_zero: ASSERTION FAILED at lib/stackinit_kunit.c:333
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 8 wide, target offset by 16)
not ok 4 - test_u64_zero
# test_char_array_zero: ASSERTION FAILED at lib/stackinit_kunit.c:333
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 5 - test_char_array_zero
# test_small_hole_zero: ASSERTION FAILED at lib/stackinit_kunit.c:334
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 6 - test_small_hole_zero
# test_big_hole_zero: ASSERTION FAILED at lib/stackinit_kunit.c:334
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 128 wide, target offset by 64)
not ok 7 - test_big_hole_zero
# test_trailing_hole_zero: ASSERTION FAILED at lib/stackinit_kunit.c:334
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 8 - test_trailing_hole_zero
# test_packed_zero: ASSERTION FAILED at lib/stackinit_kunit.c:334
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 9 - test_packed_zero
# test_small_hole_dynamic_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:337
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 10 - test_small_hole_dynamic_partial
ok 11 - test_big_hole_dynamic_partial
# test_trailing_hole_dynamic_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:337
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 12 - test_trailing_hole_dynamic_partial
# test_packed_dynamic_partial: ASSERTION FAILED at lib/stackinit_kunit.c:337
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 13 - test_packed_dynamic_partial
# test_small_hole_assigned_dynamic_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:340
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 14 - test_small_hole_assigned_dynamic_partial
ok 15 - test_big_hole_assigned_dynamic_partial
# test_trailing_hole_assigned_dynamic_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:340
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 16 - test_trailing_hole_assigned_dynamic_partial
# test_packed_assigned_dynamic_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:340
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 17 - test_packed_assigned_dynamic_partial
# test_small_hole_static_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:336
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 18 - test_small_hole_static_partial
ok 19 - test_big_hole_static_partial
# test_trailing_hole_static_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:336
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 20 - test_trailing_hole_static_partial
# test_packed_static_partial: ASSERTION FAILED at lib/stackinit_kunit.c:336
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 21 - test_packed_static_partial
# test_small_hole_static_all: ASSERTION FAILED at lib/stackinit_kunit.c:336
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 22 - test_small_hole_static_all
ok 23 - test_big_hole_static_all
# test_trailing_hole_static_all: ASSERTION FAILED at
lib/stackinit_kunit.c:336
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 24 - test_trailing_hole_static_all
# test_packed_static_all: ASSERTION FAILED at lib/stackinit_kunit.c:336
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 25 - test_packed_static_all
# test_small_hole_dynamic_all: ASSERTION FAILED at lib/stackinit_kunit.c:337
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 26 - test_small_hole_dynamic_all
ok 27 - test_big_hole_dynamic_all # SKIP XFAIL uninit bytes: 124
# test_trailing_hole_dynamic_all: ASSERTION FAILED at
lib/stackinit_kunit.c:337
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 28 - test_trailing_hole_dynamic_all
# test_packed_dynamic_all: ASSERTION FAILED at lib/stackinit_kunit.c:337
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 29 - test_packed_dynamic_all
# test_small_hole_runtime_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:338
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 30 - test_small_hole_runtime_partial
ok 31 - test_big_hole_runtime_partial # SKIP XFAIL uninit bytes: 127
# test_trailing_hole_runtime_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:338
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 32 - test_trailing_hole_runtime_partial
# test_packed_runtime_partial: ASSERTION FAILED at lib/stackinit_kunit.c:338
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 33 - test_packed_runtime_partial
# test_small_hole_runtime_all: ASSERTION FAILED at lib/stackinit_kunit.c:338
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 34 - test_small_hole_runtime_all
ok 35 - test_big_hole_runtime_all # SKIP XFAIL uninit bytes: 124
# test_trailing_hole_runtime_all: ASSERTION FAILED at
lib/stackinit_kunit.c:338
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 36 - test_trailing_hole_runtime_all
# test_packed_runtime_all: ASSERTION FAILED at lib/stackinit_kunit.c:338
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 37 - test_packed_runtime_all
# test_small_hole_assigned_static_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:339
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 38 - test_small_hole_assigned_static_partial
ok 39 - test_big_hole_assigned_static_partial
# test_trailing_hole_assigned_static_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:339
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 40 - test_trailing_hole_assigned_static_partial
# test_packed_assigned_static_partial: ASSERTION FAILED at
lib/stackinit_kunit.c:339
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 41 - test_packed_assigned_static_partial
# test_small_hole_assigned_static_all: ASSERTION FAILED at
lib/stackinit_kunit.c:339
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 42 - test_small_hole_assigned_static_all
ok 43 - test_big_hole_assigned_static_all
# test_trailing_hole_assigned_static_all: ASSERTION FAILED at
lib/stackinit_kunit.c:339
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 44 - test_trailing_hole_assigned_static_all
# test_packed_assigned_static_all: ASSERTION FAILED at
lib/stackinit_kunit.c:339
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 45 - test_packed_assigned_static_all
# test_small_hole_assigned_dynamic_all: ASSERTION FAILED at
lib/stackinit_kunit.c:340
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 46 - test_small_hole_assigned_dynamic_all
ok 47 - test_big_hole_assigned_dynamic_all # SKIP XFAIL uninit bytes: 124
# test_trailing_hole_assigned_dynamic_all: ASSERTION FAILED at
lib/stackinit_kunit.c:340
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 48 - test_trailing_hole_assigned_dynamic_all
# test_packed_assigned_dynamic_all: ASSERTION FAILED at
lib/stackinit_kunit.c:340
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 49 - test_packed_assigned_dynamic_all
# test_small_hole_assigned_copy: ASSERTION FAILED at
lib/stackinit_kunit.c:341
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 50 - test_small_hole_assigned_copy
ok 51 - test_big_hole_assigned_copy # SKIP XFAIL uninit bytes: 124
# test_trailing_hole_assigned_copy: ASSERTION FAILED at
lib/stackinit_kunit.c:341
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 52 - test_trailing_hole_assigned_copy
# test_packed_assigned_copy: ASSERTION FAILED at lib/stackinit_kunit.c:341
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 53 - test_packed_assigned_copy
# test_u8_none: ASSERTION FAILED at lib/stackinit_kunit.c:343
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 1 wide, target offset by 16)
not ok 54 - test_u8_none
# test_u16_none: ASSERTION FAILED at lib/stackinit_kunit.c:343
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 2 wide, target offset by 16)
not ok 55 - test_u16_none
# test_u32_none: ASSERTION FAILED at lib/stackinit_kunit.c:343
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 4 wide, target offset by 16)
not ok 56 - test_u32_none
# test_u64_none: ASSERTION FAILED at lib/stackinit_kunit.c:343
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 8 wide, target offset by 16)
not ok 57 - test_u64_none
# test_char_array_none: ASSERTION FAILED at lib/stackinit_kunit.c:343
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 4)
not ok 58 - test_char_array_none
# test_switch_1_none: ASSERTION FAILED at lib/stackinit_kunit.c:409
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 80 wide, target offset by 16)
not ok 59 - test_switch_1_none
# test_switch_2_none: ASSERTION FAILED at lib/stackinit_kunit.c:410
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 80 wide, target offset by 16)
not ok 60 - test_switch_2_none
# test_small_hole_none: ASSERTION FAILED at lib/stackinit_kunit.c:344
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 61 - test_small_hole_none
ok 62 - test_big_hole_none # SKIP XFAIL uninit bytes: 128
# test_trailing_hole_none: ASSERTION FAILED at lib/stackinit_kunit.c:344
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 63 - test_trailing_hole_none
# test_packed_none: ASSERTION FAILED at lib/stackinit_kunit.c:344
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 16 wide, target offset by 16)
not ok 64 - test_packed_none
# test_user: ASSERTION FAILED at lib/stackinit_kunit.c:346
Expected range_contains(fill_start, fill_size, target_start,
target_size) to be true, but is false
stack fill missed target!? (fill 14 wide, target offset by 16)
not ok 65 - test_user
# stackinit: pass:6 fail:53 skip:6 total:65
# Totals: pass:6 fail:53 skip:6 total:65
not ok 1 - stackinit
[1] https://lore.kernel.org/r/CAMuHMdW6N40+0gGQ+LSrN64Mo4A0-ELAm0pR3gWQ0mNanyBuUQ@mail.gmail.com
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] lib: stackinit: Convert to KUnit
2022-03-22 14:30 ` Geert Uytterhoeven
@ 2022-03-23 15:46 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2022-03-23 15:46 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: David Gow, Daniel Latypov, Arnd Bergmann, Nathan Chancellor,
Nick Desaulniers, Linux Kernel Mailing List, KUnit Development,
llvm, linux-hardening, linux-m68k
On Tue, Mar 22, 2022 at 03:30:22PM +0100, Geert Uytterhoeven wrote:
> Hi Kees,
>
> On Thu, Feb 24, 2022 at 9:12 AM Kees Cook <keescook@chromium.org> wrote:
> > Convert to running under Kunit (and retain being able to run stand-alone
> > too). Building under Clang (or GCC 12) with CONFIG_INIT_STACK_ALL_ZERO=y,
> > this now passes as expected:
> >
> > $ ./tools/testing/kunit/kunit.py config --make_option LLVM=1
> > $ ./tools/testing/kunit/kunit.py run overflow --make_option LLVM=1 \
> > --kconfig_add CONFIG_INIT_STACK_ALL_ZERO=y
> > ...
>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Thanks for your patch, which is now commit 02788ebcf521fe78 ("lib:
> stackinit: Convert to KUnit") upstream.
>
> Out of curiosity, I gave this a try on m68k, and it still seems to
> fail the same way of before[1]:
> [...]
> [1] https://lore.kernel.org/r/CAMuHMdW6N40+0gGQ+LSrN64Mo4A0-ELAm0pR3gWQ0mNanyBuUQ@mail.gmail.com
Ah yes! Thanks for the reminder. I will take a look at this. Clearly, it's
an issue with memory layout assumptions that don't match on m68k, etc.
--
Kees Cook
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-23 15:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-24 5:51 [PATCH] lib: stackinit: Convert to KUnit Kees Cook
2022-02-24 19:43 ` Daniel Latypov
2022-02-25 1:17 ` Kees Cook
2022-02-25 6:53 ` David Gow
2022-03-22 14:30 ` Geert Uytterhoeven
2022-03-23 15:46 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).