From b85d387c9b091953d5e8ac4c72af6747c1a554e3 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 22 Jun 2020 20:15:42 -0400 Subject: kselftest: fix TAP output for skipped tests According to the TAP specification, a skipped test must be marked as "ok" and annotated with the SKIP directive, for example ok 23 # skip Insufficient flogiston pressure. (https://testanything.org/tap-specification.html) Fix the kselftest infrastructure to match this. For ksft_exit_skip, it is preferrable to emit a dummy plan line that indicates the whole test was skipped, but this is not always possible because of ksft_exit_skip being used as a "shortcut" by the tests. In that case, print the test counts and a normal "ok" line. The format is now the same independent of whether msg is NULL or not (but it is never NULL in any caller right now). Signed-off-by: Paolo Bonzini Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest.h | 28 ++++++++++++++++++++-------- tools/testing/selftests/kselftest/runner.sh | 2 +- 2 files changed, 21 insertions(+), 9 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 0ac49d91a260..5699e4d39337 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -128,7 +128,7 @@ static inline void ksft_test_result_skip(const char *msg, ...) ksft_cnt.ksft_xskip++; va_start(args, msg); - printf("not ok %d # SKIP ", ksft_test_num()); + printf("ok %d # SKIP ", ksft_test_num()); errno = saved_errno; vprintf(msg, args); va_end(args); @@ -190,18 +190,30 @@ static inline int ksft_exit_xpass(void) static inline int ksft_exit_skip(const char *msg, ...) { - if (msg) { - int saved_errno = errno; - va_list args; + int saved_errno = errno; + va_list args; - va_start(args, msg); - printf("not ok %d # SKIP ", 1 + ksft_test_num()); + va_start(args, msg); + + /* + * FIXME: several tests misuse ksft_exit_skip so produce + * something sensible if some tests have already been run + * or a plan has been printed. Those tests should use + * ksft_test_result_skip or ksft_exit_fail_msg instead. + */ + if (ksft_plan || ksft_test_num()) { + ksft_cnt.ksft_xskip++; + printf("ok %d # SKIP ", 1 + ksft_test_num()); + } else { + printf("1..0 # SKIP "); + } + if (msg) { errno = saved_errno; vprintf(msg, args); va_end(args); - } else { - ksft_print_cnts(); } + if (ksft_test_num()) + ksft_print_cnts(); exit(KSFT_SKIP); } diff --git a/tools/testing/selftests/kselftest/runner.sh b/tools/testing/selftests/kselftest/runner.sh index 676b3a8b114d..f4815cbcd60f 100644 --- a/tools/testing/selftests/kselftest/runner.sh +++ b/tools/testing/selftests/kselftest/runner.sh @@ -77,7 +77,7 @@ run_one() echo "ok $test_num $TEST_HDR_MSG") || (rc=$?; \ if [ $rc -eq $skip_rc ]; then \ - echo "not ok $test_num $TEST_HDR_MSG # SKIP" + echo "ok $test_num $TEST_HDR_MSG # SKIP" elif [ $rc -eq $timeout_rc ]; then \ echo "#" echo "not ok $test_num $TEST_HDR_MSG # TIMEOUT" -- cgit v1.2.3 From ce32659b3673767cd92c4919d4000aa0dc056c1c Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 22 Jun 2020 20:15:43 -0400 Subject: selftests: breakpoints: fix computation of test plan The computation of the test plan uses the available_cpus bitset before calling sched_getaffinity to fill it in. The resulting plan is bogus, fix it. Signed-off-by: Paolo Bonzini Signed-off-by: Shuah Khan --- tools/testing/selftests/breakpoints/step_after_suspend_test.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/breakpoints/step_after_suspend_test.c b/tools/testing/selftests/breakpoints/step_after_suspend_test.c index b3ead29c6089..983ee6182e25 100644 --- a/tools/testing/selftests/breakpoints/step_after_suspend_test.c +++ b/tools/testing/selftests/breakpoints/step_after_suspend_test.c @@ -183,6 +183,10 @@ int main(int argc, char **argv) } } + err = sched_getaffinity(0, sizeof(available_cpus), &available_cpus); + if (err < 0) + ksft_exit_fail_msg("sched_getaffinity() failed\n"); + for (cpu = 0; cpu < CPU_SETSIZE; cpu++) { if (!CPU_ISSET(cpu, &available_cpus)) continue; @@ -193,10 +197,6 @@ int main(int argc, char **argv) if (do_suspend) suspend(); - err = sched_getaffinity(0, sizeof(available_cpus), &available_cpus); - if (err < 0) - ksft_exit_fail_msg("sched_getaffinity() failed\n"); - for (cpu = 0; cpu < CPU_SETSIZE; cpu++) { bool test_success; -- cgit v1.2.3 From f000a39c27cc5792f7ac0e8bf2a6cae40de39807 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 22 Jun 2020 20:15:44 -0400 Subject: selftests: breakpoints: do not use ksft_exit_skip after ksft_set_plan Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Use ksft_test_result_skip for the individual tests. The call in suspend() is fine, but ksft_set_plan should be after it. Signed-off-by: Paolo Bonzini Signed-off-by: Shuah Khan --- .../breakpoints/step_after_suspend_test.c | 45 +++++++++++++--------- 1 file changed, 26 insertions(+), 19 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/breakpoints/step_after_suspend_test.c b/tools/testing/selftests/breakpoints/step_after_suspend_test.c index 983ee6182e25..2cf6f10ab7c4 100644 --- a/tools/testing/selftests/breakpoints/step_after_suspend_test.c +++ b/tools/testing/selftests/breakpoints/step_after_suspend_test.c @@ -47,7 +47,7 @@ void child(int cpu) _exit(0); } -bool run_test(int cpu) +int run_test(int cpu) { int status; pid_t pid = fork(); @@ -55,7 +55,7 @@ bool run_test(int cpu) if (pid < 0) { ksft_print_msg("fork() failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (pid == 0) child(cpu); @@ -63,67 +63,68 @@ bool run_test(int cpu) wpid = waitpid(pid, &status, __WALL); if (wpid != pid) { ksft_print_msg("waitpid() failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (!WIFSTOPPED(status)) { ksft_print_msg("child did not stop: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (WSTOPSIG(status) != SIGSTOP) { ksft_print_msg("child did not stop with SIGSTOP: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (ptrace(PTRACE_SINGLESTEP, pid, NULL, NULL) < 0) { if (errno == EIO) { - ksft_exit_skip( + ksft_print_msg( "ptrace(PTRACE_SINGLESTEP) not supported on this architecture: %s\n", strerror(errno)); + return KSFT_SKIP; } ksft_print_msg("ptrace(PTRACE_SINGLESTEP) failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } wpid = waitpid(pid, &status, __WALL); if (wpid != pid) { ksft_print_msg("waitpid() failed: $s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (WIFEXITED(status)) { ksft_print_msg("child did not single-step: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (!WIFSTOPPED(status)) { ksft_print_msg("child did not stop: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (WSTOPSIG(status) != SIGTRAP) { ksft_print_msg("child did not stop with SIGTRAP: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) { ksft_print_msg("ptrace(PTRACE_CONT) failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } wpid = waitpid(pid, &status, __WALL); if (wpid != pid) { ksft_print_msg("waitpid() failed: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } if (!WIFEXITED(status)) { ksft_print_msg("child did not exit after PTRACE_CONT: %s\n", strerror(errno)); - return false; + return KSFT_FAIL; } - return true; + return KSFT_PASS; } void suspend(void) @@ -192,23 +193,29 @@ int main(int argc, char **argv) continue; tests++; } - ksft_set_plan(tests); if (do_suspend) suspend(); + ksft_set_plan(tests); for (cpu = 0; cpu < CPU_SETSIZE; cpu++) { - bool test_success; + int test_success; if (!CPU_ISSET(cpu, &available_cpus)) continue; test_success = run_test(cpu); - if (test_success) { + switch (test_success) { + case KSFT_PASS: ksft_test_result_pass("CPU %d\n", cpu); - } else { + break; + case KSFT_SKIP: + ksft_test_result_skip("CPU %d\n", cpu); + break; + case KSFT_FAIL: ksft_test_result_fail("CPU %d\n", cpu); succeeded = false; + break; } } -- cgit v1.2.3 From 5b0b77ac41e8d97260b35bade6eea21b2d74fdca Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 22 Jun 2020 20:15:46 -0400 Subject: selftests: sigaltstack: do not use ksft_exit_skip after ksft_set_plan Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Use ksft_test_result_skip when possible, or just bail out if memory corruption is detected. Signed-off-by: Paolo Bonzini Signed-off-by: Shuah Khan --- tools/testing/selftests/sigaltstack/sas.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c index ad0f8df2ca0a..8934a3766d20 100644 --- a/tools/testing/selftests/sigaltstack/sas.c +++ b/tools/testing/selftests/sigaltstack/sas.c @@ -71,7 +71,7 @@ void my_usr1(int sig, siginfo_t *si, void *u) swapcontext(&sc, &uc); ksft_print_msg("%s\n", p->msg); if (!p->flag) { - ksft_exit_skip("[RUN]\tAborting\n"); + ksft_exit_fail_msg("[RUN]\tAborting\n"); exit(EXIT_FAILURE); } } @@ -144,7 +144,7 @@ int main(void) err = sigaltstack(&stk, NULL); if (err) { if (errno == EINVAL) { - ksft_exit_skip( + ksft_test_result_skip( "[NOTE]\tThe running kernel doesn't support SS_AUTODISARM\n"); /* * If test cases for the !SS_AUTODISARM variant were -- cgit v1.2.3 From 63aa57f52ce431682206f874a9d39a426e5ce7ea Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 22 Jun 2020 20:15:47 -0400 Subject: selftests: sync_test: do not use ksft_exit_skip after ksft_set_plan Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Move it before. Signed-off-by: Paolo Bonzini Signed-off-by: Shuah Khan --- tools/testing/selftests/sync/sync_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/sync/sync_test.c b/tools/testing/selftests/sync/sync_test.c index 3824b66f41a0..414a617db993 100644 --- a/tools/testing/selftests/sync/sync_test.c +++ b/tools/testing/selftests/sync/sync_test.c @@ -86,9 +86,9 @@ int main(void) int err; ksft_print_header(); - ksft_set_plan(3 + 7); sync_api_supported(); + ksft_set_plan(3 + 7); ksft_print_msg("[RUN]\tTesting sync framework\n"); -- cgit v1.2.3 From 51ad5b54b61d67f6316795da3696c5163aa805cb Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 Jun 2020 11:16:44 -0700 Subject: selftests/clone3: Reorder reporting output Selftest output reporting was happening before the TAP headers and plan had been emitted. Move the first test reports later. Acked-by: Christian Brauner Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/clone3/clone3.c | 2 +- tools/testing/selftests/clone3/clone3_clear_sighand.c | 3 +-- tools/testing/selftests/clone3/clone3_set_tid.c | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c index f14c269a5a18..b7e6dec36173 100644 --- a/tools/testing/selftests/clone3/clone3.c +++ b/tools/testing/selftests/clone3/clone3.c @@ -131,9 +131,9 @@ int main(int argc, char *argv[]) uid_t uid = getuid(); - test_clone3_supported(); ksft_print_header(); ksft_set_plan(17); + test_clone3_supported(); /* Just a simple clone3() should return 0.*/ test_clone3(0, 0, 0, CLONE3_ARGS_NO_TEST); diff --git a/tools/testing/selftests/clone3/clone3_clear_sighand.c b/tools/testing/selftests/clone3/clone3_clear_sighand.c index 9e1af8aa7698..db5fc9c5edcf 100644 --- a/tools/testing/selftests/clone3/clone3_clear_sighand.c +++ b/tools/testing/selftests/clone3/clone3_clear_sighand.c @@ -119,9 +119,8 @@ static void test_clone3_clear_sighand(void) int main(int argc, char **argv) { ksft_print_header(); - test_clone3_supported(); - ksft_set_plan(1); + test_clone3_supported(); test_clone3_clear_sighand(); diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c b/tools/testing/selftests/clone3/clone3_set_tid.c index 25beb22f35b5..5831c1082d6d 100644 --- a/tools/testing/selftests/clone3/clone3_set_tid.c +++ b/tools/testing/selftests/clone3/clone3_set_tid.c @@ -157,8 +157,8 @@ int main(int argc, char *argv[]) pid_t set_tid[MAX_PID_NS_LEVEL * 2]; ksft_print_header(); - test_clone3_supported(); ksft_set_plan(29); + test_clone3_supported(); if (pipe(pipe_1) < 0 || pipe(pipe_2) < 0) ksft_exit_fail_msg("pipe() failed\n"); -- cgit v1.2.3 From ce79097a8f8391fdec835d1deaa112fba4b18362 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 Jun 2020 11:16:45 -0700 Subject: selftests: Remove unneeded selftest API headers Remove unused includes of the kselftest.h header. Acked-by: Christian Brauner Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/pid_namespace/regression_enomem.c | 1 - tools/testing/selftests/pidfd/pidfd_getfd_test.c | 1 - tools/testing/selftests/pidfd/pidfd_setns_test.c | 1 - tools/testing/selftests/uevent/uevent_filtering.c | 1 - 4 files changed, 4 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/pid_namespace/regression_enomem.c b/tools/testing/selftests/pid_namespace/regression_enomem.c index 73d532556d17..7d84097ad45c 100644 --- a/tools/testing/selftests/pid_namespace/regression_enomem.c +++ b/tools/testing/selftests/pid_namespace/regression_enomem.c @@ -11,7 +11,6 @@ #include #include -#include "../kselftest.h" #include "../kselftest_harness.h" #include "../pidfd/pidfd.h" diff --git a/tools/testing/selftests/pidfd/pidfd_getfd_test.c b/tools/testing/selftests/pidfd/pidfd_getfd_test.c index 84b65ecccb04..7758c98be015 100644 --- a/tools/testing/selftests/pidfd/pidfd_getfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_getfd_test.c @@ -18,7 +18,6 @@ #include #include "pidfd.h" -#include "../kselftest.h" #include "../kselftest_harness.h" /* diff --git a/tools/testing/selftests/pidfd/pidfd_setns_test.c b/tools/testing/selftests/pidfd/pidfd_setns_test.c index 9418108eae13..dc95125ae037 100644 --- a/tools/testing/selftests/pidfd/pidfd_setns_test.c +++ b/tools/testing/selftests/pidfd/pidfd_setns_test.c @@ -20,7 +20,6 @@ #include "pidfd.h" #include "../clone3/clone3_selftests.h" -#include "../kselftest.h" #include "../kselftest_harness.h" enum { diff --git a/tools/testing/selftests/uevent/uevent_filtering.c b/tools/testing/selftests/uevent/uevent_filtering.c index f83391aa42cf..5cebfb356345 100644 --- a/tools/testing/selftests/uevent/uevent_filtering.c +++ b/tools/testing/selftests/uevent/uevent_filtering.c @@ -19,7 +19,6 @@ #include #include -#include "../kselftest.h" #include "../kselftest_harness.h" #define __DEV_FULL "/sys/devices/virtual/mem/full/uevent" -- cgit v1.2.3 From eaa163caa4cc72c4381504562b6021a6ffbadc50 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 Jun 2020 11:16:46 -0700 Subject: selftests/binderfs: Fix harness API usage The binderfs test mixed the full harness API and the selftest API. Adjust to use only the harness API so that the harness API can switch to using the selftest API internally in future patches. Acked-by: Christian Brauner Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- .../selftests/filesystems/binderfs/binderfs_test.c | 284 +++++++++++---------- 1 file changed, 146 insertions(+), 138 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c index 8a6b507e34a8..1d27f52c61e6 100644 --- a/tools/testing/selftests/filesystems/binderfs/binderfs_test.c +++ b/tools/testing/selftests/filesystems/binderfs/binderfs_test.c @@ -21,7 +21,6 @@ #include #include -#include "../../kselftest.h" #include "../../kselftest_harness.h" #define DEFAULT_THREADS 4 @@ -37,37 +36,26 @@ fd = -EBADF; \ } -#define log_exit(format, ...) \ - ({ \ - fprintf(stderr, format "\n", ##__VA_ARGS__); \ - exit(EXIT_FAILURE); \ - }) - -static void change_mountns(void) +static void change_mountns(struct __test_metadata *_metadata) { int ret; ret = unshare(CLONE_NEWNS); - if (ret < 0) - ksft_exit_fail_msg("%s - Failed to unshare mount namespace\n", - strerror(errno)); + ASSERT_EQ(ret, 0) { + TH_LOG("%s - Failed to unshare mount namespace", + strerror(errno)); + } ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0); - if (ret < 0) - ksft_exit_fail_msg("%s - Failed to mount / as private\n", - strerror(errno)); -} - -static void rmdir_protect_errno(const char *dir) -{ - int saved_errno = errno; - (void)rmdir(dir); - errno = saved_errno; + ASSERT_EQ(ret, 0) { + TH_LOG("%s - Failed to mount / as private", + strerror(errno)); + } } -static int __do_binderfs_test(void) +static int __do_binderfs_test(struct __test_metadata *_metadata) { - int fd, ret, saved_errno; + int fd, ret, saved_errno, result = 1; size_t len; ssize_t wret; struct binderfs_device device = { 0 }; @@ -75,113 +63,107 @@ static int __do_binderfs_test(void) char binderfs_mntpt[] = P_tmpdir "/binderfs_XXXXXX", device_path[sizeof(P_tmpdir "/binderfs_XXXXXX/") + BINDERFS_MAX_NAME]; - change_mountns(); + change_mountns(_metadata); - if (!mkdtemp(binderfs_mntpt)) - ksft_exit_fail_msg( - "%s - Failed to create binderfs mountpoint\n", + EXPECT_NE(mkdtemp(binderfs_mntpt), NULL) { + TH_LOG("%s - Failed to create binderfs mountpoint", strerror(errno)); + goto out; + } ret = mount(NULL, binderfs_mntpt, "binder", 0, 0); - if (ret < 0) { - if (errno != ENODEV) - ksft_exit_fail_msg("%s - Failed to mount binderfs\n", - strerror(errno)); - - rmdir_protect_errno(binderfs_mntpt); - return 1; + EXPECT_EQ(ret, 0) { + if (errno == ENODEV) + XFAIL(goto out, "binderfs missing"); + TH_LOG("%s - Failed to mount binderfs", strerror(errno)); + goto rmdir; } - /* binderfs mount test passed */ - ksft_inc_pass_cnt(); + /* success: binderfs mounted */ memcpy(device.name, "my-binder", strlen("my-binder")); snprintf(device_path, sizeof(device_path), "%s/binder-control", binderfs_mntpt); fd = open(device_path, O_RDONLY | O_CLOEXEC); - if (fd < 0) - ksft_exit_fail_msg( - "%s - Failed to open binder-control device\n", + EXPECT_GE(fd, 0) { + TH_LOG("%s - Failed to open binder-control device", strerror(errno)); + goto umount; + } ret = ioctl(fd, BINDER_CTL_ADD, &device); saved_errno = errno; close(fd); errno = saved_errno; - if (ret < 0) { - rmdir_protect_errno(binderfs_mntpt); - ksft_exit_fail_msg( - "%s - Failed to allocate new binder device\n", + EXPECT_GE(ret, 0) { + TH_LOG("%s - Failed to allocate new binder device", strerror(errno)); + goto umount; } - ksft_print_msg( - "Allocated new binder device with major %d, minor %d, and name %s\n", + TH_LOG("Allocated new binder device with major %d, minor %d, and name %s", device.major, device.minor, device.name); - /* binder device allocation test passed */ - ksft_inc_pass_cnt(); + /* success: binder device allocation */ snprintf(device_path, sizeof(device_path), "%s/my-binder", binderfs_mntpt); fd = open(device_path, O_CLOEXEC | O_RDONLY); - if (fd < 0) { - rmdir_protect_errno(binderfs_mntpt); - ksft_exit_fail_msg("%s - Failed to open my-binder device\n", - strerror(errno)); + EXPECT_GE(fd, 0) { + TH_LOG("%s - Failed to open my-binder device", + strerror(errno)); + goto umount; } ret = ioctl(fd, BINDER_VERSION, &version); saved_errno = errno; close(fd); errno = saved_errno; - if (ret < 0) { - rmdir_protect_errno(binderfs_mntpt); - ksft_exit_fail_msg( - "%s - Failed to open perform BINDER_VERSION request\n", + EXPECT_GE(ret, 0) { + TH_LOG("%s - Failed to open perform BINDER_VERSION request", strerror(errno)); + goto umount; } - ksft_print_msg("Detected binder version: %d\n", - version.protocol_version); + TH_LOG("Detected binder version: %d", version.protocol_version); - /* binder transaction with binderfs binder device passed */ - ksft_inc_pass_cnt(); + /* success: binder transaction with binderfs binder device */ ret = unlink(device_path); - if (ret < 0) { - rmdir_protect_errno(binderfs_mntpt); - ksft_exit_fail_msg("%s - Failed to delete binder device\n", - strerror(errno)); + EXPECT_EQ(ret, 0) { + TH_LOG("%s - Failed to delete binder device", + strerror(errno)); + goto umount; } - /* binder device removal passed */ - ksft_inc_pass_cnt(); + /* success: binder device removal */ snprintf(device_path, sizeof(device_path), "%s/binder-control", binderfs_mntpt); ret = unlink(device_path); - if (!ret) { - rmdir_protect_errno(binderfs_mntpt); - ksft_exit_fail_msg("Managed to delete binder-control device\n"); - } else if (errno != EPERM) { - rmdir_protect_errno(binderfs_mntpt); - ksft_exit_fail_msg( - "%s - Failed to delete binder-control device but exited with unexpected error code\n", + EXPECT_NE(ret, 0) { + TH_LOG("Managed to delete binder-control device"); + goto umount; + } + EXPECT_EQ(errno, EPERM) { + TH_LOG("%s - Failed to delete binder-control device but exited with unexpected error code", strerror(errno)); + goto umount; } - /* binder-control device removal failed as expected */ - ksft_inc_xfail_cnt(); + /* success: binder-control device removal failed as expected */ + result = 0; -on_error: +umount: ret = umount2(binderfs_mntpt, MNT_DETACH); - rmdir_protect_errno(binderfs_mntpt); - if (ret < 0) - ksft_exit_fail_msg("%s - Failed to unmount binderfs\n", - strerror(errno)); - - /* binderfs unmount test passed */ - ksft_inc_pass_cnt(); - return 0; + EXPECT_EQ(ret, 0) { + TH_LOG("%s - Failed to unmount binderfs", strerror(errno)); + } +rmdir: + ret = rmdir(binderfs_mntpt); + EXPECT_EQ(ret, 0) { + TH_LOG("%s - Failed to rmdir binderfs mount", strerror(errno)); + } +out: + return result; } static int wait_for_pid(pid_t pid) @@ -291,7 +273,7 @@ static int write_id_mapping(enum idmap_type type, pid_t pid, const char *buf, return 0; } -static void change_userns(int syncfds[2]) +static void change_userns(struct __test_metadata *_metadata, int syncfds[2]) { int ret; char buf; @@ -299,25 +281,29 @@ static void change_userns(int syncfds[2]) close_prot_errno_disarm(syncfds[1]); ret = unshare(CLONE_NEWUSER); - if (ret < 0) - ksft_exit_fail_msg("%s - Failed to unshare user namespace\n", - strerror(errno)); + ASSERT_EQ(ret, 0) { + TH_LOG("%s - Failed to unshare user namespace", + strerror(errno)); + } ret = write_nointr(syncfds[0], "1", 1); - if (ret != 1) - ksft_exit_fail_msg("write_nointr() failed\n"); + ASSERT_EQ(ret, 1) { + TH_LOG("write_nointr() failed"); + } ret = read_nointr(syncfds[0], &buf, 1); - if (ret != 1) - ksft_exit_fail_msg("read_nointr() failed\n"); + ASSERT_EQ(ret, 1) { + TH_LOG("read_nointr() failed"); + } close_prot_errno_disarm(syncfds[0]); - if (setid_userns_root()) - ksft_exit_fail_msg("setid_userns_root() failed"); + ASSERT_EQ(setid_userns_root(), 0) { + TH_LOG("setid_userns_root() failed"); + } } -static void change_idmaps(int syncfds[2], pid_t pid) +static void change_idmaps(struct __test_metadata *_metadata, int syncfds[2], pid_t pid) { int ret; char buf; @@ -326,35 +312,42 @@ static void change_idmaps(int syncfds[2], pid_t pid) close_prot_errno_disarm(syncfds[0]); ret = read_nointr(syncfds[1], &buf, 1); - if (ret != 1) - ksft_exit_fail_msg("read_nointr() failed\n"); + ASSERT_EQ(ret, 1) { + TH_LOG("read_nointr() failed"); + } snprintf(id_map, sizeof(id_map), "0 %d 1\n", getuid()); ret = write_id_mapping(UID_MAP, pid, id_map, strlen(id_map)); - if (ret) - ksft_exit_fail_msg("write_id_mapping(UID_MAP) failed"); + ASSERT_EQ(ret, 0) { + TH_LOG("write_id_mapping(UID_MAP) failed"); + } snprintf(id_map, sizeof(id_map), "0 %d 1\n", getgid()); ret = write_id_mapping(GID_MAP, pid, id_map, strlen(id_map)); - if (ret) - ksft_exit_fail_msg("write_id_mapping(GID_MAP) failed"); + ASSERT_EQ(ret, 0) { + TH_LOG("write_id_mapping(GID_MAP) failed"); + } ret = write_nointr(syncfds[1], "1", 1); - if (ret != 1) - ksft_exit_fail_msg("write_nointr() failed"); + ASSERT_EQ(ret, 1) { + TH_LOG("write_nointr() failed"); + } close_prot_errno_disarm(syncfds[1]); } +struct __test_metadata *_thread_metadata; static void *binder_version_thread(void *data) { + struct __test_metadata *_metadata = _thread_metadata; int fd = PTR_TO_INT(data); struct binder_version version = { 0 }; int ret; ret = ioctl(fd, BINDER_VERSION, &version); if (ret < 0) - ksft_print_msg("%s - Failed to open perform BINDER_VERSION request\n", strerror(errno)); + TH_LOG("%s - Failed to open perform BINDER_VERSION request\n", + strerror(errno)); pthread_exit(data); } @@ -377,68 +370,79 @@ TEST(binderfs_stress) device_path[sizeof(P_tmpdir "/binderfs_XXXXXX/") + BINDERFS_MAX_NAME]; ret = socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, syncfds); - if (ret < 0) - ksft_exit_fail_msg("%s - Failed to create socket pair", strerror(errno)); + ASSERT_EQ(ret, 0) { + TH_LOG("%s - Failed to create socket pair", strerror(errno)); + } pid = fork(); - if (pid < 0) { + ASSERT_GE(pid, 0) { + TH_LOG("%s - Failed to fork", strerror(errno)); close_prot_errno_disarm(syncfds[0]); close_prot_errno_disarm(syncfds[1]); - ksft_exit_fail_msg("%s - Failed to fork", strerror(errno)); } if (pid == 0) { int i, j, k, nthreads; pthread_attr_t attr; pthread_t threads[DEFAULT_THREADS]; - change_userns(syncfds); - change_mountns(); + change_userns(_metadata, syncfds); + change_mountns(_metadata); - if (!mkdtemp(binderfs_mntpt)) - log_exit("%s - Failed to create binderfs mountpoint\n", - strerror(errno)); + ASSERT_NE(mkdtemp(binderfs_mntpt), NULL) { + TH_LOG("%s - Failed to create binderfs mountpoint", + strerror(errno)); + } ret = mount(NULL, binderfs_mntpt, "binder", 0, 0); - if (ret < 0) - log_exit("%s - Failed to mount binderfs\n", strerror(errno)); + ASSERT_EQ(ret, 0) { + TH_LOG("%s - Failed to mount binderfs", strerror(errno)); + } for (int i = 0; i < ARRAY_SIZE(fds); i++) { snprintf(device_path, sizeof(device_path), "%s/binder-control", binderfs_mntpt); fd = open(device_path, O_RDONLY | O_CLOEXEC); - if (fd < 0) - log_exit("%s - Failed to open binder-control device\n", strerror(errno)); + ASSERT_GE(fd, 0) { + TH_LOG("%s - Failed to open binder-control device", + strerror(errno)); + } memset(&device, 0, sizeof(device)); snprintf(device.name, sizeof(device.name), "%d", i); ret = ioctl(fd, BINDER_CTL_ADD, &device); close_prot_errno_disarm(fd); - if (ret < 0) - log_exit("%s - Failed to allocate new binder device\n", strerror(errno)); + ASSERT_EQ(ret, 0) { + TH_LOG("%s - Failed to allocate new binder device", + strerror(errno)); + } snprintf(device_path, sizeof(device_path), "%s/%d", binderfs_mntpt, i); fds[i] = open(device_path, O_RDONLY | O_CLOEXEC); - if (fds[i] < 0) - log_exit("%s - Failed to open binder device\n", strerror(errno)); + ASSERT_GE(fds[i], 0) { + TH_LOG("%s - Failed to open binder device", strerror(errno)); + } } ret = umount2(binderfs_mntpt, MNT_DETACH); - rmdir_protect_errno(binderfs_mntpt); - if (ret < 0) - log_exit("%s - Failed to unmount binderfs\n", strerror(errno)); + ASSERT_EQ(ret, 0) { + TH_LOG("%s - Failed to unmount binderfs", strerror(errno)); + rmdir(binderfs_mntpt); + } nthreads = get_nprocs_conf(); if (nthreads > DEFAULT_THREADS) nthreads = DEFAULT_THREADS; + _thread_metadata = _metadata; pthread_attr_init(&attr); for (k = 0; k < ARRAY_SIZE(fds); k++) { for (i = 0; i < nthreads; i++) { ret = pthread_create(&threads[i], &attr, binder_version_thread, INT_TO_PTR(fds[k])); if (ret) { - ksft_print_msg("%s - Failed to create thread %d\n", strerror(errno), i); + TH_LOG("%s - Failed to create thread %d", + strerror(errno), i); break; } } @@ -448,7 +452,8 @@ TEST(binderfs_stress) ret = pthread_join(threads[j], &fdptr); if (ret) - ksft_print_msg("%s - Failed to join thread %d for fd %d\n", strerror(errno), j, PTR_TO_INT(fdptr)); + TH_LOG("%s - Failed to join thread %d for fd %d", + strerror(errno), j, PTR_TO_INT(fdptr)); } } pthread_attr_destroy(&attr); @@ -459,11 +464,12 @@ TEST(binderfs_stress) exit(EXIT_SUCCESS); } - change_idmaps(syncfds, pid); + change_idmaps(_metadata, syncfds, pid); ret = wait_for_pid(pid); - if (ret) - ksft_exit_fail_msg("wait_for_pid() failed"); + ASSERT_EQ(ret, 0) { + TH_LOG("wait_for_pid() failed"); + } } TEST(binderfs_test_privileged) @@ -471,7 +477,7 @@ TEST(binderfs_test_privileged) if (geteuid() != 0) XFAIL(return, "Tests are not run as root. Skipping privileged tests"); - if (__do_binderfs_test() == 1) + if (__do_binderfs_test(_metadata)) XFAIL(return, "The Android binderfs filesystem is not available"); } @@ -482,31 +488,33 @@ TEST(binderfs_test_unprivileged) pid_t pid; ret = socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, syncfds); - if (ret < 0) - ksft_exit_fail_msg("%s - Failed to create socket pair", strerror(errno)); + ASSERT_EQ(ret, 0) { + TH_LOG("%s - Failed to create socket pair", strerror(errno)); + } pid = fork(); - if (pid < 0) { + ASSERT_GE(pid, 0) { close_prot_errno_disarm(syncfds[0]); close_prot_errno_disarm(syncfds[1]); - ksft_exit_fail_msg("%s - Failed to fork", strerror(errno)); + TH_LOG("%s - Failed to fork", strerror(errno)); } if (pid == 0) { - change_userns(syncfds); - if (__do_binderfs_test() == 1) + change_userns(_metadata, syncfds); + if (__do_binderfs_test(_metadata)) exit(2); exit(EXIT_SUCCESS); } - change_idmaps(syncfds, pid); + change_idmaps(_metadata, syncfds, pid); ret = wait_for_pid(pid); if (ret) { if (ret == 2) XFAIL(return, "The Android binderfs filesystem is not available"); - else - ksft_exit_fail_msg("wait_for_pid() failed"); + ASSERT_EQ(ret, 0) { + TH_LOG("wait_for_pid() failed"); + } } } -- cgit v1.2.3 From 245dd6041d0d923fb6563a0704749d3a7de16c56 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 Jun 2020 11:16:47 -0700 Subject: selftests: Add header documentation and helpers Add "how to use this API" documentation to kselftest.h, and include some addition helpers and notes to make things easier to use. Additionally removes the incorrect "Bail out!" line from the standard exit path. The TAP13 specification says that "Bail out!" should be used when giving up before all tests have been run. For a "normal" execution run, the selftests should not report "Bail out!". Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest.h | 73 ++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 5699e4d39337..0451cc29d68d 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -6,6 +6,37 @@ * Copyright (c) 2014 Shuah Khan * Copyright (c) 2014 Samsung Electronics Co., Ltd. * + * Using this API consists of first counting how many tests your code + * has to run, and then starting up the reporting: + * + * ksft_print_header(); + * ksft_set_plan(total_number_of_tests); + * + * For each test, report any progress, debugging, etc with: + * + * ksft_print_msg(fmt, ...); + * + * and finally report the pass/fail/skip/xfail state of the test with one of: + * + * ksft_test_result(condition, fmt, ...); + * ksft_test_result_pass(fmt, ...); + * ksft_test_result_fail(fmt, ...); + * ksft_test_result_skip(fmt, ...); + * ksft_test_result_xfail(fmt, ...); + * ksft_test_result_error(fmt, ...); + * + * When all tests are finished, clean up and exit the program with one of: + * + * ksft_exit(condition); + * ksft_exit_pass(); + * ksft_exit_fail(); + * + * If the program wants to report details on why the entire program has + * failed, it can instead exit with a message (this is usually done when + * the program is aborting before finishing all tests): + * + * ksft_exit_fail_msg(fmt, ...); + * */ #ifndef __KSELFTEST_H #define __KSELFTEST_H @@ -74,7 +105,7 @@ static inline void ksft_print_cnts(void) if (ksft_plan != ksft_test_num()) printf("# Planned tests != run tests (%u != %u)\n", ksft_plan, ksft_test_num()); - printf("# Pass %d Fail %d Xfail %d Xpass %d Skip %d Error %d\n", + printf("# Totals: pass:%d fail:%d xfail:%d xpass:%d skip:%d error:%d\n", ksft_cnt.ksft_pass, ksft_cnt.ksft_fail, ksft_cnt.ksft_xfail, ksft_cnt.ksft_xpass, ksft_cnt.ksft_xskip, ksft_cnt.ksft_error); @@ -120,6 +151,32 @@ static inline void ksft_test_result_fail(const char *msg, ...) va_end(args); } +/** + * ksft_test_result() - Report test success based on truth of condition + * + * @condition: if true, report test success, otherwise failure. + */ +#define ksft_test_result(condition, fmt, ...) do { \ + if (!!(condition)) \ + ksft_test_result_pass(fmt, ##__VA_ARGS__);\ + else \ + ksft_test_result_fail(fmt, ##__VA_ARGS__);\ + } while (0) + +static inline void ksft_test_result_xfail(const char *msg, ...) +{ + int saved_errno = errno; + va_list args; + + ksft_cnt.ksft_xfail++; + + va_start(args, msg); + printf("ok %d # XFAIL ", ksft_test_num()); + errno = saved_errno; + vprintf(msg, args); + va_end(args); +} + static inline void ksft_test_result_skip(const char *msg, ...) { int saved_errno = errno; @@ -134,6 +191,7 @@ static inline void ksft_test_result_skip(const char *msg, ...) va_end(args); } +/* TODO: how does "error" differ from "fail" or "skip"? */ static inline void ksft_test_result_error(const char *msg, ...) { int saved_errno = errno; @@ -156,11 +214,22 @@ static inline int ksft_exit_pass(void) static inline int ksft_exit_fail(void) { - printf("Bail out!\n"); ksft_print_cnts(); exit(KSFT_FAIL); } +/** + * ksft_exit() - Exit selftest based on truth of condition + * + * @condition: if true, exit self test with success, otherwise fail. + */ +#define ksft_exit(condition) do { \ + if (!!(condition)) \ + ksft_exit_pass(); \ + else \ + ksft_exit_fail(); \ + } while (0) + static inline int ksft_exit_fail_msg(const char *msg, ...) { int saved_errno = errno; -- cgit v1.2.3 From e80068be21824e4d2726eeea41cac6178d212415 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 Jun 2020 11:16:48 -0700 Subject: selftests/harness: Switch to TAP output Using the kselftest_harness.h would result in non-TAP test reporting, which didn't make much sense given that all the requirements for using the low-level API were met. Switch to using ksft_*() helpers while retaining as much of a human-readability as possible. Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest.h | 5 +-- tools/testing/selftests/kselftest_harness.h | 52 +++++++++++++++++------------ 2 files changed, 33 insertions(+), 24 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 0451cc29d68d..3af0311de158 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -1,7 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0 */ /* - * kselftest.h: kselftest framework return codes to include from - * selftests. + * kselftest.h: low-level kselftest framework to include from + * selftest programs. When possible, please use + * kselftest_harness.h instead. * * Copyright (c) 2014 Shuah Khan * Copyright (c) 2014 Samsung Electronics Co., Ltd. diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index c9f03ef93338..f8f7e47c739a 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -50,7 +50,9 @@ #ifndef __KSELFTEST_HARNESS_H #define __KSELFTEST_HARNESS_H +#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endif #include #include #include @@ -62,6 +64,8 @@ #include #include +#include "kselftest.h" + #define TEST_TIMEOUT_DEFAULT 30 /* Utilities exposed to the test definitions */ @@ -104,7 +108,7 @@ /* Unconditional logger for internal use. */ #define __TH_LOG(fmt, ...) \ - fprintf(TH_LOG_STREAM, "%s:%d:%s:" fmt "\n", \ + fprintf(TH_LOG_STREAM, "# %s:%d:%s:" fmt "\n", \ __FILE__, __LINE__, _metadata->name, ##__VA_ARGS__) /** @@ -119,7 +123,7 @@ */ #define XFAIL(statement, fmt, ...) do { \ if (TH_LOG_ENABLED) { \ - fprintf(TH_LOG_STREAM, "[ XFAIL! ] " fmt "\n", \ + fprintf(TH_LOG_STREAM, "# XFAIL " fmt "\n", \ ##__VA_ARGS__); \ } \ /* TODO: find a way to pass xfail to test runner process. */ \ @@ -813,12 +817,12 @@ static void __timeout_handler(int sig, siginfo_t *info, void *ucontext) /* Sanity check handler execution environment. */ if (!t) { fprintf(TH_LOG_STREAM, - "no active test in SIGALRM handler!?\n"); + "# no active test in SIGALRM handler!?\n"); abort(); } if (sig != SIGALRM || sig != info->si_signo) { fprintf(TH_LOG_STREAM, - "%s: SIGALRM handler caught signal %d!?\n", + "# %s: SIGALRM handler caught signal %d!?\n", t->name, sig != SIGALRM ? sig : info->si_signo); abort(); } @@ -839,7 +843,7 @@ void __wait_for_test(struct __test_metadata *t) if (sigaction(SIGALRM, &action, &saved_action)) { t->passed = 0; fprintf(TH_LOG_STREAM, - "%s: unable to install SIGALRM handler\n", + "# %s: unable to install SIGALRM handler\n", t->name); return; } @@ -851,7 +855,7 @@ void __wait_for_test(struct __test_metadata *t) if (sigaction(SIGALRM, &saved_action, NULL)) { t->passed = 0; fprintf(TH_LOG_STREAM, - "%s: unable to uninstall SIGALRM handler\n", + "# %s: unable to uninstall SIGALRM handler\n", t->name); return; } @@ -860,18 +864,17 @@ void __wait_for_test(struct __test_metadata *t) if (t->timed_out) { t->passed = 0; fprintf(TH_LOG_STREAM, - "%s: Test terminated by timeout\n", t->name); + "# %s: Test terminated by timeout\n", t->name); } else if (WIFEXITED(status)) { t->passed = t->termsig == -1 ? !WEXITSTATUS(status) : 0; if (t->termsig != -1) { fprintf(TH_LOG_STREAM, - "%s: Test exited normally " - "instead of by signal (code: %d)\n", + "# %s: Test exited normally instead of by signal (code: %d)\n", t->name, WEXITSTATUS(status)); } else if (!t->passed) { fprintf(TH_LOG_STREAM, - "%s: Test failed at step #%d\n", + "# %s: Test failed at step #%d\n", t->name, WEXITSTATUS(status)); } @@ -879,20 +882,19 @@ void __wait_for_test(struct __test_metadata *t) t->passed = 0; if (WTERMSIG(status) == SIGABRT) { fprintf(TH_LOG_STREAM, - "%s: Test terminated by assertion\n", + "# %s: Test terminated by assertion\n", t->name); } else if (WTERMSIG(status) == t->termsig) { t->passed = 1; } else { fprintf(TH_LOG_STREAM, - "%s: Test terminated unexpectedly " - "by signal %d\n", + "# %s: Test terminated unexpectedly by signal %d\n", t->name, WTERMSIG(status)); } } else { fprintf(TH_LOG_STREAM, - "%s: Test ended in some other way [%u]\n", + "# %s: Test ended in some other way [%u]\n", t->name, status); } @@ -908,11 +910,11 @@ void __run_test(struct __fixture_metadata *f, t->step = 0; t->no_print = 0; - printf("[ RUN ] %s%s%s.%s\n", + ksft_print_msg(" RUN %s%s%s.%s ...\n", f->name, variant->name[0] ? "." : "", variant->name, t->name); t->pid = fork(); if (t->pid < 0) { - printf("ERROR SPAWNING TEST CHILD\n"); + ksft_print_msg("ERROR SPAWNING TEST CHILD\n"); t->passed = 0; } else if (t->pid == 0) { t->fn(t, variant); @@ -921,7 +923,9 @@ void __run_test(struct __fixture_metadata *f, } else { __wait_for_test(t); } - printf("[ %4s ] %s%s%s.%s\n", (t->passed ? "OK" : "FAIL"), + ksft_print_msg(" %4s %s%s%s.%s\n", t->passed ? "OK" : "FAIL", + f->name, variant->name[0] ? "." : "", variant->name, t->name); + ksft_test_result(t->passed, "%s%s%s.%s\n", f->name, variant->name[0] ? "." : "", variant->name, t->name); } @@ -945,8 +949,9 @@ static int test_harness_run(int __attribute__((unused)) argc, } } - /* TODO(wad) add optional arguments similar to gtest. */ - printf("[==========] Running %u tests from %u test cases.\n", + ksft_print_header(); + ksft_set_plan(test_count); + ksft_print_msg("Starting %u tests from %u test cases.\n", test_count, case_count); for (f = __fixture_list; f; f = f->next) { for (v = f->variant ?: &no_variant; v; v = v->next) { @@ -960,9 +965,12 @@ static int test_harness_run(int __attribute__((unused)) argc, } } } - printf("[==========] %u / %u tests passed.\n", pass_count, count); - printf("[ %s ]\n", (ret ? "FAILED" : "PASSED")); - return ret; + ksft_print_msg("%s: %u / %u tests passed.\n", ret ? "FAILED" : "PASSED", + pass_count, count); + ksft_exit(ret == 0); + + /* unreachable */ + return KSFT_FAIL; } static void __attribute__((constructor)) __constructor_order_first(void) -- cgit v1.2.3 From 9847d24af95c7fa146c9a0e82505a78e29161c10 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 Jun 2020 11:16:49 -0700 Subject: selftests/harness: Refactor XFAIL into SKIP Plumb the old XFAIL result into a TAP SKIP. Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest_harness.h | 64 ++++++++++++++++++++------- tools/testing/selftests/seccomp/seccomp_bpf.c | 8 ++-- 2 files changed, 52 insertions(+), 20 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index f8f7e47c739a..b519765904a6 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -112,22 +112,22 @@ __FILE__, __LINE__, _metadata->name, ##__VA_ARGS__) /** - * XFAIL(statement, fmt, ...) + * SKIP(statement, fmt, ...) * - * @statement: statement to run after reporting XFAIL + * @statement: statement to run after reporting SKIP * @fmt: format string * @...: optional arguments * - * This forces a "pass" after reporting a failure with an XFAIL prefix, + * This forces a "pass" after reporting why something is being skipped * and runs "statement", which is usually "return" or "goto skip". */ -#define XFAIL(statement, fmt, ...) do { \ +#define SKIP(statement, fmt, ...) do { \ if (TH_LOG_ENABLED) { \ - fprintf(TH_LOG_STREAM, "# XFAIL " fmt "\n", \ + fprintf(TH_LOG_STREAM, "# SKIP " fmt "\n", \ ##__VA_ARGS__); \ } \ - /* TODO: find a way to pass xfail to test runner process. */ \ _metadata->passed = 1; \ + _metadata->skip = 1; \ _metadata->trigger = 0; \ statement; \ } while (0) @@ -777,6 +777,7 @@ struct __test_metadata { struct __fixture_metadata *fixture; int termsig; int passed; + int skip; /* did SKIP get used? */ 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? */ @@ -866,17 +867,31 @@ void __wait_for_test(struct __test_metadata *t) fprintf(TH_LOG_STREAM, "# %s: Test terminated by timeout\n", t->name); } else if (WIFEXITED(status)) { - t->passed = t->termsig == -1 ? !WEXITSTATUS(status) : 0; if (t->termsig != -1) { + t->passed = 0; fprintf(TH_LOG_STREAM, "# %s: Test exited normally instead of by signal (code: %d)\n", t->name, WEXITSTATUS(status)); - } else if (!t->passed) { - fprintf(TH_LOG_STREAM, - "# %s: Test failed at step #%d\n", - t->name, - WEXITSTATUS(status)); + } else { + switch (WEXITSTATUS(status)) { + /* Success */ + case 0: + t->passed = 1; + break; + /* SKIP */ + case 255: + t->passed = 1; + t->skip = 1; + break; + /* Other failure, assume step report. */ + default: + t->passed = 0; + fprintf(TH_LOG_STREAM, + "# %s: Test failed at step #%d\n", + t->name, + WEXITSTATUS(status)); + } } } else if (WIFSIGNALED(status)) { t->passed = 0; @@ -906,6 +921,7 @@ void __run_test(struct __fixture_metadata *f, { /* reset test struct */ t->passed = 1; + t->skip = 0; t->trigger = 0; t->step = 0; t->no_print = 0; @@ -918,15 +934,31 @@ void __run_test(struct __fixture_metadata *f, t->passed = 0; } else if (t->pid == 0) { t->fn(t, variant); - /* return the step that failed or 0 */ - _exit(t->passed ? 0 : t->step); + /* Make sure step doesn't get lost in reporting */ + if (t->step >= 255) { + ksft_print_msg("Too many test steps (%u)!?\n", t->step); + t->step = 254; + } + /* Use 255 for SKIP */ + if (t->skip) + _exit(255); + /* Pass is exit 0 */ + if (t->passed) + _exit(0); + /* Something else happened, report the step. */ + _exit(t->step); } else { __wait_for_test(t); } ksft_print_msg(" %4s %s%s%s.%s\n", t->passed ? "OK" : "FAIL", f->name, variant->name[0] ? "." : "", variant->name, t->name); - ksft_test_result(t->passed, "%s%s%s.%s\n", - f->name, variant->name[0] ? "." : "", variant->name, t->name); + + if (t->skip) + ksft_test_result_skip("%s%s%s.%s\n", + f->name, variant->name[0] ? "." : "", variant->name, t->name); + else + ksft_test_result(t->passed, "%s%s%s.%s\n", + f->name, variant->name[0] ? "." : "", variant->name, t->name); } static int test_harness_run(int __attribute__((unused)) argc, diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 252140a52553..8c1cc8033c09 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3069,7 +3069,7 @@ TEST(get_metadata) /* Only real root can get metadata. */ if (geteuid()) { - XFAIL(return, "get_metadata requires real root"); + SKIP(return, "get_metadata requires real root"); return; } @@ -3112,7 +3112,7 @@ TEST(get_metadata) ret = ptrace(PTRACE_SECCOMP_GET_METADATA, pid, sizeof(md), &md); EXPECT_EQ(sizeof(md), ret) { if (errno == EINVAL) - XFAIL(goto skip, "Kernel does not support PTRACE_SECCOMP_GET_METADATA (missing CONFIG_CHECKPOINT_RESTORE?)"); + SKIP(goto skip, "Kernel does not support PTRACE_SECCOMP_GET_METADATA (missing CONFIG_CHECKPOINT_RESTORE?)"); } EXPECT_EQ(md.flags, SECCOMP_FILTER_FLAG_LOG); @@ -3673,7 +3673,7 @@ TEST(user_notification_continue) resp.val = 0; EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0) { if (errno == EINVAL) - XFAIL(goto skip, "Kernel does not support SECCOMP_USER_NOTIF_FLAG_CONTINUE"); + SKIP(goto skip, "Kernel does not support SECCOMP_USER_NOTIF_FLAG_CONTINUE"); } skip: @@ -3681,7 +3681,7 @@ skip: EXPECT_EQ(true, WIFEXITED(status)); EXPECT_EQ(0, WEXITSTATUS(status)) { if (WEXITSTATUS(status) == 2) { - XFAIL(return, "Kernel does not support kcmp() syscall"); + SKIP(return, "Kernel does not support kcmp() syscall"); return; } } -- cgit v1.2.3 From d088c92802549fc1cf77a12a4e3986160d63662a Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 Jun 2020 11:16:50 -0700 Subject: selftests/harness: Display signed values correctly Since forever the harness output for signed value tests have reported unsigned values to avoid casting. Instead, actually test the variable types and perform the correct casts and choose the correct format specifiers. Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest_harness.h | 42 +++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 5 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index b519765904a6..ae51b762d120 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -679,17 +679,49 @@ if (_metadata->passed && _metadata->step < 255) \ _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)) { \ - unsigned long long __exp_print = (uintptr_t)__exp; \ - unsigned long long __seen_print = (uintptr_t)__seen; \ - __TH_LOG("Expected %s (%llu) %s %s (%llu)", \ - _expected_str, __exp_print, #_t, \ - _seen_str, __seen_print); \ + /* Report with actual signedness to avoid weird output. */ \ + switch (is_signed_type(__exp) * 2 + is_signed_type(__seen)) { \ + case 0: { \ + unsigned long long __exp_print = (uintptr_t)__exp; \ + unsigned long long __seen_print = (uintptr_t)__seen; \ + __TH_LOG("Expected %s (%llu) %s %s (%llu)", \ + _expected_str, __exp_print, #_t, \ + _seen_str, __seen_print); \ + break; \ + } \ + case 1: { \ + unsigned long long __exp_print = (uintptr_t)__exp; \ + long long __seen_print = (intptr_t)__seen; \ + __TH_LOG("Expected %s (%llu) %s %s (%lld)", \ + _expected_str, __exp_print, #_t, \ + _seen_str, __seen_print); \ + break; \ + } \ + case 2: { \ + long long __exp_print = (intptr_t)__exp; \ + unsigned long long __seen_print = (uintptr_t)__seen; \ + __TH_LOG("Expected %s (%lld) %s %s (%llu)", \ + _expected_str, __exp_print, #_t, \ + _seen_str, __seen_print); \ + break; \ + } \ + case 3: { \ + long long __exp_print = (intptr_t)__exp; \ + long long __seen_print = (intptr_t)__seen; \ + __TH_LOG("Expected %s (%lld) %s %s (%lld)", \ + _expected_str, __exp_print, #_t, \ + _seen_str, __seen_print); \ + break; \ + } \ + } \ _metadata->passed = 0; \ /* Ensure the optional handler is triggered */ \ _metadata->trigger = 1; \ -- cgit v1.2.3 From 0ef67a888375b0d11930a9811e1152b6ebc385a8 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 Jun 2020 11:16:51 -0700 Subject: selftests/harness: Report skip reason Use a share memory segment to pass string information between forked test and the test runner for the skip reason. Signed-off-by: Kees Cook Signed-off-by: Shuah Khan --- tools/testing/selftests/kselftest_harness.h | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index ae51b762d120..438c19740838 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -60,6 +60,7 @@ #include #include #include +#include #include #include #include @@ -122,9 +123,11 @@ * and runs "statement", which is usually "return" or "goto skip". */ #define SKIP(statement, fmt, ...) do { \ + snprintf(_metadata->results->reason, \ + sizeof(_metadata->results->reason), fmt, ##__VA_ARGS__); \ if (TH_LOG_ENABLED) { \ - fprintf(TH_LOG_STREAM, "# SKIP " fmt "\n", \ - ##__VA_ARGS__); \ + fprintf(TH_LOG_STREAM, "# SKIP %s\n", \ + _metadata->results->reason); \ } \ _metadata->passed = 1; \ _metadata->skip = 1; \ @@ -762,6 +765,10 @@ } \ } +struct __test_results { + char reason[1024]; /* Reason for test result */ +}; + struct __test_metadata; struct __fixture_variant_metadata; @@ -815,6 +822,7 @@ struct __test_metadata { bool timed_out; /* did this test timeout instead of exiting? */ __u8 step; bool no_print; /* manual trigger when TH_LOG_STREAM is not available */ + struct __test_results *results; struct __test_metadata *prev, *next; }; @@ -957,6 +965,7 @@ void __run_test(struct __fixture_metadata *f, t->trigger = 0; t->step = 0; t->no_print = 0; + memset(t->results->reason, 0, sizeof(t->results->reason)); ksft_print_msg(" RUN %s%s%s.%s ...\n", f->name, variant->name[0] ? "." : "", variant->name, t->name); @@ -986,8 +995,8 @@ void __run_test(struct __fixture_metadata *f, f->name, variant->name[0] ? "." : "", variant->name, t->name); if (t->skip) - ksft_test_result_skip("%s%s%s.%s\n", - f->name, variant->name[0] ? "." : "", variant->name, t->name); + ksft_test_result_skip("%s\n", t->results->reason[0] ? + t->results->reason : "unknown"); else ksft_test_result(t->passed, "%s%s%s.%s\n", f->name, variant->name[0] ? "." : "", variant->name, t->name); @@ -999,6 +1008,7 @@ static int test_harness_run(int __attribute__((unused)) argc, struct __fixture_variant_metadata no_variant = { .name = "", }; struct __fixture_variant_metadata *v; struct __fixture_metadata *f; + struct __test_results *results; struct __test_metadata *t; int ret = 0; unsigned int case_count = 0, test_count = 0; @@ -1013,6 +1023,9 @@ static int test_harness_run(int __attribute__((unused)) argc, } } + results = mmap(NULL, sizeof(*results), PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, -1, 0); + ksft_print_header(); ksft_set_plan(test_count); ksft_print_msg("Starting %u tests from %u test cases.\n", @@ -1021,7 +1034,9 @@ static int test_harness_run(int __attribute__((unused)) argc, for (v = f->variant ?: &no_variant; v; v = v->next) { for (t = f->tests; t; t = t->next) { count++; + t->results = results; __run_test(f, v, t); + t->results = NULL; if (t->passed) pass_count++; else @@ -1029,6 +1044,8 @@ static int test_harness_run(int __attribute__((unused)) argc, } } } + munmap(results, sizeof(*results)); + ksft_print_msg("%s: %u / %u tests passed.\n", ret ? "FAILED" : "PASSED", pass_count, count); ksft_exit(ret == 0); -- cgit v1.2.3 From bb91c0ca7b74efde2257ca4f0b1ba93e834e47e1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 7 Jul 2020 11:39:26 -0400 Subject: selftests: pidfd: do not use ksft_exit_skip after ksft_set_plan Calling ksft_exit_skip after ksft_set_plan results in executing fewer tests than planned. Use ksft_test_result_skip instead. The plan passed to ksft_set_plan was wrong, too, so fix it while at it. Acked-by: Christian Brauner Signed-off-by: Paolo Bonzini Signed-off-by: Shuah Khan --- tools/testing/selftests/pidfd/pidfd_test.c | 34 ++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index 7aff2d3b42c0..d2b001425cf8 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -27,6 +28,8 @@ #define MAX_EVENTS 5 +static bool have_pidfd_send_signal; + static pid_t pidfd_clone(int flags, int *pidfd, int (*fn)(void *)) { size_t stack_size = 1024; @@ -56,6 +59,13 @@ static int test_pidfd_send_signal_simple_success(void) int pidfd, ret; const char *test_name = "pidfd_send_signal send SIGUSR1"; + if (!have_pidfd_send_signal) { + ksft_test_result_skip( + "%s test: pidfd_send_signal() syscall not supported\n", + test_name); + return 0; + } + pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC); if (pidfd < 0) ksft_exit_fail_msg( @@ -86,6 +96,13 @@ static int test_pidfd_send_signal_exited_fail(void) pid_t pid; const char *test_name = "pidfd_send_signal signal exited process"; + if (!have_pidfd_send_signal) { + ksft_test_result_skip( + "%s test: pidfd_send_signal() syscall not supported\n", + test_name); + return 0; + } + pid = fork(); if (pid < 0) ksft_exit_fail_msg("%s test: Failed to create new process\n", @@ -137,6 +154,13 @@ static int test_pidfd_send_signal_recycled_pid_fail(void) pid_t pid1; const char *test_name = "pidfd_send_signal signal recycled pid"; + if (!have_pidfd_send_signal) { + ksft_test_result_skip( + "%s test: pidfd_send_signal() syscall not supported\n", + test_name); + return 0; + } + ret = unshare(CLONE_NEWPID); if (ret < 0) ksft_exit_fail_msg("%s test: Failed to unshare pid namespace\n", @@ -325,15 +349,17 @@ static int test_pidfd_send_signal_syscall_support(void) ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0); if (ret < 0) { - if (errno == ENOSYS) - ksft_exit_skip( + if (errno == ENOSYS) { + ksft_test_result_skip( "%s test: pidfd_send_signal() syscall not supported\n", test_name); - + return 0; + } ksft_exit_fail_msg("%s test: Failed to send signal\n", test_name); } + have_pidfd_send_signal = true; close(pidfd); ksft_test_result_pass( "%s test: pidfd_send_signal() syscall is supported. Tests can be executed\n", @@ -521,7 +547,7 @@ static void test_pidfd_poll_leader_exit(int use_waitpid) int main(int argc, char **argv) { ksft_print_header(); - ksft_set_plan(4); + ksft_set_plan(8); test_pidfd_poll_exec(0); test_pidfd_poll_exec(1); -- cgit v1.2.3 From 05790fd7f8d3a77c26f1acd16b6d0f8f7d61d98a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 7 Jul 2020 11:39:27 -0400 Subject: selftests: pidfd: skip test if unshare fails with EPERM Similar to how ENOSYS causes a skip if pidfd_send_signal is not present, we can do the same for unshare if it fails with EPERM. This way, running the test without privileges causes four tests to skip but no early bail out. Acked-by: Christian Brauner Signed-off-by: Paolo Bonzini Signed-off-by: Shuah Khan --- tools/testing/selftests/pidfd/pidfd_test.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index d2b001425cf8..c585aaa2acd8 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -162,15 +162,26 @@ static int test_pidfd_send_signal_recycled_pid_fail(void) } ret = unshare(CLONE_NEWPID); - if (ret < 0) + if (ret < 0) { + if (errno == EPERM) { + ksft_test_result_skip("%s test: Unsharing pid namespace not permitted\n", + test_name); + return 0; + } ksft_exit_fail_msg("%s test: Failed to unshare pid namespace\n", test_name); + } ret = unshare(CLONE_NEWNS); - if (ret < 0) - ksft_exit_fail_msg( - "%s test: Failed to unshare mount namespace\n", - test_name); + if (ret < 0) { + if (errno == EPERM) { + ksft_test_result_skip("%s test: Unsharing mount namespace not permitted\n", + test_name); + return 0; + } + ksft_exit_fail_msg("%s test: Failed to unshare mount namespace\n", + test_name); + } ret = mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, 0); if (ret < 0) -- cgit v1.2.3 From 99aacebecb75c61e6a6a1bc29f5d87c9453f3b73 Mon Sep 17 00:00:00 2001 From: Yauheni Kaliuta Date: Wed, 27 May 2020 10:16:57 +0300 Subject: selftests: do not use .ONESHELL Using one shell for the whole recipe with long lists can cause make[1]: execvp: /bin/sh: Argument list too long with some shells. Triggered by commit 309b81f0fdc4 ("selftests/bpf: Install generated test progs") It requires to change the rule which rely on the one shell behaviour (run_tests). Simplify also INSTALL_SINGLE_RULE, remove extra echo, required to workaround .ONESHELL. Signed-off-by: Yauheni Kaliuta Cc: Jiri Benc Cc: Shuah Khan Signed-off-by: Shuah Khan --- tools/testing/selftests/lib.mk | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index b0556c752443..5b82433d88e3 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -59,9 +59,8 @@ else all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) endif -.ONESHELL: define RUN_TESTS - @BASE_DIR="$(selfdir)"; \ + BASE_DIR="$(selfdir)"; \ . $(selfdir)/kselftest/runner.sh; \ if [ "X$(summary)" != "X" ]; then \ per_test_logging=1; \ @@ -71,22 +70,21 @@ endef run_tests: all ifdef building_out_of_srctree - @if [ "X$(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)" != "X" ]; then - @rsync -aq $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT) + @if [ "X$(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)" != "X" ]; then \ + rsync -aq $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT); \ fi - @if [ "X$(TEST_PROGS)" != "X" ]; then - $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(OUTPUT)/$(TEST_PROGS)) - else - $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS)) + @if [ "X$(TEST_PROGS)" != "X" ]; then \ + $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(OUTPUT)/$(TEST_PROGS)) ; \ + else \ + $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS)); \ fi else - $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS)) + @$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) $(TEST_PROGS)) endif define INSTALL_SINGLE_RULE $(if $(INSTALL_LIST),@mkdir -p $(INSTALL_PATH)) - $(if $(INSTALL_LIST),@echo rsync -a $(INSTALL_LIST) $(INSTALL_PATH)/) - $(if $(INSTALL_LIST),@rsync -a $(INSTALL_LIST) $(INSTALL_PATH)/) + $(if $(INSTALL_LIST),rsync -a $(INSTALL_LIST) $(INSTALL_PATH)/) endef define INSTALL_RULE -- cgit v1.2.3 From c9f75047eb9b38b2461819398e8a30a172ca3d46 Mon Sep 17 00:00:00 2001 From: Yauheni Kaliuta Date: Wed, 27 May 2020 10:16:58 +0300 Subject: selftests: fix condition in run_tests The check if there are any files to install in case of no files compares "X " with "X" so never false. Remove extra spaces. It may make sense to use make's $(if) function here. Signed-off-by: Yauheni Kaliuta Signed-off-by: Shuah Khan --- tools/testing/selftests/lib.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/testing') diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index 5b82433d88e3..7a17ea815736 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -70,7 +70,7 @@ endef run_tests: all ifdef building_out_of_srctree - @if [ "X$(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES)" != "X" ]; then \ + @if [ "X