2019-07-26 09:42:55

by Christian Brauner

[permalink] [raw]
Subject: [PATCH v1 0/2] pidfd: waiting on processes through pidfds

Hey everyone,

/* v1 */
This adds the ability to wait on processes using pidfds. This is one of
the few missing pieces to make it possible to manage processes using
only pidfds.

Please note the following major changes (More details can be found in
the individual commit changelogs.):
- Add the new type P_PIDFD to waitid() instead of a new dedicated
pidfd_wait() syscall.
This is the same approach we discussed a few months ago and still
prefer over adding a dedicated syscall.
- Adapt the tests to the new type P_PIDFD for waitid().
- Remove struct waitid_info patch.
This will be sent out as a separate patch.
- Remove CLONE_WAIT_PID patch.
This will be sent out as a separate patch.

The core patch for waitid is pleasantly small. The largest change is
caused by adding proper tests for waitid(P_PIDFD).

/* v0 */
Link: https://lore.kernel.org/lkml/[email protected]

Thanks!
Christian

Christian Brauner (2):
pidfd: add P_PIDFD to waitid()
pidfd: add pidfd_wait tests

include/linux/pid.h | 4 +
include/uapi/linux/wait.h | 1 +
kernel/exit.c | 25 ++-
kernel/fork.c | 8 +
kernel/signal.c | 7 +-
tools/testing/selftests/pidfd/pidfd.h | 25 +++
tools/testing/selftests/pidfd/pidfd_test.c | 14 --
tools/testing/selftests/pidfd/pidfd_wait.c | 245 +++++++++++++++++++++
8 files changed, 311 insertions(+), 18 deletions(-)
create mode 100644 tools/testing/selftests/pidfd/pidfd_wait.c

--
2.22.0



2019-07-26 09:43:15

by Christian Brauner

[permalink] [raw]
Subject: [PATCH v1 2/2] pidfd: add pidfd_wait tests

Add tests for pidfd_wait() and CLONE_WAIT_PID:
- test that waitid(P_PIDFD) can wait on a pidfd
- test that waitid(P_PIDFD) can wait on a pidfd and return siginfo_t
- test that waitid(P_PIDFD) works with WEXITED
- test that waitid(P_PIDFD) works with WSTOPPED
- test that waitid(P_PIDFD) works with WUNTRACED
- test that waitid(P_PIDFD) works with WCONTINUED
- test that waitid(P_PIDFD) works with WNOWAIT
- test that waitid(P_PIDFD)works with WNOHANG

Signed-off-by: Christian Brauner <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: "Eric W. Biederman" <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Joel Fernandes (Google) <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: David Howells <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Andy Lutomirsky <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Aleksa Sarai <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Al Viro <[email protected]>
---
v1:
- Christian Brauner <[email protected]>:
- adapt tests to new P_PIDFD concept
---
tools/testing/selftests/pidfd/pidfd.h | 25 +++
tools/testing/selftests/pidfd/pidfd_test.c | 14 --
tools/testing/selftests/pidfd/pidfd_wait.c | 245 +++++++++++++++++++++
3 files changed, 270 insertions(+), 14 deletions(-)
create mode 100644 tools/testing/selftests/pidfd/pidfd_wait.c

diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h
index 8452e910463f..7d7d0ca05e0b 100644
--- a/tools/testing/selftests/pidfd/pidfd.h
+++ b/tools/testing/selftests/pidfd/pidfd.h
@@ -16,6 +16,26 @@

#include "../kselftest.h"

+#ifndef P_PIDFD
+#define P_PIDFD 3
+#endif
+
+#ifndef CLONE_PIDFD
+#define CLONE_PIDFD 0x00001000
+#endif
+
+#ifndef __NR_pidfd_open
+#define __NR_pidfd_open -1
+#endif
+
+#ifndef __NR_pidfd_send_signal
+#define __NR_pidfd_send_signal -1
+#endif
+
+#ifndef __NR_clone3
+#define __NR_clone3 -1
+#endif
+
/*
* The kernel reserves 300 pids via RESERVED_PIDS in kernel/pid.c
* That means, when it wraps around any pid < 300 will be skipped.
@@ -53,5 +73,10 @@ int wait_for_pid(pid_t pid)
return WEXITSTATUS(status);
}

+static inline int sys_pidfd_send_signal(int pidfd, int sig, siginfo_t *info,
+ unsigned int flags)
+{
+ return syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags);
+}

#endif /* __PIDFD_H */
diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
index 7eaa8a3de262..42e3eb494d72 100644
--- a/tools/testing/selftests/pidfd/pidfd_test.c
+++ b/tools/testing/selftests/pidfd/pidfd_test.c
@@ -21,20 +21,12 @@
#include "pidfd.h"
#include "../kselftest.h"

-#ifndef __NR_pidfd_send_signal
-#define __NR_pidfd_send_signal -1
-#endif
-
#define str(s) _str(s)
#define _str(s) #s
#define CHILD_THREAD_MIN_WAIT 3 /* seconds */

#define MAX_EVENTS 5

-#ifndef CLONE_PIDFD
-#define CLONE_PIDFD 0x00001000
-#endif
-
static pid_t pidfd_clone(int flags, int *pidfd, int (*fn)(void *))
{
size_t stack_size = 1024;
@@ -47,12 +39,6 @@ static pid_t pidfd_clone(int flags, int *pidfd, int (*fn)(void *))
#endif
}

-static inline int sys_pidfd_send_signal(int pidfd, int sig, siginfo_t *info,
- unsigned int flags)
-{
- return syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags);
-}
-
static int signal_received;

static void set_signal_received_on_sigusr1(int sig)
diff --git a/tools/testing/selftests/pidfd/pidfd_wait.c b/tools/testing/selftests/pidfd/pidfd_wait.c
new file mode 100644
index 000000000000..018d806032c0
--- /dev/null
+++ b/tools/testing/selftests/pidfd/pidfd_wait.c
@@ -0,0 +1,245 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <linux/sched.h>
+#include <linux/types.h>
+#include <signal.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sched.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "pidfd.h"
+#include "../kselftest.h"
+
+#define ptr_to_u64(ptr) ((__u64)((uintptr_t)(ptr)))
+
+static pid_t sys_clone3(struct clone_args *args)
+{
+ return syscall(__NR_clone3, args, sizeof(struct clone_args));
+}
+
+static int sys_waitid(int which, pid_t pid, siginfo_t *info, int options,
+ struct rusage *ru)
+{
+ return syscall(__NR_waitid, which, pid, info, options, ru);
+}
+
+static int test_pidfd_wait_simple(void)
+{
+ const char *test_name = "pidfd wait siginfo";
+ int pidfd = -1, status = 0;
+ pid_t parent_tid = -1;
+ struct clone_args args = {
+ .parent_tid = ptr_to_u64(&parent_tid),
+ .pidfd = ptr_to_u64(&pidfd),
+ .flags = CLONE_PIDFD | CLONE_PARENT_SETTID,
+ .exit_signal = SIGCHLD,
+ };
+ int ret;
+ pid_t pid;
+ siginfo_t info = {
+ .si_signo = 0,
+ };
+
+ pid = sys_clone3(&args);
+ if (pid < 0)
+ ksft_exit_fail_msg("%s test: failed to create new process %s\n",
+ test_name, strerror(errno));
+
+ if (pid == 0)
+ exit(EXIT_SUCCESS);
+
+ pid = sys_waitid(P_PIDFD, pidfd, &info, WEXITED, NULL);
+ if (pid < 0)
+ ksft_exit_fail_msg(
+ "%s test: failed to wait on process with pid %d and pidfd %d: %s\n",
+ test_name, parent_tid, pidfd, strerror(errno));
+
+ if (!WIFEXITED(info.si_status) || WEXITSTATUS(info.si_status))
+ ksft_exit_fail_msg(
+ "%s test: unexpected status received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, parent_tid, pidfd, strerror(errno));
+ close(pidfd);
+
+ if (info.si_signo != SIGCHLD)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_signo value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_signo, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_code != CLD_EXITED)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_code value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_code, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_pid != parent_tid)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_pid value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_pid, parent_tid, pidfd,
+ strerror(errno));
+
+ ksft_test_result_pass("%s test: Passed\n", test_name);
+ return 0;
+}
+
+static int test_pidfd_wait_states(void)
+{
+ const char *test_name = "pidfd wait states";
+ int pidfd = -1, status = 0;
+ pid_t parent_tid = -1;
+ struct clone_args args = {
+ .parent_tid = ptr_to_u64(&parent_tid),
+ .pidfd = ptr_to_u64(&pidfd),
+ .flags = CLONE_PIDFD | CLONE_PARENT_SETTID,
+ .exit_signal = SIGCHLD,
+ };
+ int ret;
+ pid_t pid;
+ siginfo_t info = {
+ .si_signo = 0,
+ };
+
+ pid = sys_clone3(&args);
+ if (pid < 0)
+ ksft_exit_fail_msg("%s test: failed to create new process %s\n",
+ test_name, strerror(errno));
+
+ if (pid == 0) {
+ kill(getpid(), SIGSTOP);
+ kill(getpid(), SIGSTOP);
+ exit(EXIT_SUCCESS);
+ }
+
+ ret = sys_waitid(P_PIDFD, pidfd, &info, WSTOPPED, NULL);
+ if (ret < 0)
+ ksft_exit_fail_msg(
+ "%s test: failed to wait on process with pid %d and pidfd %d: %s\n",
+ test_name, parent_tid, pidfd, strerror(errno));
+
+ if (info.si_signo != SIGCHLD)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_signo value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_signo, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_code != CLD_STOPPED)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_code value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_code, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_pid != parent_tid)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_pid value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_pid, parent_tid, pidfd,
+ strerror(errno));
+
+ ret = sys_pidfd_send_signal(pidfd, SIGCONT, NULL, 0);
+ if (ret < 0)
+ ksft_exit_fail_msg(
+ "%s test: failed to wait on process with pid %d and pidfd %d: %s\n",
+ test_name, parent_tid, pidfd, strerror(errno));
+
+ ret = sys_waitid(P_PIDFD, pidfd, &info, WCONTINUED, NULL);
+ if (ret < 0)
+ ksft_exit_fail_msg(
+ "%s test: failed to wait on process with pid %d and pidfd %d: %s\n",
+ test_name, parent_tid, pidfd, strerror(errno));
+
+ if (info.si_signo != SIGCHLD)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_signo value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_signo, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_code != CLD_CONTINUED)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_code value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_code, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_pid != parent_tid)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_pid value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_pid, parent_tid, pidfd,
+ strerror(errno));
+
+ ret = sys_waitid(P_PIDFD, pidfd, &info, WUNTRACED, NULL);
+ if (ret < 0)
+ ksft_exit_fail_msg(
+ "%s test: failed to wait on process with pid %d and pidfd %d: %s\n",
+ test_name, parent_tid, pidfd, strerror(errno));
+
+ if (info.si_signo != SIGCHLD)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_signo value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_signo, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_code != CLD_STOPPED)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_code value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_code, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_pid != parent_tid)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_pid value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_pid, parent_tid, pidfd,
+ strerror(errno));
+
+ ret = sys_pidfd_send_signal(pidfd, SIGKILL, NULL, 0);
+ if (ret < 0)
+ ksft_exit_fail_msg(
+ "%s test: failed to wait on process with pid %d and pidfd %d: %s\n",
+ test_name, parent_tid, pidfd, strerror(errno));
+
+ ret = sys_waitid(P_PIDFD, pidfd, &info, WEXITED, NULL);
+ if (ret < 0)
+ ksft_exit_fail_msg(
+ "%s test: failed to wait on process with pid %d and pidfd %d: %s\n",
+ test_name, parent_tid, pidfd, strerror(errno));
+
+ if (info.si_signo != SIGCHLD)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_signo value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_signo, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_code != CLD_KILLED)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_code value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_code, parent_tid, pidfd,
+ strerror(errno));
+
+ if (info.si_pid != parent_tid)
+ ksft_exit_fail_msg(
+ "%s test: unexpected si_pid value %d received after waiting on process with pid %d and pidfd %d: %s\n",
+ test_name, info.si_pid, parent_tid, pidfd,
+ strerror(errno));
+
+ close(pidfd);
+
+ ksft_test_result_pass("%s test: Passed\n", test_name);
+ return 0;
+}
+
+int main(int argc, char **argv)
+{
+ ksft_print_header();
+ ksft_set_plan(2);
+
+ test_pidfd_wait_simple();
+ test_pidfd_wait_states();
+
+ return ksft_exit_pass();
+}
--
2.22.0


2019-07-26 10:04:35

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [OFFLIST] [PATCH v1 0/2] pidfd: waiting on processes through pidfds

Hi Christian,

just fyi, can't read this patch till Monday


On 07/26, Christian Brauner wrote:
> Hey everyone,
>
> /* v1 */
> This adds the ability to wait on processes using pidfds. This is one of
> the few missing pieces to make it possible to manage processes using
> only pidfds.
>
> Please note the following major changes (More details can be found in
> the individual commit changelogs.):
> - Add the new type P_PIDFD to waitid() instead of a new dedicated
> pidfd_wait() syscall.
> This is the same approach we discussed a few months ago and still
> prefer over adding a dedicated syscall.
> - Adapt the tests to the new type P_PIDFD for waitid().
> - Remove struct waitid_info patch.
> This will be sent out as a separate patch.
> - Remove CLONE_WAIT_PID patch.
> This will be sent out as a separate patch.
>
> The core patch for waitid is pleasantly small. The largest change is
> caused by adding proper tests for waitid(P_PIDFD).
>
> /* v0 */
> Link: https://lore.kernel.org/lkml/[email protected]
>
> Thanks!
> Christian
>
> Christian Brauner (2):
> pidfd: add P_PIDFD to waitid()
> pidfd: add pidfd_wait tests
>
> include/linux/pid.h | 4 +
> include/uapi/linux/wait.h | 1 +
> kernel/exit.c | 25 ++-
> kernel/fork.c | 8 +
> kernel/signal.c | 7 +-
> tools/testing/selftests/pidfd/pidfd.h | 25 +++
> tools/testing/selftests/pidfd/pidfd_test.c | 14 --
> tools/testing/selftests/pidfd/pidfd_wait.c | 245 +++++++++++++++++++++
> 8 files changed, 311 insertions(+), 18 deletions(-)
> create mode 100644 tools/testing/selftests/pidfd/pidfd_wait.c
>
> --
> 2.22.0
>