netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Kees Cook <keescook@chromium.org>,
	Mark Brown <broonie@kernel.org>, Shuah Khan <shuah@kernel.org>
Cc: "Mickaël Salaün" <mic@digikod.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: [PATCH] selftests/harness: Fix TEST_F()'s vfork handling
Date: Tue,  5 Mar 2024 21:10:29 +0100	[thread overview]
Message-ID: <20240305201029.1331333-1-mic@digikod.net> (raw)
In-Reply-To: <20240305.sheeF9yain1O@digikod.net>

Always run fixture setup in the grandchild process, and by default also
run the teardown in the same process.  However, this change makes it
possible to run the teardown in a parent process when
_metadata->teardown_parent is set to true (e.g. in fixture setup).

Fix TEST_SIGNAL() by forwarding grandchild's signal to its parent.  Fix
seccomp tests by running the test setup in the parent of the test
thread, as expected by the related test code.  Fix Landlock tests by
waiting for the grandchild before processing _metadata.

Use of exit(3) in tests should be OK because the environment in which
the vfork(2) call happen is already dedicated to the running test (with
flushed stdio, setpgrp() call), see __run_test() and the call to fork(2)
just before running the setup/test/teardown.  Even if the test
configures its own exit handlers, they will not be run by the parent
because it never calls exit(3), and the test function either ends with a
call to _exit(2) or a signal.

Cc: David S. Miller <davem@davemloft.net>
Cc: Günther Noack <gnoack@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Will Drewry <wad@chromium.org>
Fixes: 0710a1a73fb4 ("selftests/harness: Merge TEST_F_FORK() into TEST_F()")
Link: https://lore.kernel.org/r/20240305201029.1331333-1-mic@digikod.net
---
 tools/testing/selftests/kselftest_harness.h | 28 +++++++++++++--------
 tools/testing/selftests/landlock/fs_test.c  | 22 ++++++++--------
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
index 634be793ad58..4fd735e48ee7 100644
--- a/tools/testing/selftests/kselftest_harness.h
+++ b/tools/testing/selftests/kselftest_harness.h
@@ -382,29 +382,33 @@
 		/* fixture data is alloced, setup, and torn down per call. */ \
 		FIXTURE_DATA(fixture_name) self; \
 		pid_t child = 1; \
+		int status = 0; \
 		memset(&self, 0, sizeof(FIXTURE_DATA(fixture_name))); \
 		if (setjmp(_metadata->env) == 0) { \
-			fixture_name##_setup(_metadata, &self, variant->data); \
-			/* Let setup failure terminate early. */ \
-			if (_metadata->exit_code) \
-				return; \
-			_metadata->setup_completed = true; \
 			/* Use the same _metadata. */ \
 			child = vfork(); \
 			if (child == 0) { \
+				fixture_name##_setup(_metadata, &self, variant->data); \
+				/* Let setup failure terminate early. */ \
+				if (_metadata->exit_code) \
+					_exit(0); \
+				_metadata->setup_completed = true; \
 				fixture_name##_##test_name(_metadata, &self, variant->data); \
-				_exit(0); \
-			} \
-			if (child < 0) { \
+			} else if (child < 0 || child != waitpid(child, &status, 0)) { \
 				ksft_print_msg("ERROR SPAWNING TEST GRANDCHILD\n"); \
 				_metadata->exit_code = KSFT_FAIL; \
 			} \
 		} \
-		if (child == 0) \
-			/* Child failed and updated the shared _metadata. */ \
+		if (child == 0) { \
+			if (_metadata->setup_completed && !_metadata->teardown_parent) \
+				fixture_name##_teardown(_metadata, &self, variant->data); \
 			_exit(0); \
-		if (_metadata->setup_completed) \
+		} \
+		if (_metadata->setup_completed && _metadata->teardown_parent) \
 			fixture_name##_teardown(_metadata, &self, variant->data); \
+		if (!WIFEXITED(status) && WIFSIGNALED(status)) \
+			/* Forward signal to __wait_for_test(). */ \
+			kill(getpid(), WTERMSIG(status)); \
 		__test_check_assert(_metadata); \
 	} \
 	static struct __test_metadata \
@@ -414,6 +418,7 @@
 		.fixture = &_##fixture_name##_fixture_object, \
 		.termsig = signal, \
 		.timeout = tmout, \
+		.teardown_parent = false, \
 	 }; \
 	static void __attribute__((constructor)) \
 			_register_##fixture_name##_##test_name(void) \
@@ -873,6 +878,7 @@ struct __test_metadata {
 	bool timed_out;	/* did this test timeout instead of exiting? */
 	bool aborted;	/* stopped test due to failed ASSERT */
 	bool setup_completed; /* did setup finish? */
+	bool teardown_parent; /* run teardown in a parent process */
 	jmp_buf env;	/* for exiting out of test early */
 	struct __test_results *results;
 	struct __test_metadata *prev, *next;
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 98817a14c91b..9a6036fbf289 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -285,6 +285,8 @@ static void prepare_layout_opt(struct __test_metadata *const _metadata,
 
 static void prepare_layout(struct __test_metadata *const _metadata)
 {
+	_metadata->teardown_parent = true;
+
 	prepare_layout_opt(_metadata, &mnt_tmp);
 }
 
@@ -3861,9 +3863,7 @@ FIXTURE_SETUP(layout1_bind)
 
 FIXTURE_TEARDOWN(layout1_bind)
 {
-	set_cap(_metadata, CAP_SYS_ADMIN);
-	EXPECT_EQ(0, umount(dir_s2d2));
-	clear_cap(_metadata, CAP_SYS_ADMIN);
+	/* umount(dir_s2d2)) is handled by namespace lifetime. */
 
 	remove_layout1(_metadata);
 
@@ -4276,9 +4276,8 @@ FIXTURE_TEARDOWN(layout2_overlay)
 	EXPECT_EQ(0, remove_path(lower_fl1));
 	EXPECT_EQ(0, remove_path(lower_do1_fo2));
 	EXPECT_EQ(0, remove_path(lower_fo1));
-	set_cap(_metadata, CAP_SYS_ADMIN);
-	EXPECT_EQ(0, umount(LOWER_BASE));
-	clear_cap(_metadata, CAP_SYS_ADMIN);
+
+	/* umount(LOWER_BASE)) is handled by namespace lifetime. */
 	EXPECT_EQ(0, remove_path(LOWER_BASE));
 
 	EXPECT_EQ(0, remove_path(upper_do1_fu3));
@@ -4287,14 +4286,11 @@ FIXTURE_TEARDOWN(layout2_overlay)
 	EXPECT_EQ(0, remove_path(upper_do1_fo2));
 	EXPECT_EQ(0, remove_path(upper_fo1));
 	EXPECT_EQ(0, remove_path(UPPER_WORK "/work"));
-	set_cap(_metadata, CAP_SYS_ADMIN);
-	EXPECT_EQ(0, umount(UPPER_BASE));
-	clear_cap(_metadata, CAP_SYS_ADMIN);
+
+	/* umount(UPPER_BASE)) is handled by namespace lifetime. */
 	EXPECT_EQ(0, remove_path(UPPER_BASE));
 
-	set_cap(_metadata, CAP_SYS_ADMIN);
-	EXPECT_EQ(0, umount(MERGE_DATA));
-	clear_cap(_metadata, CAP_SYS_ADMIN);
+	/* umount(MERGE_DATA)) is handled by namespace lifetime. */
 	EXPECT_EQ(0, remove_path(MERGE_DATA));
 
 	cleanup_layout(_metadata);
@@ -4691,6 +4687,8 @@ FIXTURE_SETUP(layout3_fs)
 		SKIP(return, "this filesystem is not supported (setup)");
 	}
 
+	_metadata->teardown_parent = true;
+
 	slash = strrchr(variant->file_path, '/');
 	ASSERT_NE(slash, NULL);
 	dir_len = (size_t)slash - (size_t)variant->file_path;
-- 
2.44.0


  reply	other threads:[~2024-03-05 20:10 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-29  0:59 [PATCH v4 00/12] selftests: kselftest_harness: support using xfail Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 01/12] selftests/landlock: Redefine TEST_F() as TEST_F_FORK() Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 02/12] selftests/harness: Merge TEST_F_FORK() into TEST_F() Jakub Kicinski
2024-03-04 19:27   ` Mickaël Salaün
2024-03-04 19:31     ` Mickaël Salaün
2024-03-05 15:47       ` Mickaël Salaün
2024-03-05 15:56         ` [PATCH v1 0/2] " Mickaël Salaün
2024-03-05 15:56           ` [PATCH v1 1/2] selftests/landlock: Redefine TEST_F() as TEST_F_FORK() Mickaël Salaün
2024-03-05 15:56           ` [PATCH v1 2/2] selftests/harness: Merge TEST_F_FORK() into TEST_F() Mickaël Salaün
2024-02-29  0:59 ` [PATCH v4 03/12] selftests: kselftest_harness: use KSFT_* exit codes Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 04/12] selftests: kselftest_harness: generate test name once Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 05/12] selftests: kselftest_harness: save full exit code in metadata Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 06/12] selftests: kselftest_harness: use exit code to store skip Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 07/12] selftests: kselftest: add ksft_test_result_code(), handling all exit codes Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 08/12] selftests: kselftest_harness: print test name for SKIP Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 09/12] selftests: kselftest_harness: separate diagnostic message with # in ksft_test_result_code() Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 10/12] selftests: kselftest_harness: let PASS / FAIL provide diagnostic Jakub Kicinski
2024-04-16 14:11   ` Muhammad Usama Anjum
2024-02-29  0:59 ` [PATCH v4 11/12] selftests: kselftest_harness: support using xfail Jakub Kicinski
2024-02-29  0:59 ` [PATCH v4 12/12] selftests: ip_local_port_range: use XFAIL instead of SKIP Jakub Kicinski
2024-02-29 20:19   ` Jakub Sitnicki
2024-02-29 23:25     ` Xin Long
2024-03-01 10:40       ` Jakub Sitnicki
2024-03-01 10:40 ` [PATCH v4 00/12] selftests: kselftest_harness: support using xfail patchwork-bot+netdevbpf
2024-03-04 22:20 ` Mark Brown
2024-03-04 23:04   ` Jakub Kicinski
2024-03-04 23:14     ` Kees Cook
2024-03-04 23:39       ` Jakub Kicinski
2024-03-05  9:43         ` Kees Cook
2024-03-05 16:05           ` Mickaël Salaün
2024-03-05 18:06             ` Jakub Kicinski
2024-03-05 19:14               ` Mickaël Salaün
2024-03-05 20:10                 ` Mickaël Salaün [this message]
2024-03-05 20:25                   ` [PATCH] selftests/harness: Fix TEST_F()'s vfork handling Jakub Kicinski
2024-03-06  7:25                     ` Mickaël Salaün
2024-03-06  7:32                       ` Mickaël Salaün
2024-03-05 20:31                   ` Kees Cook
2024-03-06 13:25                   ` Mark Brown
2024-03-07  4:40                   ` patchwork-bot+netdevbpf
2024-05-02 18:42             ` [PATCH v4 00/12] selftests: kselftest_harness: support using xfail Sean Christopherson
2024-05-02 21:07               ` Mickaël Salaün
2024-03-05 15:48     ` Przemek Kitszel
2024-03-05 16:00       ` Mickaël Salaün
2024-03-05 16:39         ` Mickaël Salaün

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=20240305201029.1331333-1-mic@digikod.net \
    --to=mic@digikod.net \
    --cc=broonie@kernel.org \
    --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 NNTP newsgroup(s).