From: "Mickaël Salaün" <mic@digikod.net>
To: Kees Cook <keescook@chromium.org>
Cc: "Jakub Kicinski" <kuba@kernel.org>,
"Shuah Khan" <shuah@kernel.org>,
davem@davemloft.net, "Günther Noack" <gnoack@google.com>,
"Will Drewry" <wad@chromium.org>,
edumazet@google.com, jakub@cloudflare.com, pabeni@redhat.com,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-security-module@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH 2/2] selftests/harness: Merge TEST_F_FORK() into TEST_F()
Date: Mon, 26 Feb 2024 21:21:06 +0100 [thread overview]
Message-ID: <20240226.NooJ5ahBip8A@digikod.net> (raw)
In-Reply-To: <202402261102.3BE03F08DF@keescook>
On Mon, Feb 26, 2024 at 11:04:12AM -0800, Kees Cook wrote:
> On Mon, Feb 26, 2024 at 05:23:35PM +0100, Mickaël Salaün wrote:
> > Remplace Landlock-specific TEST_F_FORK() with an improved TEST_F() which
> > brings four related changes:
> >
> > Run TEST_F()'s tests in a grandchild process to make it possible to
> > drop privileges and delegate teardown to the parent.
> >
> > Compared to TEST_F_FORK(), simplify handling of the test grandchild
> > process thanks to vfork(2), and makes it generic (e.g. no explicit
> > conversion between exit code and _metadata).
> >
> > Compared to TEST_F_FORK(), run teardown even when tests failed with an
> > assert thanks to commit 63e6b2a42342 ("selftests/harness: Run TEARDOWN
> > for ASSERT failures").
> >
> > Simplify the test harness code by removing the no_print and step fields
> > which are not used. I added this feature just after I made
> > kselftest_harness.h more broadly available but this step counter
> > remained even though it wasn't needed after all. See commit 369130b63178
> > ("selftests: Enhance kselftest_harness.h to print which assert failed").
>
> I'm personally fine dropping the step counter. (I do wonder if that
> removal should be split from the grandchild launching.)
I thought about that but it was not worth it to add more lines to
review.
>
> > Replace spaces with tabs in one line of __TEST_F_IMPL().
> >
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Jakub Kicinski <kuba@kernel.org>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Shuah Khan <shuah@kernel.org>
> > Cc: Will Drewry <wad@chromium.org>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
>
> One typo below, but otherwise seems good to me:
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
>
> > [...]
> > _metadata->setup_completed = true; \
> > - fixture_name##_##test_name(_metadata, &self, variant->data); \
> > + /* Use the same _metadata. */ \
> > + child = vfork(); \
> > + if (child == 0) { \
> > + fixture_name##_##test_name(_metadata, &self, variant->data); \
> > + _exit(0); \
> > + } \
> > + if (child < 0) { \
> > + ksft_print_msg("ERROR SPAWNING TEST GANDCHILD\n"); \
>
> typo: GAND -> GRAND
Good catch!
Jakub, please fix this with the next combined+rebased series. Thanks!
>
> --
> Kees Cook
>
next prev parent reply other threads:[~2024-02-26 20:21 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-20 19:22 [PATCH net-next v3 00/11] selftests: kselftest_harness: support using xfail Jakub Kicinski
2024-02-20 19:22 ` [PATCH net-next v3 01/11] selftests: kselftest_harness: pass step via shared memory Jakub Kicinski
2024-02-20 19:22 ` [PATCH net-next v3 02/11] selftests: kselftest_harness: use KSFT_* exit codes Jakub Kicinski
2024-02-20 19:22 ` [PATCH net-next v3 03/11] selftests: kselftest_harness: generate test name once Jakub Kicinski
2024-02-20 19:22 ` [PATCH net-next v3 04/11] selftests: kselftest_harness: save full exit code in metadata Jakub Kicinski
2024-02-20 19:22 ` [PATCH net-next v3 05/11] selftests: kselftest_harness: use exit code to store skip Jakub Kicinski
2024-02-21 0:49 ` Kees Cook
2024-02-20 19:22 ` [PATCH net-next v3 06/11] selftests: kselftest: add ksft_test_result_code(), handling all exit codes Jakub Kicinski
2024-02-21 0:49 ` Kees Cook
2024-02-20 19:22 ` [PATCH net-next v3 07/11] selftests: kselftest_harness: print test name for SKIP Jakub Kicinski
2024-02-21 0:50 ` Kees Cook
2024-02-20 19:22 ` [PATCH net-next v3 08/11] selftests: kselftest_harness: separate diagnostic message with # in ksft_test_result_code() Jakub Kicinski
2024-02-21 0:50 ` Kees Cook
2024-02-20 19:22 ` [PATCH net-next v3 09/11] selftests: kselftest_harness: let PASS / FAIL provide diagnostic Jakub Kicinski
2024-02-21 0:52 ` Kees Cook
2024-02-20 19:22 ` [PATCH net-next v3 10/11] selftests: kselftest_harness: support using xfail Jakub Kicinski
2024-02-21 0:52 ` Kees Cook
2024-02-20 19:22 ` [PATCH net-next v3 11/11] selftests: ip_local_port_range: use XFAIL instead of SKIP Jakub Kicinski
2024-02-21 12:03 ` [PATCH net-next v3 00/11] selftests: kselftest_harness: support using xfail Paolo Abeni
2024-02-23 17:48 ` Kees Cook
2024-02-24 0:02 ` Jakub Kicinski
2024-02-26 16:23 ` [PATCH 0/2] Merge TEST_F_FORK() into TEST_F() Mickaël Salaün
2024-02-26 16:23 ` [PATCH 1/2] selftests/landlock: Redefine TEST_F() as TEST_F_FORK() Mickaël Salaün
2024-02-26 16:23 ` [PATCH 2/2] selftests/harness: Merge TEST_F_FORK() into TEST_F() Mickaël Salaün
2024-02-26 19:04 ` Kees Cook
2024-02-26 20:21 ` Mickaël Salaün [this message]
2024-02-26 16:27 ` [PATCH net-next v3 00/11] selftests: kselftest_harness: support using xfail Mickaël Salaün
2024-02-26 16:53 ` Jakub Kicinski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240226.NooJ5ahBip8A@digikod.net \
--to=mic@digikod.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gnoack@google.com \
--cc=jakub@cloudflare.com \
--cc=keescook@chromium.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=shuah@kernel.org \
--cc=wad@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).