The following line is missing from the test's execution. Add it to make it fully TAP conformant: # Totals: pass:27 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/testing/selftests/exec/binfmt_script.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/exec/binfmt_script.py b/tools/testing/selftests/exec/binfmt_script.py index 05f94a741c7aa..2c575a2c0eab4 100755 --- a/tools/testing/selftests/exec/binfmt_script.py +++ b/tools/testing/selftests/exec/binfmt_script.py @@ -16,6 +16,8 @@ SIZE=256 NAME_MAX=int(subprocess.check_output(["getconf", "NAME_MAX", "."]))
test_num=0 +pass_num=0 +fail_num=0
code='''#!/usr/bin/perl print "Executed interpreter! Args:\n"; @@ -42,7 +44,7 @@ foreach my $a (@ARGV) { # ... def test(name, size, good=True, leading="", root="./", target="/perl", fill="A", arg="", newline="\n", hashbang="#!"): - global test_num, tests, NAME_MAX + global test_num, pass_num, fail_num, tests, NAME_MAX test_num += 1 if test_num > tests: raise ValueError("more binfmt_script tests than expected! (want %d, expected %d)" @@ -80,16 +82,20 @@ def test(name, size, good=True, leading="", root="./", target="/perl", if good: print("ok %d - binfmt_script %s (successful good exec)" % (test_num, name)) + pass_num += 1 else: print("not ok %d - binfmt_script %s succeeded when it should have failed" % (test_num, name)) + fail_num = 1 else: if good: print("not ok %d - binfmt_script %s failed when it should have succeeded (rc:%d)" % (test_num, name, proc.returncode)) + fail_num = 1 else: print("ok %d - binfmt_script %s (correctly failed bad exec)" % (test_num, name)) + pass_num += 1
# Clean up crazy binaries os.unlink(script) @@ -166,6 +172,8 @@ test(name="two-under-trunc-arg", size=int(SIZE/2), arg=" ") test(name="two-under-leading", size=int(SIZE/2), leading=" ") test(name="two-under-lead-trunc-arg", size=int(SIZE/2), leading=" ", arg=" ")
+print("# Totals: pass:%d fail:%d xfail:0 xpass:0 skip:0 error:0" % (pass_num, fail_num)) + if test_num != tests: raise ValueError("fewer binfmt_script tests than expected! (ran %d, expected %d" % (test_num, tests))
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- tools/testing/selftests/exec/load_address.c | 34 +++++++++------------ 1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/exec/load_address.c b/tools/testing/selftests/exec/load_address.c index d487c2f6a6150..17e3207d34ae7 100644 --- a/tools/testing/selftests/exec/load_address.c +++ b/tools/testing/selftests/exec/load_address.c @@ -5,6 +5,7 @@ #include <link.h> #include <stdio.h> #include <stdlib.h> +#include "../kselftest.h"
struct Statistics { unsigned long long load_address; @@ -41,28 +42,23 @@ int main(int argc, char **argv) unsigned long long misalign; int ret;
+ ksft_print_header(); + ksft_set_plan(1); + ret = dl_iterate_phdr(ExtractStatistics, &extracted); - if (ret != 1) { - fprintf(stderr, "FAILED\n"); - return 1; - } + if (ret != 1) + ksft_exit_fail_msg("FAILED: dl_iterate_phdr\n");
- if (extracted.alignment == 0) { - fprintf(stderr, "No alignment found\n"); - return 1; - } else if (extracted.alignment & (extracted.alignment - 1)) { - fprintf(stderr, "Alignment is not a power of 2\n"); - return 1; - } + if (extracted.alignment == 0) + ksft_exit_fail_msg("FAILED: No alignment found\n"); + else if (extracted.alignment & (extracted.alignment - 1)) + ksft_exit_fail_msg("FAILED: Alignment is not a power of 2\n");
misalign = extracted.load_address & (extracted.alignment - 1); - if (misalign) { - printf("alignment = %llu, load_address = %llu\n", - extracted.alignment, extracted.load_address); - fprintf(stderr, "FAILED\n"); - return 1; - } + if (misalign) + ksft_exit_fail_msg("FAILED: alignment = %llu, load_address = %llu\n", + extracted.alignment, extracted.load_address);
- fprintf(stderr, "PASS\n"); - return 0; + ksft_test_result_pass("Completed\n"); + ksft_finished(); }
On Mon, Mar 04, 2024 at 08:59:24PM +0500, Muhammad Usama Anjum wrote:
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/exec/load_address.c | 34 +++++++++------------ 1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/exec/load_address.c b/tools/testing/selftests/exec/load_address.c index d487c2f6a6150..17e3207d34ae7 100644 --- a/tools/testing/selftests/exec/load_address.c +++ b/tools/testing/selftests/exec/load_address.c @@ -5,6 +5,7 @@ #include <link.h> #include <stdio.h> #include <stdlib.h> +#include "../kselftest.h" struct Statistics { unsigned long long load_address; @@ -41,28 +42,23 @@ int main(int argc, char **argv) unsigned long long misalign; int ret;
- ksft_print_header();
- ksft_set_plan(1);
- ret = dl_iterate_phdr(ExtractStatistics, &extracted);
- if (ret != 1) {
fprintf(stderr, "FAILED\n");
return 1;
- }
- if (ret != 1)
ksft_exit_fail_msg("FAILED: dl_iterate_phdr\n");
I'm for this series, but I do note a weird glitch in the ksft API. ksft_exit_fail_msg does:
va_start(args, msg); printf("Bail out! "); errno = saved_errno; vprintf(msg, args); va_end(args);
"Bail out!" is not very descriptive. I think I'd rather this should be:
"FAILED: "
and then that added prefix doesn't need to be added everywhere in this patch, nor the "error: " prefix in the next patch.
On 3/5/24 1:58 AM, Kees Cook wrote:
On Mon, Mar 04, 2024 at 08:59:24PM +0500, Muhammad Usama Anjum wrote:
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/exec/load_address.c | 34 +++++++++------------ 1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/exec/load_address.c b/tools/testing/selftests/exec/load_address.c index d487c2f6a6150..17e3207d34ae7 100644 --- a/tools/testing/selftests/exec/load_address.c +++ b/tools/testing/selftests/exec/load_address.c @@ -5,6 +5,7 @@ #include <link.h> #include <stdio.h> #include <stdlib.h> +#include "../kselftest.h" struct Statistics { unsigned long long load_address; @@ -41,28 +42,23 @@ int main(int argc, char **argv) unsigned long long misalign; int ret;
- ksft_print_header();
- ksft_set_plan(1);
- ret = dl_iterate_phdr(ExtractStatistics, &extracted);
- if (ret != 1) {
fprintf(stderr, "FAILED\n");
return 1;
- }
- if (ret != 1)
ksft_exit_fail_msg("FAILED: dl_iterate_phdr\n");
I'm for this series, but I do note a weird glitch in the ksft API. ksft_exit_fail_msg does:
va_start(args, msg); printf("Bail out! "); errno = saved_errno; vprintf(msg, args); va_end(args);
"Bail out!" is not very descriptive. I think I'd rather this should be:
"FAILED: "
and then that added prefix doesn't need to be added everywhere in this patch, nor the "error: " prefix in the next patch.
If we want to make this change, FAILED should be removed from all the tests. We should do it in separate patch. I've taken note and will do it separate from this series.
Conform the layout, informational and status messages to TAP. No functional change is intended other than the layout of output messages. While at it, do minor cleanups like move the declarations of the variables on top of the function.
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com --- .../testing/selftests/exec/recursion-depth.c | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/exec/recursion-depth.c b/tools/testing/selftests/exec/recursion-depth.c index 2dbd5bc45b3ed..b2f37d86a5f62 100644 --- a/tools/testing/selftests/exec/recursion-depth.c +++ b/tools/testing/selftests/exec/recursion-depth.c @@ -23,45 +23,44 @@ #include <fcntl.h> #include <sys/mount.h> #include <unistd.h> +#include "../kselftest.h"
int main(void) { + int fd, rv; + + ksft_print_header(); + ksft_set_plan(1); + if (unshare(CLONE_NEWNS) == -1) { if (errno == ENOSYS || errno == EPERM) { - fprintf(stderr, "error: unshare, errno %d\n", errno); - return 4; + ksft_test_result_skip("error: unshare, errno %d\n", errno); + ksft_finished(); } - fprintf(stderr, "error: unshare, errno %d\n", errno); - return 1; - } - if (mount(NULL, "/", NULL, MS_PRIVATE|MS_REC, NULL) == -1) { - fprintf(stderr, "error: mount '/', errno %d\n", errno); - return 1; + ksft_exit_fail_msg("error: unshare, errno %d\n", errno); } + + if (mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL) == -1) + ksft_exit_fail_msg("error: mount '/', errno %d\n", errno); + /* Require "exec" filesystem. */ - if (mount(NULL, "/tmp", "ramfs", 0, NULL) == -1) { - fprintf(stderr, "error: mount ramfs, errno %d\n", errno); - return 1; - } + if (mount(NULL, "/tmp", "ramfs", 0, NULL) == -1) + ksft_exit_fail_msg("error: mount ramfs, errno %d\n", errno);
#define FILENAME "/tmp/1"
- int fd = creat(FILENAME, 0700); - if (fd == -1) { - fprintf(stderr, "error: creat, errno %d\n", errno); - return 1; - } + fd = creat(FILENAME, 0700); + if (fd == -1) + ksft_exit_fail_msg("error: creat, errno %d\n", errno); + #define S "#!" FILENAME "\n" - if (write(fd, S, strlen(S)) != strlen(S)) { - fprintf(stderr, "error: write, errno %d\n", errno); - return 1; - } + if (write(fd, S, strlen(S)) != strlen(S)) + ksft_exit_fail_msg("error: write, errno %d\n", errno); + close(fd);
- int rv = execve(FILENAME, NULL, NULL); - if (rv == -1 && errno == ELOOP) { - return 0; - } - fprintf(stderr, "error: execve, rv %d, errno %d\n", rv, errno); - return 1; + rv = execve(FILENAME, NULL, NULL); + ksft_test_result(rv == -1 && errno == ELOOP, + "execve failed as expected (ret %d, errno %d)\n", rv, errno); + ksft_finished(); }
On Mon, Mar 04, 2024 at 08:59:23PM +0500, Muhammad Usama Anjum wrote:
The following line is missing from the test's execution. Add it to make it fully TAP conformant: # Totals: pass:27 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
Thanks, looks good.
Reviewed-by: Kees Cook keescook@chromium.org
Soft reminder!
On 3/4/24 8:59 PM, Muhammad Usama Anjum wrote:
The following line is missing from the test's execution. Add it to make it fully TAP conformant: # Totals: pass:27 fail:0 xfail:0 xpass:0 skip:0 error:0
Signed-off-by: Muhammad Usama Anjum usama.anjum@collabora.com
tools/testing/selftests/exec/binfmt_script.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/exec/binfmt_script.py b/tools/testing/selftests/exec/binfmt_script.py index 05f94a741c7aa..2c575a2c0eab4 100755 --- a/tools/testing/selftests/exec/binfmt_script.py +++ b/tools/testing/selftests/exec/binfmt_script.py @@ -16,6 +16,8 @@ SIZE=256 NAME_MAX=int(subprocess.check_output(["getconf", "NAME_MAX", "."])) test_num=0 +pass_num=0 +fail_num=0 code='''#!/usr/bin/perl print "Executed interpreter! Args:\n"; @@ -42,7 +44,7 @@ foreach my $a (@ARGV) { # ... def test(name, size, good=True, leading="", root="./", target="/perl", fill="A", arg="", newline="\n", hashbang="#!"):
- global test_num, tests, NAME_MAX
- global test_num, pass_num, fail_num, tests, NAME_MAX test_num += 1 if test_num > tests: raise ValueError("more binfmt_script tests than expected! (want %d, expected %d)"
@@ -80,16 +82,20 @@ def test(name, size, good=True, leading="", root="./", target="/perl", if good: print("ok %d - binfmt_script %s (successful good exec)" % (test_num, name))
pass_num += 1 else: print("not ok %d - binfmt_script %s succeeded when it should have failed" % (test_num, name))
else: if good: print("not ok %d - binfmt_script %s failed when it should have succeeded (rc:%d)" % (test_num, name, proc.returncode))fail_num = 1
fail_num = 1 else: print("ok %d - binfmt_script %s (correctly failed bad exec)" % (test_num, name))
pass_num += 1
# Clean up crazy binaries os.unlink(script) @@ -166,6 +172,8 @@ test(name="two-under-trunc-arg", size=int(SIZE/2), arg=" ") test(name="two-under-leading", size=int(SIZE/2), leading=" ") test(name="two-under-lead-trunc-arg", size=int(SIZE/2), leading=" ", arg=" ") +print("# Totals: pass:%d fail:%d xfail:0 xpass:0 skip:0 error:0" % (pass_num, fail_num))
if test_num != tests: raise ValueError("fewer binfmt_script tests than expected! (ran %d, expected %d" % (test_num, tests))
linux-kselftest-mirror@lists.linaro.org