This series converts the execveat test to generate KTAP output so it plays a bit more nicely with automation, KTAP means that kselftest runners can track the individual tests in the suite rather than just an overall pass/fail for the suite as a whole.
The first patch adding a perror() equivalent for kselftest was previously sent as part of a similar conversion for the timers tests:
https://lore.kernel.org/linux-kselftest/8734yyfx00.ffs@tglx/T
there's probably no harm in applying it twice or possibly these should both go via the kselftest tree - I'm not sure who usually applies timers test changes.
Signed-off-by: Mark Brown broonie@kernel.org --- Mark Brown (2): kselftest: Add a ksft_perror() helper selftests/exec: Convert execveat test to generate KTAP output
tools/testing/selftests/exec/execveat.c | 87 ++++++++++++++++++++------------- tools/testing/selftests/kselftest.h | 14 ++++++ 2 files changed, 66 insertions(+), 35 deletions(-) --- base-commit: 6465e260f48790807eef06b583b38ca9789b6072 change-id: 20230928-ktap-exec-45ea8d28309a
Best regards,
The standard library perror() function provides a convenient way to print an error message based on the current errno but this doesn't play nicely with KTAP output. Provide a helper which does an equivalent thing in a KTAP compatible format.
nolibc doesn't have a strerror() and adding the table of strings required doesn't seem like a good fit for what it's trying to do so when we're using that only print the errno.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/kselftest.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h index 529d29a35900..af9f1202d423 100644 --- a/tools/testing/selftests/kselftest.h +++ b/tools/testing/selftests/kselftest.h @@ -48,6 +48,7 @@ #include <stdlib.h> #include <unistd.h> #include <stdarg.h> +#include <string.h> #include <stdio.h> #endif
@@ -155,6 +156,19 @@ static inline void ksft_print_msg(const char *msg, ...) va_end(args); }
+static inline void ksft_perror(const char *msg) +{ +#ifndef NOLIBC + ksft_print_msg("%s: %s (%d)\n", msg, strerror(errno), errno); +#else + /* + * nolibc doesn't provide strerror() and it seems + * inappropriate to add one, just print the errno. + */ + ksft_print_msg("%s: %d)\n", msg, errno); +#endif +} + static inline void ksft_test_result_pass(const char *msg, ...) { int saved_errno = errno;
On Thu, Sep 28, 2023 at 04:38:11PM +0200, Mark Brown wrote:
The standard library perror() function provides a convenient way to print an error message based on the current errno but this doesn't play nicely with KTAP output. Provide a helper which does an equivalent thing in a KTAP compatible format.
nolibc doesn't have a strerror() and adding the table of strings required doesn't seem like a good fit for what it's trying to do so when we're using that only print the errno.
Signed-off-by: Mark Brown broonie@kernel.org
Oh, interesting... what environment ends up without strerror()?
Reviewed-by: Kees Cook keescook@chromium.org
On Thu, Sep 28, 2023 at 05:48:22PM -0700, Kees Cook wrote:
nolibc doesn't have a strerror() and adding the table of strings required doesn't seem like a good fit for what it's trying to do so when we're using that only print the errno.
Oh, interesting... what environment ends up without strerror()?
Like I say it's for nolibc - it's just some header files (all in the kernel source), while it generally aims to be libc compatible it's intentionally very small.
On Fri, Sep 29, 2023 at 09:50:53AM +0200, Mark Brown wrote:
On Thu, Sep 28, 2023 at 05:48:22PM -0700, Kees Cook wrote:
nolibc doesn't have a strerror() and adding the table of strings required doesn't seem like a good fit for what it's trying to do so when we're using that only print the errno.
Oh, interesting... what environment ends up without strerror()?
Like I say it's for nolibc - it's just some header files (all in the kernel source), while it generally aims to be libc compatible it's intentionally very small.
Right, I mean, how would one normally encounter this environment? Running the selftests on m68k userspace or something?
On Fri, Sep 29, 2023 at 10:31:56AM -0700, Kees Cook wrote:
On Fri, Sep 29, 2023 at 09:50:53AM +0200, Mark Brown wrote:
Like I say it's for nolibc - it's just some header files (all in the kernel source), while it generally aims to be libc compatible it's intentionally very small.
Right, I mean, how would one normally encounter this environment? Running the selftests on m68k userspace or something?
There's a bunch of selftests that cover interfaces that are intended to be used by libc which are built with nolibc in order to avoid the tests and glibc stomping over each other.
Currently the execveat test does not produce KTAP output but rather a custom format. This means that we only get a pass/fail for the suite, not for each individual test that the suite does. Convert to using the standard kselftest output functions which result in KTAP output being generated.
The main trick with this is that, being an exec() related test, the program executes itself and returns specific exit codes to verify success meaning that we need to only use the top level kselftest header/summary functions when invoked directly rather than when run as part of a test.
Signed-off-by: Mark Brown broonie@kernel.org --- tools/testing/selftests/exec/execveat.c | 87 ++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c index 67bf7254a48f..bf79d664c8e6 100644 --- a/tools/testing/selftests/exec/execveat.c +++ b/tools/testing/selftests/exec/execveat.c @@ -23,6 +23,9 @@
#include "../kselftest.h"
+#define TESTS_EXPECTED 51 +#define TEST_NAME_LEN (PATH_MAX * 4) + static char longpath[2 * PATH_MAX] = ""; static char *envp[] = { "IN_TEST=yes", NULL, NULL }; static char *argv[] = { "execveat", "99", NULL }; @@ -43,71 +46,85 @@ static int execveat_(int fd, const char *path, char **argv, char **envp, static int _check_execveat_fail(int fd, const char *path, int flags, int expected_errno, const char *errno_str) { + char test_name[TEST_NAME_LEN]; int rc;
errno = 0; - printf("Check failure of execveat(%d, '%s', %d) with %s... ", - fd, path?:"(null)", flags, errno_str); + snprintf(test_name, sizeof(test_name), + "Check failure of execveat(%d, '%s', %d) with %s", + fd, path?:"(null)", flags, errno_str); rc = execveat_(fd, path, argv, envp, flags);
if (rc > 0) { - printf("[FAIL] (unexpected success from execveat(2))\n"); + ksft_print_msg("unexpected success from execveat(2)\n"); + ksft_test_result_fail("%s\n", test_name); return 1; } if (errno != expected_errno) { - printf("[FAIL] (expected errno %d (%s) not %d (%s)\n", - expected_errno, strerror(expected_errno), - errno, strerror(errno)); + ksft_print_msg("expected errno %d (%s) not %d (%s)\n", + expected_errno, strerror(expected_errno), + errno, strerror(errno)); + ksft_test_result_fail("%s\n", test_name); return 1; } - printf("[OK]\n"); + ksft_test_result_pass("%s\n", test_name); return 0; }
static int check_execveat_invoked_rc(int fd, const char *path, int flags, int expected_rc, int expected_rc2) { + char test_name[TEST_NAME_LEN]; int status; int rc; pid_t child; int pathlen = path ? strlen(path) : 0;
if (pathlen > 40) - printf("Check success of execveat(%d, '%.20s...%s', %d)... ", - fd, path, (path + pathlen - 20), flags); + snprintf(test_name, sizeof(test_name), + "Check success of execveat(%d, '%.20s...%s', %d)... ", + fd, path, (path + pathlen - 20), flags); else - printf("Check success of execveat(%d, '%s', %d)... ", - fd, path?:"(null)", flags); + snprintf(test_name, sizeof(test_name), + "Check success of execveat(%d, '%s', %d)... ", + fd, path?:"(null)", flags); + child = fork(); if (child < 0) { - printf("[FAIL] (fork() failed)\n"); + ksft_perror("fork() failed"); + ksft_test_result_fail("%s\n", test_name); return 1; } if (child == 0) { /* Child: do execveat(). */ rc = execveat_(fd, path, argv, envp, flags); - printf("[FAIL]: execveat() failed, rc=%d errno=%d (%s)\n", - rc, errno, strerror(errno)); + ksft_print_msg("execveat() failed, rc=%d errno=%d (%s)\n", + rc, errno, strerror(errno)); + ksft_test_result_fail("%s\n", test_name); exit(1); /* should not reach here */ } /* Parent: wait for & check child's exit status. */ rc = waitpid(child, &status, 0); if (rc != child) { - printf("[FAIL] (waitpid(%d,...) returned %d)\n", child, rc); + ksft_print_msg("waitpid(%d,...) returned %d\n", child, rc); + ksft_test_result_fail("%s\n", test_name); return 1; } if (!WIFEXITED(status)) { - printf("[FAIL] (child %d did not exit cleanly, status=%08x)\n", - child, status); + ksft_print_msg("child %d did not exit cleanly, status=%08x\n", + child, status); + ksft_test_result_fail("%s\n", test_name); return 1; } if ((WEXITSTATUS(status) != expected_rc) && (WEXITSTATUS(status) != expected_rc2)) { - printf("[FAIL] (child %d exited with %d not %d nor %d)\n", - child, WEXITSTATUS(status), expected_rc, expected_rc2); + ksft_print_msg("child %d exited with %d not %d nor %d\n", + child, WEXITSTATUS(status), expected_rc, + expected_rc2); + ksft_test_result_fail("%s\n", test_name); return 1; } - printf("[OK]\n"); + ksft_test_result_pass("%s\n", test_name); return 0; }
@@ -129,11 +146,9 @@ static int open_or_die(const char *filename, int flags) { int fd = open(filename, flags);
- if (fd < 0) { - printf("Failed to open '%s'; " + if (fd < 0) + ksft_exit_fail_msg("Failed to open '%s'; " "check prerequisites are available\n", filename); - exit(1); - } return fd; }
@@ -162,8 +177,7 @@ static int check_execveat_pathmax(int root_dfd, const char *src, int is_script) char *cwd = getcwd(NULL, 0);
if (!cwd) { - printf("Failed to getcwd(), errno=%d (%s)\n", - errno, strerror(errno)); + ksft_perror("Failed to getcwd()"); return 2; } strcpy(longpath, cwd); @@ -193,12 +207,12 @@ static int check_execveat_pathmax(int root_dfd, const char *src, int is_script) */ fd = open(longpath, O_RDONLY); if (fd > 0) { - printf("Invoke copy of '%s' via filename of length %zu:\n", - src, strlen(longpath)); + ksft_print_msg("Invoke copy of '%s' via filename of length %zu:\n", + src, strlen(longpath)); fail += check_execveat(fd, "", AT_EMPTY_PATH); } else { - printf("Failed to open length %zu filename, errno=%d (%s)\n", - strlen(longpath), errno, strerror(errno)); + ksft_print_msg("Failed to open length %zu filename, errno=%d (%s)\n", + strlen(longpath), errno, strerror(errno)); fail++; }
@@ -405,28 +419,31 @@ int main(int argc, char **argv) const char *in_test = getenv("IN_TEST");
if (verbose) { - printf(" invoked with:"); + ksft_print_msg("invoked with:\n"); for (ii = 0; ii < argc; ii++) - printf(" [%d]='%s'", ii, argv[ii]); - printf("\n"); + ksft_print_msg("\t[%d]='%s\n'", ii, argv[ii]); }
/* Check expected environment transferred. */ if (!in_test || strcmp(in_test, "yes") != 0) { - printf("[FAIL] (no IN_TEST=yes in env)\n"); + ksft_print_msg("no IN_TEST=yes in env\n"); return 1; }
/* Use the final argument as an exit code. */ rc = atoi(argv[argc - 1]); - fflush(stdout); + exit(rc); } else { + ksft_print_header(); + ksft_set_plan(TESTS_EXPECTED); prerequisites(); if (verbose) envp[1] = "VERBOSE=1"; rc = run_tests(); if (rc > 0) printf("%d tests failed\n", rc); + ksft_finished(); } + return rc; }
On Thu, Sep 28, 2023 at 04:38:12PM +0200, Mark Brown wrote:
Currently the execveat test does not produce KTAP output but rather a custom format. This means that we only get a pass/fail for the suite, not for each individual test that the suite does. Convert to using the standard kselftest output functions which result in KTAP output being generated.
The main trick with this is that, being an exec() related test, the program executes itself and returns specific exit codes to verify success meaning that we need to only use the top level kselftest header/summary functions when invoked directly rather than when run as part of a test.
Signed-off-by: Mark Brown broonie@kernel.org
Yay! More KTAP! :)
Reviewed-by: Kees Cook keescook@chromium.org
linux-kselftest-mirror@lists.linaro.org
-
Kees Cook -
Mark Brown