From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935972AbdDSWGv (ORCPT ); Wed, 19 Apr 2017 18:06:51 -0400 Received: from smtp-sh2.infomaniak.ch ([128.65.195.6]:41427 "EHLO smtp-sh2.infomaniak.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1764938AbdDSWGq (ORCPT ); Wed, 19 Apr 2017 18:06:46 -0400 Subject: Re: [PATCH net-next v6 09/11] seccomp: Enhance test_harness with an assert step mechanism To: Kees Cook References: <20170328234650.19695-1-mic@digikod.net> <20170328234650.19695-10-mic@digikod.net> <94ac6ddc-eaac-8548-f83f-826ddf05ac69@digikod.net> Cc: LKML , Alexei Starovoitov , Andy Lutomirski , Arnaldo Carvalho de Melo , Casey Schaufler , Daniel Borkmann , David Drysdale , "David S . Miller" , "Eric W . Biederman" , James Morris , Jann Horn , Jonathan Corbet , Matthew Garrett , Michael Kerrisk , Paul Moore , Sargun Dhillon , "Serge E . Hallyn" , Shuah Khan , Tejun Heo , Thomas Graf , Will Drewry , "kernel-hardening@lists.openwall.com" , Linux API , linux-security-module , Network Development From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= Message-ID: Date: Thu, 20 Apr 2017 00:05:35 +0200 User-Agent: MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Xh3kFodRXOvR8KOieMI8EPxuRxTmxhTkN" 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) --Xh3kFodRXOvR8KOieMI8EPxuRxTmxhTkN Content-Type: multipart/mixed; boundary="EkGVdDnj6v9DhNwDxX0VXwKPftfbEesU8"; protected-headers="v1" From: =?UTF-8?Q?Micka=c3=abl_Sala=c3=bcn?= To: Kees Cook Cc: LKML , Alexei Starovoitov , Andy Lutomirski , Arnaldo Carvalho de Melo , Casey Schaufler , Daniel Borkmann , David Drysdale , "David S . Miller" , "Eric W . Biederman" , James Morris , Jann Horn , Jonathan Corbet , Matthew Garrett , Michael Kerrisk , Paul Moore , Sargun Dhillon , "Serge E . Hallyn" , Shuah Khan , Tejun Heo , Thomas Graf , Will Drewry , "kernel-hardening@lists.openwall.com" , Linux API , linux-security-module , Network Development Message-ID: Subject: Re: [PATCH net-next v6 09/11] seccomp: Enhance test_harness with an assert step mechanism References: <20170328234650.19695-1-mic@digikod.net> <20170328234650.19695-10-mic@digikod.net> <94ac6ddc-eaac-8548-f83f-826ddf05ac69@digikod.net> In-Reply-To: --EkGVdDnj6v9DhNwDxX0VXwKPftfbEesU8 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 20/04/2017 00:02, Kees Cook wrote: > On Wed, Apr 19, 2017 at 2:51 PM, Micka=C3=ABl Sala=C3=BCn wrote: >> >> On 19/04/2017 02:02, Kees Cook wrote: >>> On Tue, Mar 28, 2017 at 4:46 PM, Micka=C3=ABl Sala=C3=BCn wrote: >>>> This is useful to return an information about the error without bein= g >>>> able to write to TH_LOG_STREAM. >>>> >>>> Helpers from test_harness.h may be useful outside of the seccomp >>>> directory. >>>> >>>> Signed-off-by: Micka=C3=ABl Sala=C3=BCn >>>> Cc: Andy Lutomirski >>>> Cc: Arnaldo Carvalho de Melo >>>> Cc: Kees Cook >>>> Cc: Shuah Khan >>>> Cc: Will Drewry >>>> --- >>>> tools/testing/selftests/seccomp/test_harness.h | 8 +++++++- >>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/tools/testing/selftests/seccomp/test_harness.h b/tools/= testing/selftests/seccomp/test_harness.h >>>> index a786c69c7584..77e407663e06 100644 >>>> --- a/tools/testing/selftests/seccomp/test_harness.h >>>> +++ b/tools/testing/selftests/seccomp/test_harness.h >>>> @@ -397,7 +397,7 @@ struct __test_metadata { >>>> const char *name; >>>> void (*fn)(struct __test_metadata *); >>>> int termsig; >>>> - int passed; >>>> + __s8 passed; >>> >>> Why the reduction here? int is signed too? >> >> Because the return code of a process is capped to 8 bits and I use a >> negative value to not mess with the current interpretation of 0 (error= ) >> and 1 (OK) for the "passed" variable. >> >>> >>>> int trigger; /* extra handler after the evaluation */ >>>> struct __test_metadata *prev, *next; >>>> }; >>>> @@ -476,6 +476,12 @@ void __run_test(struct __test_metadata *t) >>>> "instead of by signal (code:= %d)\n", >>>> t->name, >>>> WEXITSTATUS(status)); >>>> + } else if (t->passed < 0) { >>>> + fprintf(TH_LOG_STREAM, >>>> + "%s: Failed at step #%d\n", >>>> + t->name, >>>> + t->passed * -1); >>>> + t->passed =3D 0; >>>> } >>> >>> Instead of creating an overloaded mechanism here, perhaps have an >>> option reporting mechanism that can be enabled. Like adding to >>> __test_metadata "bool no_stream; int test_number;" and adding >>> test_number++ to each ASSERT/EXCEPT call, and doing something like: >>> >>> if (t->no_stream) { >>> fprintf(TH_LOG_STREAM, >>> "%s: Failed at step #%d\n", >>> t->name, >>> t->test_number); >>> } >>> >>> It'd be a cleaner approach, maybe? >> >> Good idea, we will then be able to use 255 steps! >> >> Do you want me to send this as a separate patch? >> >> Can we move test_harness.h outside of the seccomp directory to be >> available to other subsystems as well? >=20 > Yeah, I would do two patches, and send them out separately (to shuah > with lkml and me in cc at least), one to move test_hardness.h into > some include/ directory, and then to add the new logic for streamless > reporting. >=20 > Thanks! >=20 > -Kees >=20 >=20 Good, in which place and name would it fit better? --EkGVdDnj6v9DhNwDxX0VXwKPftfbEesU8-- --Xh3kFodRXOvR8KOieMI8EPxuRxTmxhTkN Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCgAdFiEEUysCyY8er9Axt7hqIt7+33O9apUFAlj33y8ACgkQIt7+33O9 apVk8Qf6A9txX+Ur7BiCQW88dhEGwn6UTW70UA+VLgOn1PTw+hc0NoD7JQ4iRZFq e4ehtjGu5aOvH/XW7oKi06ZzTXF26AF0kkTmjW30K7KkYd6O+1kl/PePbSDLi+Oa BNMY1mvGU12pIDeN2WmwiIEWmlz4MyMGVAFH7trzWIBkjMPWpQZwEJGPZYrNFvWX Sjuc1cgZdKGtXc8wMuOrULEWePs8+DRHwX3bIDYrzK1X7icS7mwh5JqXd2MQTlzy MVJPuAOpHlvBwLsp9wHImzEzCcmZdP0hRoWmteEdhI/TiqOHxt5etm/Yhlsv/CGS JoRIb0k/3cWRnVHTVmbptrWF8WH8Cg== =671j -----END PGP SIGNATURE----- --Xh3kFodRXOvR8KOieMI8EPxuRxTmxhTkN--