Skip to content

Commit 4cbd93c

Browse files
CmdrMoozyshuahkh
authored andcommitted
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>
1 parent ec04989 commit 4cbd93c

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

tools/testing/selftests/pidfd/pidfd.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
#define PIDFD_SKIP 3
6969
#define PIDFD_XFAIL 4
7070

71-
int wait_for_pid(pid_t pid)
71+
static inline int wait_for_pid(pid_t pid)
7272
{
7373
int status, ret;
7474

@@ -78,13 +78,20 @@ int wait_for_pid(pid_t pid)
7878
if (errno == EINTR)
7979
goto again;
8080

81+
ksft_print_msg("waitpid returned -1, errno=%d\n", errno);
8182
return -1;
8283
}
8384

84-
if (!WIFEXITED(status))
85+
if (!WIFEXITED(status)) {
86+
ksft_print_msg(
87+
"waitpid !WIFEXITED, WIFSIGNALED=%d, WTERMSIG=%d\n",
88+
WIFSIGNALED(status), WTERMSIG(status));
8589
return -1;
90+
}
8691

87-
return WEXITSTATUS(status);
92+
ret = WEXITSTATUS(status);
93+
ksft_print_msg("waitpid WEXITSTATUS=%d\n", ret);
94+
return ret;
8895
}
8996

9097
static inline int sys_pidfd_open(pid_t pid, unsigned int flags)

tools/testing/selftests/pidfd/pidfd_fdinfo_test.c

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <string.h>
1313
#include <syscall.h>
1414
#include <sys/wait.h>
15+
#include <sys/mman.h>
1516

1617
#include "pidfd.h"
1718
#include "../kselftest.h"
@@ -80,7 +81,10 @@ static inline int error_check(struct error *err, const char *test_name)
8081
return err->code;
8182
}
8283

84+
#define CHILD_STACK_SIZE 8192
85+
8386
struct child {
87+
char *stack;
8488
pid_t pid;
8589
int fd;
8690
};
@@ -89,17 +93,22 @@ static struct child clone_newns(int (*fn)(void *), void *args,
8993
struct error *err)
9094
{
9195
static int flags = CLONE_PIDFD | CLONE_NEWPID | CLONE_NEWNS | SIGCHLD;
92-
size_t stack_size = 1024;
93-
char *stack[1024] = { 0 };
9496
struct child ret;
9597

9698
if (!(flags & CLONE_NEWUSER) && geteuid() != 0)
9799
flags |= CLONE_NEWUSER;
98100

101+
ret.stack = mmap(NULL, CHILD_STACK_SIZE, PROT_READ | PROT_WRITE,
102+
MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
103+
if (ret.stack == MAP_FAILED) {
104+
error_set(err, -1, "mmap of stack failed (errno %d)", errno);
105+
return ret;
106+
}
107+
99108
#ifdef __ia64__
100-
ret.pid = __clone2(fn, stack, stack_size, flags, args, &ret.fd);
109+
ret.pid = __clone2(fn, ret.stack, CHILD_STACK_SIZE, flags, args, &ret.fd);
101110
#else
102-
ret.pid = clone(fn, stack + stack_size, flags, args, &ret.fd);
111+
ret.pid = clone(fn, ret.stack + CHILD_STACK_SIZE, flags, args, &ret.fd);
103112
#endif
104113

105114
if (ret.pid < 0) {
@@ -129,6 +138,11 @@ static inline int child_join(struct child *child, struct error *err)
129138
else if (r > 0)
130139
error_set(err, r, "child %d reported: %d", child->pid, r);
131140

141+
if (munmap(child->stack, CHILD_STACK_SIZE)) {
142+
error_set(err, -1, "munmap of child stack failed (errno %d)", errno);
143+
r = -1;
144+
}
145+
132146
return r;
133147
}
134148

0 commit comments

Comments
 (0)