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 <[email protected]>
---
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))
--
2.39.2
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 <[email protected]>
---
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();
}
--
2.39.2
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 <[email protected]>
---
.../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();
}
--
2.39.2
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 <[email protected]>
Thanks, looks good.
Reviewed-by: Kees Cook <[email protected]>
--
Kees Cook
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 <[email protected]>
> ---
> 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.
--
Kees Cook
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 <[email protected]>
>> ---
>> 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.
--
BR,
Muhammad Usama Anjum
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 <[email protected]>
> ---
> 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))
--
BR,
Muhammad Usama Anjum
On Mon, Mar 11, 2024 at 10:10:21PM +0500, Muhammad Usama Anjum wrote:
> Soft reminder!
Ah yes! Thanks for the reminder on these. I will get them into -next
shortly.
--
Kees Cook