Skip to content

Commit

Permalink
selftests/harness: Merge TEST_F_FORK() into TEST_F()
Browse files Browse the repository at this point in the history 
Replace 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 63e6b2a ("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 369130b
("selftests: Enhance kselftest_harness.h to print which assert failed").

Replace spaces with tabs in one line of __TEST_F_IMPL().

Cc: Günther Noack <gnoack@google.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Will Drewry <wad@chromium.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Mickaël Salaün authored and David S. Miller committed Mar 1, 2024
1 parent e740486 commit 0710a1a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 91 deletions.
56 changes: 25 additions & 31 deletions tools/testing/selftests/kselftest_harness.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,6 @@
* 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 running the
* test (e.g. not allowed to write to stderr), it is still possible to get the
* ASSERT_* number for which the test failed. This behavior can be enabled by
* writing `_metadata->no_print = true;` before the check sequence that is
* unable to print. When an error occur, instead of printing an error 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) \
Expand Down Expand Up @@ -363,6 +355,11 @@
* Defines a test that depends on a fixture (e.g., is part of a test case).
* Very similar to TEST() except that *self* is the setup instance of fixture's
* datatype exposed for use by the implementation.
*
* The @test_name code is run in a separate process sharing the same memory
* (i.e. vfork), which means that the test process can update its privileges
* without impacting the related FIXTURE_TEARDOWN() (e.g. to remove files from
* a directory where write access was dropped).
*/
#define TEST_F(fixture_name, test_name) \
__TEST_F_IMPL(fixture_name, test_name, -1, TEST_TIMEOUT_DEFAULT)
Expand All @@ -384,15 +381,28 @@
{ \
/* fixture data is alloced, setup, and torn down per call. */ \
FIXTURE_DATA(fixture_name) self; \
pid_t child = 1; \
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->passed || _metadata->skip) \
if (!_metadata->passed || _metadata->skip) \
return; \
_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 GRANDCHILD\n"); \
_metadata->passed = 0; \
} \
} \
if (child == 0) \
/* Child failed and updated the shared _metadata. */ \
_exit(0); \
if (_metadata->setup_completed) \
fixture_name##_teardown(_metadata, &self, variant->data); \
__test_check_assert(_metadata); \
Expand Down Expand Up @@ -694,18 +704,12 @@
for (; _metadata->trigger; _metadata->trigger = \
__bail(_assert, _metadata))

#define __INC_STEP(_metadata) \
/* Keep "step" below 255 (which is used for "SKIP" reporting). */ \
if (_metadata->passed && _metadata->step < 253) \
_metadata->step++;

#define is_signed_type(var) (!!(((__typeof__(var))(-1)) < (__typeof__(var))1))

#define __EXPECT(_expected, _expected_str, _seen, _seen_str, _t, _assert) do { \
/* Avoid multiple evaluation of the cases */ \
__typeof__(_expected) __exp = (_expected); \
__typeof__(_seen) __seen = (_seen); \
if (_assert) __INC_STEP(_metadata); \
if (!(__exp _t __seen)) { \
/* Report with actual signedness to avoid weird output. */ \
switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { \
Expand Down Expand Up @@ -751,7 +755,6 @@
#define __EXPECT_STR(_expected, _seen, _t, _assert) do { \
const char *__exp = (_expected); \
const char *__seen = (_seen); \
if (_assert) __INC_STEP(_metadata); \
if (!(strcmp(__exp, __seen) _t 0)) { \
__TH_LOG("Expected '%s' %s '%s'.", __exp, #_t, __seen); \
_metadata->passed = 0; \
Expand Down Expand Up @@ -837,8 +840,6 @@ struct __test_metadata {
int trigger; /* extra handler after the evaluation */
int timeout; /* seconds to wait for test timeout */
bool timed_out; /* did this test timeout instead of exiting? */
__u8 step;
bool no_print; /* manual trigger when TH_LOG_STREAM is not available */
bool aborted; /* stopped test due to failed ASSERT */
bool setup_completed; /* did setup finish? */
jmp_buf env; /* for exiting out of test early */
Expand Down Expand Up @@ -873,11 +874,8 @@ static inline int __bail(int for_realz, struct __test_metadata *t)

static inline void __test_check_assert(struct __test_metadata *t)
{
if (t->aborted) {
if (t->no_print)
_exit(t->step);
if (t->aborted)
abort();
}
}

struct __test_metadata *__active_test;
Expand Down Expand Up @@ -954,13 +952,12 @@ void __wait_for_test(struct __test_metadata *t)
case 0:
t->passed = 1;
break;
/* Other failure, assume step report. */
/* Failure */
default:
t->passed = 0;
fprintf(TH_LOG_STREAM,
"# %s: Test failed at step #%d\n",
t->name,
WEXITSTATUS(status));
"# %s: Test failed\n",
t->name);
}
}
} else if (WIFSIGNALED(status)) {
Expand Down Expand Up @@ -1114,8 +1111,6 @@ void __run_test(struct __fixture_metadata *f,
t->passed = 1;
t->skip = 0;
t->trigger = 0;
t->step = 1;
t->no_print = 0;
memset(t->results->reason, 0, sizeof(t->results->reason));

ksft_print_msg(" RUN %s%s%s.%s ...\n",
Expand All @@ -1137,8 +1132,7 @@ void __run_test(struct __fixture_metadata *f,
/* Pass is exit 0 */
if (t->passed)
_exit(0);
/* Something else happened, report the step. */
_exit(t->step);
_exit(1);
} else {
__wait_for_test(t);
}
Expand Down
62 changes: 2 additions & 60 deletions tools/testing/selftests/landlock/common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,66 +23,8 @@
#define __maybe_unused __attribute__((__unused__))
#endif

/*
* TEST_F_FORK() is useful when a test drop privileges but the corresponding
* FIXTURE_TEARDOWN() requires them (e.g. to remove files from a directory
* where write actions are denied). For convenience, FIXTURE_TEARDOWN() is
* also called when the test failed, but not when FIXTURE_SETUP() failed. For
* this to be possible, we must not call abort() but instead exit smoothly
* (hence the step print).
*/
/* clang-format off */
#define TEST_F_FORK(fixture_name, test_name) \
static void fixture_name##_##test_name##_child( \
struct __test_metadata *_metadata, \
FIXTURE_DATA(fixture_name) *self, \
const FIXTURE_VARIANT(fixture_name) *variant); \
__TEST_F_IMPL(fixture_name, test_name, -1, TEST_TIMEOUT_DEFAULT) \
{ \
int status; \
const pid_t child = fork(); \
if (child < 0) \
abort(); \
if (child == 0) { \
_metadata->no_print = 1; \
fixture_name##_##test_name##_child(_metadata, self, variant); \
if (_metadata->skip) \
_exit(255); \
if (_metadata->passed) \
_exit(0); \
_exit(_metadata->step); \
} \
if (child != waitpid(child, &status, 0)) \
abort(); \
if (WIFSIGNALED(status) || !WIFEXITED(status)) { \
_metadata->passed = 0; \
_metadata->step = 1; \
return; \
} \
switch (WEXITSTATUS(status)) { \
case 0: \
_metadata->passed = 1; \
break; \
case 255: \
_metadata->passed = 1; \
_metadata->skip = 1; \
break; \
default: \
_metadata->passed = 0; \
_metadata->step = WEXITSTATUS(status); \
break; \
} \
} \
static void fixture_name##_##test_name##_child( \
struct __test_metadata __attribute__((unused)) *_metadata, \
FIXTURE_DATA(fixture_name) __attribute__((unused)) *self, \
const FIXTURE_VARIANT(fixture_name) \
__attribute__((unused)) *variant)
/* clang-format on */

/* Makes backporting easier. */
#undef TEST_F
#define TEST_F(fixture_name, test_name) TEST_F_FORK(fixture_name, test_name)
/* TEST_F_FORK() should not be used for new tests. */
#define TEST_F_FORK(fixture_name, test_name) TEST_F(fixture_name, test_name)

#ifndef landlock_create_ruleset
static inline int
Expand Down

0 comments on commit 0710a1a

Please sign in to comment.