2023-04-15 21:47:24

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 0/2] tools/nolibc: fork: fix on s390 and add test

The generic fork() implementation in nolibc falls back to the clone()
syscall. On s390 the first two arguments to clone() are swapped compared
to other architectures, breaking the implementation in nolibc.

Add a custom implementation of fork() to s390 that works.

While at it also add a testcase for fork().

Signed-off-by: Thomas Weißschuh <[email protected]>
---
Thomas Weißschuh (2):
tools/nolibc: s390: provide custom implementation for sys_fork
tools/nolibc: add testcase for fork()/waitpid()

tools/include/nolibc/arch-s390.h | 8 ++++++++
tools/include/nolibc/sys.h | 2 ++
tools/testing/selftests/nolibc/nolibc-test.c | 20 ++++++++++++++++++++
3 files changed, 30 insertions(+)
---
base-commit: c1c4f33b6be9b3412d9e0ba01b367f4ffe47c379
change-id: 20230415-nolibc-fork-b7087a345166

Best regards,
--
Thomas Weißschuh <[email protected]>


2023-04-15 21:47:57

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH 2/2] tools/nolibc: add testcase for fork()/waitpid()

On s390 the arguments to clone() which is used by fork() are different
than other archs.
Make sure everything works correctly.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 888da60eb5ba..d294338cf141 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -474,6 +474,25 @@ static int test_getpagesize(void)
return !c;
}

+static int test_fork(void)
+{
+ int status;
+ pid_t pid = fork();
+
+ switch (pid) {
+ case -1:
+ return 1;
+
+ case 0:
+ exit(123);
+
+ default:
+ pid = waitpid(pid, &status, 0);
+
+ return pid == -1 || !WIFEXITED(status) || WEXITSTATUS(status) != 123;
+ }
+}
+
/* Run syscall tests between IDs <min> and <max>.
* Return 0 on success, non-zero on failure.
*/
@@ -530,6 +549,7 @@ int run_syscall(int min, int max)
CASE_TEST(dup3_0); tmp = dup3(0, 100, 0); EXPECT_SYSNE(1, tmp, -1); close(tmp); break;
CASE_TEST(dup3_m1); tmp = dup3(-1, 100, 0); EXPECT_SYSER(1, tmp, -1, EBADF); if (tmp != -1) close(tmp); break;
CASE_TEST(execve_root); EXPECT_SYSER(1, execve("/", (char*[]){ [0] = "/", [1] = NULL }, NULL), -1, EACCES); break;
+ CASE_TEST(fork); EXPECT_SYSZR(1, test_fork()); break;
CASE_TEST(getdents64_root); EXPECT_SYSNE(1, test_getdents64("/"), -1); break;
CASE_TEST(getdents64_null); EXPECT_SYSER(1, test_getdents64("/dev/null"), -1, ENOTDIR); break;
CASE_TEST(gettimeofday_null); EXPECT_SYSZR(1, gettimeofday(NULL, NULL)); break;

--
2.40.0

2023-04-16 05:44:57

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH 0/2] tools/nolibc: fork: fix on s390 and add test

Hi Thomas,

On Sat, Apr 15, 2023 at 11:28:46PM +0200, Thomas Wei?schuh wrote:
> The generic fork() implementation in nolibc falls back to the clone()
> syscall. On s390 the first two arguments to clone() are swapped compared
> to other architectures, breaking the implementation in nolibc.
>
> Add a custom implementation of fork() to s390 that works.
>
> While at it also add a testcase for fork().
>
> Signed-off-by: Thomas Wei?schuh <[email protected]>

Thanks for these. Please always Cc the authors of code you're fixing
(e.g. there could be more subtle details that only the author knows
about). Here I think it's OK, but I've CCed Sven and will add him to
your fix.

Thank you!
Willy