From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751429AbdH1Qmz (ORCPT ); Mon, 28 Aug 2017 12:42:55 -0400 Received: from mailout.easymail.ca ([64.68.200.34]:50614 "EHLO mailout.easymail.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751300AbdH1Qmx (ORCPT ); Mon, 28 Aug 2017 12:42:53 -0400 Reply-To: shuah@kernel.org Subject: Re: [PATCH v1] selftests: Enhance kselftest_harness.h to print which assert failed To: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= , linux-kernel@vger.kernel.org Cc: Andy Lutomirski , Kees Cook , Will Drewry , linux-kselftest@vger.kernel.org, Shuah Khan , Shuah Khan References: <20170806232337.4191-1-mic@digikod.net> From: Shuah Khan Message-ID: Date: Mon, 28 Aug 2017 10:42:41 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170806232337.4191-1-mic@digikod.net> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Cc: Andy Lutomirski > Cc: Kees Cook > Cc: Shuah Khan > Cc: Will Drewry > Link: https://lkml.kernel.org/r/CAGXu5j+D-FP8Kt9unNOqKrQJP4DYTpmgkJxWykZyrYiVPz3Y3Q@mail.gmail.com 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 > +#include > +#include > #include > #include > #include > @@ -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) >