pidfd: fix test failure due to stack overflow on some arches

When running the pidfd_fdinfo_test on arm64, it fails for me. After some
digging, the reason is that the child exits due to SIGBUS, because it
overflows the 1024 byte stack we've reserved for it.

To fix the issue, increase the stack size to 8192 bytes (this number is
somewhat arbitrary, and was arrived at through experimentation -- I kept
doubling until the failure no longer occurred).

Also, let's make the issue easier to debug. wait_for_pid() returns an
ambiguous value: it may return -1 in all of these cases:

1. waitpid() itself returned -1
2. waitpid() returned success, but we found !WIFEXITED(status).
3. The child process exited, but it did so with a -1 exit code.

There's no way for the caller to tell the difference. So, at least log
which occurred, so the test runner can debug things.

While debugging this, I found that we had !WIFEXITED(), because the
child exited due to a signal. This seems like a reasonably common case,
so also print out whether or not we have WIFSIGNALED(), and the
associated WTERMSIG() (if any). This lets us see the SIGBUS I'm fixing
clearly when it occurs.

Finally, I'm suspicious of allocating the child's stack on our stack.
man clone(2) suggests that the correct way to do this is with mmap(),
and in particular by setting MAP_STACK. So, switch to doing it that way
instead.

Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
Acked-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>

diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h
index 01f8d3c0cf2cb844ecc233e5cc1f6b7fa1a42c0f..6922d6417e1cf06094c10627e0bd491d292d6fd2 100644 (file)
--- a/tools/testing/selftests/pidfd/pidfd.h
+++ b/tools/testing/selftests/pidfd/pidfd.h
@@ -68,7 +68,7 @@
 #define PIDFD_SKIP 3
 #define PIDFD_XFAIL 4
 
-int wait_for_pid(pid_t pid)
+static inline int wait_for_pid(pid_t pid)
 {
        int status, ret;
 
@@ -78,13 +78,20 @@ again:
                if (errno == EINTR)
                        goto again;
 
+               ksft_print_msg("waitpid returned -1, errno=%d\n", errno);
                return -1;
        }
 
-       if (!WIFEXITED(status))
+       if (!WIFEXITED(status)) {
+               ksft_print_msg(
+                      "waitpid !WIFEXITED, WIFSIGNALED=%d, WTERMSIG=%d\n",
+                      WIFSIGNALED(status), WTERMSIG(status));
                return -1;
+       }
 
-       return WEXITSTATUS(status);
+       ret = WEXITSTATUS(status);
+       ksft_print_msg("waitpid WEXITSTATUS=%d\n", ret);
+       return ret;
 }
 
 static inline int sys_pidfd_open(pid_t pid, unsigned int flags)

diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
index 22558524f71c34da86e9e0dacc5a73c523254e72..3fd8e903118f532d43a2ac251ec71531f19ca161 100644 (file)
--- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
+++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
@@ -12,6 +12,7 @@
 #include <string.h>
 #include <syscall.h>
 #include <sys/wait.h>
+#include <sys/mman.h>
 
 #include "pidfd.h"
 #include "../kselftest.h"
@@ -80,7 +81,10 @@ static inline int error_check(struct error *err, const char *test_name)
        return err->code;
 }
 
+#define CHILD_STACK_SIZE 8192
+
 struct child {
+       char *stack;
        pid_t pid;
        int   fd;
 };
@@ -89,17 +93,22 @@ static struct child clone_newns(int (*fn)(void *), void *args,
                                struct error *err)
 {
        static int flags = CLONE_PIDFD | CLONE_NEWPID | CLONE_NEWNS | SIGCHLD;
-       size_t stack_size = 1024;
-       char *stack[1024] = { 0 };
        struct child ret;
 
        if (!(flags & CLONE_NEWUSER) && geteuid() != 0)
                flags |= CLONE_NEWUSER;
 
+       ret.stack = mmap(NULL, CHILD_STACK_SIZE, PROT_READ | PROT_WRITE,
+                        MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+       if (ret.stack == MAP_FAILED) {
+               error_set(err, -1, "mmap of stack failed (errno %d)", errno);
+               return ret;
+       }
+
 #ifdef __ia64__
-       ret.pid = __clone2(fn, stack, stack_size, flags, args, &ret.fd);
+       ret.pid = __clone2(fn, ret.stack, CHILD_STACK_SIZE, flags, args, &ret.fd);
 #else
-       ret.pid = clone(fn, stack + stack_size, flags, args, &ret.fd);
+       ret.pid = clone(fn, ret.stack + CHILD_STACK_SIZE, flags, args, &ret.fd);
 #endif
 
        if (ret.pid < 0) {
@@ -129,6 +138,11 @@ static inline int child_join(struct child *child, struct error *err)
        else if (r > 0)
                error_set(err, r, "child %d reported: %d", child->pid, r);
 
+       if (munmap(child->stack, CHILD_STACK_SIZE)) {
+               error_set(err, -1, "munmap of child stack failed (errno %d)", errno);
+               r = -1;
+       }
+
        return r;
 }