From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753279AbdIEUjA (ORCPT ); Tue, 5 Sep 2017 16:39:00 -0400 Received: from smtp-sh2.infomaniak.ch ([128.65.195.6]:41869 "EHLO smtp-sh2.infomaniak.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752111AbdIEUi5 (ORCPT ); Tue, 5 Sep 2017 16:38:57 -0400 Subject: Re: [PATCH v1] selftests: Enhance kselftest_harness.h to print which assert failed To: shuah@kernel.org, linux-kernel@vger.kernel.org References: <20170806232337.4191-1-mic@digikod.net> Cc: Andy Lutomirski , Kees Cook , Will Drewry , linux-kselftest@vger.kernel.org, Shuah Khan From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= Message-ID: Date: Tue, 5 Sep 2017 22:38:31 +0200 User-Agent: MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="N7lPhbAsqNUdh9ETXjupFwpI0tJ9jqcnP" X-Antivirus: Dr.Web (R) for Unix mail servers drweb plugin ver.6.0.2.8 X-Antivirus-Code: 0x100000 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --N7lPhbAsqNUdh9ETXjupFwpI0tJ9jqcnP Content-Type: multipart/mixed; boundary="SRMdisKdQpAff448KmXLsjnK7EFIwUdKD"; protected-headers="v1" From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= To: shuah@kernel.org, linux-kernel@vger.kernel.org Cc: Andy Lutomirski , Kees Cook , Will Drewry , linux-kselftest@vger.kernel.org, Shuah Khan Message-ID: Subject: Re: [PATCH v1] selftests: Enhance kselftest_harness.h to print which assert failed References: <20170806232337.4191-1-mic@digikod.net> In-Reply-To: --SRMdisKdQpAff448KmXLsjnK7EFIwUdKD Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 28/08/2017 18:42, Shuah Khan wrote: > On 08/06/2017 05:23 PM, Micka=C3=ABl Sala=C3=BCn 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 failur= e. >> 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=C3=ABl Sala=C3=BCn >> Cc: Andy Lutomirski >> Cc: Kees Cook >> Cc: Shuah Khan >> Cc: Will Drewry >> Link: https://lkml.kernel.org/r/CAGXu5j+D-FP8Kt9unNOqKrQJP4DYTpmgkJxWy= kZyrYiVPz3Y3Q@mail.gmail.com >=20 > I am looking through my Inbox and found this one. Okay to pull > this in for 4.14-rc1? Yes, could you please pull this one instead of the one from my Landlock patch series [1] (which is already in your tree)? This patch is more up-to-date and include documentation. Thanks, Micka=C3=ABl [1] https://lkml.kernel.org/r/af24658e-4225-118e-59aa-0b78d6227cb8@kernel.org= >> --- >> >> 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/testi= ng/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 >> =20 >> #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 runni= ng the >> + * test (e.g. not allowed to write to stderr), it is still possible t= o get the >> + * ASSERT_* number for which the test failed. This behavior can be e= nabled by >> + * writing `_metadata->no_print =3D true;` before the check sequence = that is >> + * unable to print. When an error occur, instead of printing an erro= r 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= =2E >> */ >> #define OPTIONAL_HANDLER(_assert) \ >> - for (; _metadata->trigger; _metadata->trigger =3D __bail(_assert)) >> + for (; _metadata->trigger; _metadata->trigger =3D \ >> + __bail(_assert, _metadata->no_print, _metadata->step)) >> + >> +#define __INC_STEP(_metadata) \ >> + if (_metadata->passed && _metadata->step < 255) \ >> + _metadata->step++; >> =20 >> #define __EXPECT(_expected, _seen, _t, _assert) do { \ >> /* Avoid multiple evaluation of the cases */ \ >> __typeof__(_expected) __exp =3D (_expected); \ >> __typeof__(_seen) __seen =3D (_seen); \ >> + if (_assert) __INC_STEP(_metadata); \ >> if (!(__exp _t __seen)) { \ >> unsigned long long __exp_print =3D (uintptr_t)__exp; \ >> unsigned long long __seen_print =3D (uintptr_t)__seen; \ >> @@ -576,6 +593,7 @@ >> #define __EXPECT_STR(_expected, _seen, _t, _assert) do { \ >> const char *__exp =3D (_expected); \ >> const char *__seen =3D (_seen); \ >> + if (_assert) __INC_STEP(_metadata); \ >> if (!(strcmp(__exp, __seen) _t 0)) { \ >> __TH_LOG("Expected '%s' %s '%s'.", __exp, #_t, __seen); \ >> _metadata->passed =3D 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; >> }; >> =20 >> @@ -634,10 +654,13 @@ static inline void __register_test(struct __test= _metadata *t) >> } >> } >> =20 >> -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; >> } >> =20 >> @@ -655,18 +678,24 @@ void __run_test(struct __test_metadata *t) >> t->passed =3D 0; >> } else if (child_pid =3D=3D 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 =3D t->termsig =3D=3D -1 ? WEXITSTATUS(status) : 0; >> + t->passed =3D t->termsig =3D=3D -1 ? !WEXITSTATUS(status) : 0; >> if (t->termsig !=3D -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 =3D 0; >> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/tes= ting/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); >> } >> =20 >> TEST_SIGNAL(mode_strict_cannot_call_prctl, SIGKILL) >> >=20 >=20 --SRMdisKdQpAff448KmXLsjnK7EFIwUdKD-- --N7lPhbAsqNUdh9ETXjupFwpI0tJ9jqcnP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEUysCyY8er9Axt7hqIt7+33O9apUFAlmvC0cACgkQIt7+33O9 apV9CQf9GQMfY1/49PM+NCbJ7N1/LFL8a/Si1qxGrW3n7PEnXXoKPsn2PzXbaSTU IheUnqHsN/8a3gRUP0NHZtMmNQpJl8/4gPMak4kOHlp3U5P2L9rLF4M/19VJ3XAA TVfvraEFWhKQB7O878qeqYHOco9qyXIfGFyXzdJm6mrSTy9Wek0dTb7h4JDpCmF2 oGH8NQy/dH/Saqtr1RXdI1ix33L6XVasuGy/fdEc8sY4fSwg2mFhoEy7uEMFqdJs 9bQumy5egN745AvrX/hbM0BJKaOrsQ+o30OGrXD5rxVtli0HU/l0m9DDXz3Kmy+t qMpjjXWFWUeZeZYpKRFlBtvqj/kkBA== =eglN -----END PGP SIGNATURE----- --N7lPhbAsqNUdh9ETXjupFwpI0tJ9jqcnP--