Skip to content

Commit

Permalink
selftests/harness: Fix TEST_F()'s vfork handling
Browse files Browse the repository at this point in the history
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: Günther Noack <gnoack@google.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Will Drewry <wad@chromium.org>
Fixes: 0710a1a ("selftests/harness: Merge TEST_F_FORK() into TEST_F()")
Reviewed-by: Kees Cook <keescook@chromium.org>
Tested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Reported-by: Mark Brown <broonie@kernel.org>
Tested-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/r/20240305201029.1331333-1-mic@digikod.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Mickaël Salaün authored and Jakub Kicinski committed Mar 7, 2024
1 parent a2f24c8 commit 41cca05
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 23 deletions.
28 changes: 17 additions & 11 deletions tools/testing/selftests/kselftest_harness.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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) \
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 10 additions & 12 deletions tools/testing/selftests/landlock/fs_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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));
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 41cca05

Please sign in to comment.