lkml.org 
[lkml]   [2022]   [Mar]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH v3 7/7] UAPI: Introduce KUnit userspace compatibility
On Mon, Feb 28, 2022 at 2:45 AM Kees Cook <keescook@chromium.org> wrote:
>
> The original lib/test_stackinit.c, which exclusively tests toolchain
> features, was designed to also be built without the full Linux kernel
> sources so that compiler developers and distro maintainers had an easy
> way to check for toolchain behaviors. When it was ported to KUnit, this
> mode was removed to simplify the code.
>
> Add a small header that provides a minimally operational KUnit API that
> can allow unit tests that don't depend on kernel-specific behaviors
> to build and run strictly from userspace without kernel sources. Add
> userspace-build support back to the renamed lib/stackinit_kunit.c test.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> v1: https://lore.kernel.org/lkml/20220224055145.1853657-1-keescook@chromium.org
> v2:
> - split from stackinit_kunit.c refactoring patch
> - add missing returns (Daniel)
> - report expression mismatch in assert msg (Daniel)
> - emulate kunit_test_suites() (Daniel)
> - emit valid KTAP (David)
> ---

This looks pretty good on-the-whole to me, modulo one bug (test suites
being marked as SKIPPED instead of FAILED) below. And checkpatch being
grumpy.

I do like the idea of putting this in uapi/ -- it solves the problem
of distributing it quite elegantly, IMO.

-- David

> include/uapi/misc/kunit.h | 181 ++++++++++++++++++++++++++++++++++++++
> lib/stackinit_kunit.c | 11 +++
> 2 files changed, 192 insertions(+)
> create mode 100644 include/uapi/misc/kunit.h
>
> diff --git a/include/uapi/misc/kunit.h b/include/uapi/misc/kunit.h
> new file mode 100644
> index 000000000000..afdffda583ae
> --- /dev/null
> +++ b/include/uapi/misc/kunit.h
> @@ -0,0 +1,181 @@
> +#ifndef __UAPI_MISC_KUNIT_H__
> +#define __UAPI_MISC_KUNIT_H__
> +/*
> + * This is a light-weight userspace drop-in replacement for the in-kernel
> + * KUnit API. It seeks to implement a minimal subset of features so that
> + * a concisely written KUnit test can be made to run entirely in userspace
> + * when it doesn't actually depend on any real kernel internals.
> + *
> + * Additionally contains many refactored kernel-isms to support building
> + * and running in userspace without full kernel source.
> + */
> +
> +#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>
> +
> +#define __user /**/
> +#define noinline __attribute__((__noinline__))
> +#define __aligned(x) __attribute__((__aligned__(x)))
> +#ifdef __clang__
> +# define __compiletime_error(message) /**/
> +#else
> +# define __compiletime_error(message) __attribute__((__error__(message)))
> +#endif
> +#define __compiletime_assert(condition, msg, prefix, suffix) \
> + do { \
> + extern void prefix ## suffix(void) __compiletime_error(msg); \
> + if (!(condition)) \
> + prefix ## suffix(); \
> + } while (0)
> +#define _compiletime_assert(condition, msg, prefix, suffix) \
> + __compiletime_assert(condition, msg, prefix, suffix)
> +#define compiletime_assert(condition, msg) \
> + _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> +#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> +#define BUILD_BUG_ON(condition) \
> + BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
> +
> +#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0]))
> +
> +#define MODULE_LICENSE(str) /* str */
> +
> +typedef uint8_t u8;
> +typedef uint16_t u16;
> +typedef uint32_t u32;
> +typedef uint64_t u64;
> +
> +#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;
> +};

Nit: tabs/spaces.

There are a few other checkpatch warnings, mostly it just complaining
about the use of 'return' in macros, which I think is probably still
better than trying to hack something out of setjmp/longjmp at this
point.

> +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__); \
> + return; \
> + } \
> +} while (0)
> +
> +#define KUNIT_ASSERT_EQ_MSG(test, left, right, fmt, ...) \
> + KUNIT_ASSERT_TRUE_MSG(test, (left) == (right), \
> + #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__); \
> + return; \
> +} while (0)
> +
> +static int do_kunit_test_suite(struct kunit_suite *suite)
> +{
> + const struct kunit_case *test_case;
> + int pass = 0, fail = 0, skip = 0;
> + int rc = 0;
> + size_t i = 0;
> +
> + printf(" TAP version 14\n");
> + for (test_case = suite->test_cases; test_case->run_case; test_case++)
> + i++;
> + printf(" 1..%zu\n", i);
> + i = 0;
> + for (test_case = suite->test_cases; test_case->run_case; test_case++) {
> + struct kunit test = { };
> +
> + i++;
> + test_case->run_case(&test);
> + switch (test.status) {
> + default:
> + case TEST_FAIL:
> + fprintf(stderr, " not ok %zu - %s%s%s",
> + i, test_case->name,
> + test.msg ? " # ERROR " : "",
> + test.msg ?: "\n");
> + rc = 1;

What is this function trying to return? If 'rc' is supposed to be a
result status, this should be TEST_FAIL (2)?

As-is, when a test case fails, the whole suite is being marked as SKIPPED.

The other thing worth noting is that -- if this is fixed -- there's no
way a whole suite can be marked SKIPPED. KUnit will mark a suite as
skipped if all of its subtests are skipped. (This is a much more niche
case, though.)

> + fail++;
> + break;
> + case TEST_SKIP:
> + fprintf(stdout, " ok %zu - %s # SKIP%s%s",
> + i, test_case->name,
> + test.msg ? " " : "",
> + test.msg ?: "\n");
> + skip++;
> + break;
> + case TEST_PASS:
> + fprintf(stdout, " ok %zu - %s\n",
> + i, test_case->name);
> + pass++;
> + break;
> + }
> + if (test.msg)
> + free(test.msg);
> + }
> + printf("# %s: pass:%d fail:%d skip:%d total:%zu\n",
> + suite->name, pass, fail, skip, i);
> + return rc;
> +}
> +
> +static int run_suites(char *name, struct kunit_suite *suites[], size_t count)
> +{
> + int pass = 0, fail = 0, skip = 0;
> + int one, ret = 0;
> + size_t i;
> +
> + printf("TAP version 14\n");
> + printf("1..%zu\n", count);
> + for (i = 0; i < count; ++i) {
> + one = do_kunit_test_suite(suites[i]);
> + switch (one) {
> + case TEST_SKIP:
> + skip++;
> + break;
> + case TEST_PASS:
> + pass++;
> + break;
> + default:
> + fail++;
> + break;
> + }
> + printf("%sok %zu - %s\n",
> + one == TEST_FAIL ? "not " : "",
> + i + 1, suites[i]->name);
> + ret |= one;
> + }
> + printf("# %s: pass:%d fail:%d skip:%d total:%zu\n",
> + name, pass, fail, skip, count);
> + return ret;
> +}
> +
> +#define kunit_test_suites(suite...) \
> +int main(int argc, char *argv[]) { \

Nit: checkpatch really wants the '{' on the next line.

> + static struct kunit_suite *suites[] = { suite }; \
> + return run_suites(argv[0], suites, ARRAY_SIZE(suites)); \
> +}
> +
> +#endif /* __UAPI_MISC_KUNIT_H__ */
> diff --git a/lib/stackinit_kunit.c b/lib/stackinit_kunit.c
> index 35c69aa425b2..6d468630c90a 100644
> --- a/lib/stackinit_kunit.c
> +++ b/lib/stackinit_kunit.c
> @@ -8,7 +8,13 @@
> * --make_option LLVM=1 \
> * --kconfig_add CONFIG_INIT_STACK_ALL_ZERO=y
> *
> + * External build example:
> + * clang -O2 -Wall -ftrivial-auto-var-init=pattern \
> + * -o stackinit_kunit stackinit_kunit.c
> + * ./stackinit_kunit
> + *
> */
> +#ifdef __KERNEL__
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <kunit/test.h>
> @@ -17,6 +23,11 @@
> #include <linux/module.h>
> #include <linux/string.h>
>
> +#else
> +/* Userspace KUnit stub header. */
> +#include <misc/kunit.h>
> +#endif
> +
> /* Exfiltration buffer. */
> #define MAX_VAR_SIZE 128
> static u8 check_buf[MAX_VAR_SIZE];
> --
> 2.32.0
>
[unhandled content-type:application/pkcs7-signature]
\
 
 \ /
  Last update: 2022-03-03 09:27    [W:0.131 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site