On 08/06/2017 05:23 PM, Mickaël Salaün wrote: > When a test process is not able to write to TH_LOG_STREAM, this step > mechanism enable to print the assert number which triggered the failure. > This can be enabled by setting _metadata->no_print to true at the > beginning of the test sequence. > > Update the seccomp-bpf test to return 0 if a test succeeded. > > This feature is needed for the Landlock tests. > > Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxx> > Cc: Andy Lutomirski <luto@xxxxxxxxxxxxxx> > Cc: Kees Cook <keescook@xxxxxxxxxxxx> > Cc: Shuah Khan <shuah@xxxxxxxxxx> > Cc: Will Drewry <wad@xxxxxxxxxxxx> > Link: https://lkml.kernel.org/r/CAGXu5j+D-FP8Kt9unNOqKrQJP4DYTpmgkJxWykZyrYiVPz3Y3Q@xxxxxxxxxxxxxx I am looking through my Inbox and found this one. Okay to pull this in for 4.14-rc1? thanks, -- Shuah > --- > > Changes since the patch from the Landlock series: > * add the step counter in assert/expect macros and use _metadata to > enable the counter (suggested by Kees Cook) > * only count asserts > * add documentation > --- > tools/testing/selftests/kselftest_harness.h | 39 +++++++++++++++++++++++---- > tools/testing/selftests/seccomp/seccomp_bpf.c | 2 +- > 2 files changed, 35 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h > index c56f72e07cd7..e81bd28bdd89 100644 > --- a/tools/testing/selftests/kselftest_harness.h > +++ b/tools/testing/selftests/kselftest_harness.h > @@ -51,6 +51,9 @@ > #define __KSELFTEST_HARNESS_H > > #define _GNU_SOURCE > +#include <asm/types.h> > +#include <errno.h> > +#include <stdbool.h> > #include <stdint.h> > #include <stdio.h> > #include <stdlib.h> > @@ -84,6 +87,14 @@ > * E.g., #define TH_LOG_ENABLED 1 > * > * If no definition is provided, logging is enabled by default. > + * > + * If there is no way to print an error message for the process running the > + * test (e.g. not allowed to write to stderr), it is still possible to get the > + * ASSERT_* number for which the test failed. This behavior can be enabled by > + * writing `_metadata->no_print = true;` before the check sequence that is > + * unable to print. When an error occur, instead of printing an error message > + * and calling `abort(3)`, the test process call `_exit(2)` with the assert > + * number as argument, which is then printed by the parent process. > */ > #define TH_LOG(fmt, ...) do { \ > if (TH_LOG_ENABLED) \ > @@ -555,12 +566,18 @@ > * return while still providing an optional block to the API consumer. > */ > #define OPTIONAL_HANDLER(_assert) \ > - for (; _metadata->trigger; _metadata->trigger = __bail(_assert)) > + for (; _metadata->trigger; _metadata->trigger = \ > + __bail(_assert, _metadata->no_print, _metadata->step)) > + > +#define __INC_STEP(_metadata) \ > + if (_metadata->passed && _metadata->step < 255) \ > + _metadata->step++; > > #define __EXPECT(_expected, _seen, _t, _assert) do { \ > /* Avoid multiple evaluation of the cases */ \ > __typeof__(_expected) __exp = (_expected); \ > __typeof__(_seen) __seen = (_seen); \ > + if (_assert) __INC_STEP(_metadata); \ > if (!(__exp _t __seen)) { \ > unsigned long long __exp_print = (uintptr_t)__exp; \ > unsigned long long __seen_print = (uintptr_t)__seen; \ > @@ -576,6 +593,7 @@ > #define __EXPECT_STR(_expected, _seen, _t, _assert) do { \ > const char *__exp = (_expected); \ > const char *__seen = (_seen); \ > + if (_assert) __INC_STEP(_metadata); \ > if (!(strcmp(__exp, __seen) _t 0)) { \ > __TH_LOG("Expected '%s' %s '%s'.", __exp, #_t, __seen); \ > _metadata->passed = 0; \ > @@ -590,6 +608,8 @@ struct __test_metadata { > int termsig; > int passed; > int trigger; /* extra handler after the evaluation */ > + __u8 step; > + bool no_print; /* manual trigger when TH_LOG_STREAM is not available */ > struct __test_metadata *prev, *next; > }; > > @@ -634,10 +654,13 @@ static inline void __register_test(struct __test_metadata *t) > } > } > > -static inline int __bail(int for_realz) > +static inline int __bail(int for_realz, bool no_print, __u8 step) > { > - if (for_realz) > + if (for_realz) { > + if (no_print) > + _exit(step); > abort(); > + } > return 0; > } > > @@ -655,18 +678,24 @@ void __run_test(struct __test_metadata *t) > t->passed = 0; > } else if (child_pid == 0) { > t->fn(t); > - _exit(t->passed); > + /* return the step that failed or 0 */ > + _exit(t->passed ? 0 : t->step); > } else { > /* TODO(wad) add timeout support. */ > waitpid(child_pid, &status, 0); > if (WIFEXITED(status)) { > - t->passed = t->termsig == -1 ? WEXITSTATUS(status) : 0; > + t->passed = t->termsig == -1 ? !WEXITSTATUS(status) : 0; > if (t->termsig != -1) { > fprintf(TH_LOG_STREAM, > "%s: Test exited normally " > "instead of by signal (code: %d)\n", > t->name, > WEXITSTATUS(status)); > + } else if (!t->passed) { > + fprintf(TH_LOG_STREAM, > + "%s: Test failed at step #%d\n", > + t->name, > + WEXITSTATUS(status)); > } > } else if (WIFSIGNALED(status)) { > t->passed = 0; > diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c > index 73f5ea6778ce..4d6f92a9df6b 100644 > --- a/tools/testing/selftests/seccomp/seccomp_bpf.c > +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c > @@ -107,7 +107,7 @@ TEST(mode_strict_support) > ASSERT_EQ(0, ret) { > TH_LOG("Kernel does not support CONFIG_SECCOMP"); > } > - syscall(__NR_exit, 1); > + syscall(__NR_exit, 0); > } > > TEST_SIGNAL(mode_strict_cannot_call_prctl, SIGKILL) > -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
