[v6,2/2] selftests: add tests for pidfd_send_signal()

Commit Message

Christian Brauner Dec. 29, 2018, 10:27 p.m. UTC

As suggested by Andrew Morton in [1] add selftests for the new
sys_pidfd_send_signal() syscall.
This tests whether we can send a signal to an existing process and whether
sending a signal to a process that has already exited fails with ESRCH.

[1]: https://lore.kernel.org/lkml/[e-mail]/

Cc: Arnd Bergmann <[e-mail]>
Cc: "Eric W. Biederman" <[e-mail]>
Cc: Kees Cook <[e-mail]>
Cc: Serge Hallyn <[e-mail]>
Cc: Jann Horn <[e-mail]>
Cc: Andy Lutomirsky <[e-mail]>
Cc: Andrew Morton <[e-mail]>
Cc: Oleg Nesterov <[e-mail]>
Cc: Aleksa Sarai <[e-mail]>
Cc: Al Viro <[e-mail]>
Cc: Florian Weimer <[e-mail]>
Signed-off-by: Christian Brauner <[e-mail]>
---
/* Changelog */
v6:
- patch introduced
v5..v0:
- patch not present
---
 tools/testing/selftests/Makefile           |   1 +
 tools/testing/selftests/pidfd/Makefile     |   6 +
 tools/testing/selftests/pidfd/pidfd_test.c | 130 +++++++++++++++++++++
 3 files changed, 137 insertions(+)
 create mode 100644 tools/testing/selftests/pidfd/Makefile
 create mode 100644 tools/testing/selftests/pidfd/pidfd_test.c

Comments

Serge E. Hallyn Dec. 30, 2018, 9:02 p.m. UTC | #1

On Sat, Dec 29, 2018 at 11:27:56PM +0100, Christian Brauner wrote:
> As suggested by Andrew Morton in [1] add selftests for the new
> sys_pidfd_send_signal() syscall.
> This tests whether we can send a signal to an existing process and whether
> sending a signal to a process that has already exited fails with ESRCH.
> 
> [1]: https://lore.kernel.org/lkml/[e-mail]/
> 
> Cc: Arnd Bergmann <[e-mail]>
> Cc: "Eric W. Biederman" <[e-mail]>
> Cc: Kees Cook <[e-mail]>
> Cc: Serge Hallyn <[e-mail]>

Acked-by: Serge Hallyn <[e-mail]>

Not saying you need to do this, but it would be neat if you could test
sending to a pid which has been recycled :)

> Cc: Jann Horn <[e-mail]>
> Cc: Andy Lutomirsky <[e-mail]>
> Cc: Andrew Morton <[e-mail]>
> Cc: Oleg Nesterov <[e-mail]>
> Cc: Aleksa Sarai <[e-mail]>
> Cc: Al Viro <[e-mail]>
> Cc: Florian Weimer <[e-mail]>
> Signed-off-by: Christian Brauner <[e-mail]>
> ---
> /* Changelog */
> v6:
> - patch introduced
> v5..v0:
> - patch not present
> ---
>  tools/testing/selftests/Makefile           |   1 +
>  tools/testing/selftests/pidfd/Makefile     |   6 +
>  tools/testing/selftests/pidfd/pidfd_test.c | 130 +++++++++++++++++++++
>  3 files changed, 137 insertions(+)
>  create mode 100644 tools/testing/selftests/pidfd/Makefile
>  create mode 100644 tools/testing/selftests/pidfd/pidfd_test.c
> 
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 24b9934fb269..63b0d8a0ebf7 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -27,6 +27,7 @@ TARGETS += net
>  TARGETS += netfilter
>  TARGETS += networking/timestamping
>  TARGETS += nsfs
> +TARGETS += pidfd
>  TARGETS += powerpc
>  TARGETS += proc
>  TARGETS += pstore
> diff --git a/tools/testing/selftests/pidfd/Makefile b/tools/testing/selftests/pidfd/Makefile
> new file mode 100644
> index 000000000000..deaf8073bc06
> --- /dev/null
> +++ b/tools/testing/selftests/pidfd/Makefile
> @@ -0,0 +1,6 @@
> +CFLAGS += -g -I../../../../usr/include/
> +
> +TEST_GEN_PROGS := pidfd_test
> +
> +include ../lib.mk
> +
> diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
> new file mode 100644
> index 000000000000..edcd59979b10
> --- /dev/null
> +++ b/tools/testing/selftests/pidfd/pidfd_test.c
> @@ -0,0 +1,130 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <linux/types.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <syscall.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +
> +#include "../kselftest.h"
> +
> +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 do_exit_success(int sig)
> +{
> +	signal_received = 1;
> +}
> +
> +/*
> + * Straightforward test to see whether pidfd_send_signal() works is to send
> + * a signal to ourselves.
> + */
> +static int test_pidfd_send_signal_simple_success(void)
> +{
> +	int pidfd, ret;
> +	const char *test_name = "pidfd_send_signal send SIGUSR1";
> +
> +	pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC);
> +	if (pidfd < 0)
> +		ksft_exit_fail_msg(
> +			"%s test: Failed to open process file descriptor\n",
> +			test_name);
> +
> +	signal(SIGUSR1, do_exit_success);
> +
> +	ret = sys_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0);
> +	close(pidfd);
> +	if (ret < 0)
> +		ksft_exit_fail_msg("%s test: Failed to send signal\n",
> +				   test_name);
> +
> +	if (signal_received != 1)
> +		ksft_exit_fail_msg("%s test: Failed to receive signal\n",
> +				   test_name);
> +
> +	signal_received = 0;
> +	ksft_test_result_pass("%s test: Sent signal\n", test_name);
> +	return 0;
> +}
> +
> +static void wait_for_pid(pid_t pid)
> +{
> +	int status, ret;
> +
> +again:
> +	ret = waitpid(pid, &status, 0);
> +	if (ret == -1) {
> +		if (errno == EINTR)
> +			goto again;
> +
> +		return;
> +	}
> +
> +	if (ret != pid)
> +		goto again;
> +}
> +
> +static int test_pidfd_send_signal_exited_fail(void)
> +{
> +	int pidfd, ret, saved_errno;
> +	char buf[256];
> +	pid_t pid;
> +	const char *test_name = "pidfd_send_signal signal exited process";
> +
> +	pid = fork();
> +	if (pid < 0)
> +		ksft_exit_fail_msg("%s test: Failed to create new process\n",
> +				   test_name);
> +
> +	if (pid == 0)
> +		_exit(EXIT_SUCCESS);
> +
> +	snprintf(buf, sizeof(buf), "/proc/%d", pid);
> +
> +	pidfd = open(buf, O_DIRECTORY | O_CLOEXEC);
> +
> +	wait_for_pid(pid);
> +
> +	if (pidfd < 0)
> +		ksft_exit_fail_msg(
> +			"%s test: Failed to open process file descriptor\n",
> +			test_name);
> +
> +	ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0);
> +	saved_errno = errno;
> +	close(pidfd);
> +	if (ret == 0)
> +		ksft_exit_fail_msg(
> +			"%s test: Managed to send signal to process even though it should have failed\n",
> +			test_name);
> +
> +	if (saved_errno != ESRCH)
> +		ksft_exit_fail_msg(
> +			"%s test: Expected to receive ESRCH as errno value but received %d instead\n",
> +			test_name, saved_errno);
> +
> +	ksft_test_result_pass("%s test: Failed to send signal as expected\n",
> +			      test_name);
> +	return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	ksft_print_header();
> +
> +	test_pidfd_send_signal_simple_success();
> +	test_pidfd_send_signal_exited_fail();
> +
> +	return ksft_exit_pass();
> +}
> -- 
> 2.19.1

Christian Brauner Dec. 30, 2018, 11:27 p.m. UTC | #2

On Sun, Dec 30, 2018 at 03:02:45PM -0600, Serge Hallyn wrote:
> On Sat, Dec 29, 2018 at 11:27:56PM +0100, Christian Brauner wrote:
> > As suggested by Andrew Morton in [1] add selftests for the new
> > sys_pidfd_send_signal() syscall.
> > This tests whether we can send a signal to an existing process and whether
> > sending a signal to a process that has already exited fails with ESRCH.
> > 
> > [1]: https://lore.kernel.org/lkml/[e-mail]/
> > 
> > Cc: Arnd Bergmann <[e-mail]>
> > Cc: "Eric W. Biederman" <[e-mail]>
> > Cc: Kees Cook <[e-mail]>
> > Cc: Serge Hallyn <[e-mail]>
> 
> Acked-by: Serge Hallyn <[e-mail]>
> 
> Not saying you need to do this, but it would be neat if you could test
> sending to a pid which has been recycled :)

Yeah, I thought about it but it's a little weird code. First of all, we
can't set /proc/sys/kernel/pid_max to a very low value since this is a
system wide setting. So we need to recycle a lot via fork(). Something
along the lines of:
- unshare pid namespace
- fork to create pid 1 in new pid namespace
- cycle with fork() until pid > 300 since pids lower than 300 are
  reserved by the kernel.
  (That means if we simply use the first fork() after we created pid 1 we
  would never be able to recycle the pid since we skip over it. :))
- get pidfd to the pid > 300 we just created
- wait on the pid > 300
- cycle via fork() until we have reached the same pid > 300 again
- send SIGSTOP to that recycled process
- test that we cannot send SIGCONT to this SIGSTOPed task via the pidfd we
  received before
- send SIGCONT to the SIGSTOPed recycled pid and exit

Christian

> 
> > Cc: Jann Horn <[e-mail]>
> > Cc: Andy Lutomirsky <[e-mail]>
> > Cc: Andrew Morton <[e-mail]>
> > Cc: Oleg Nesterov <[e-mail]>
> > Cc: Aleksa Sarai <[e-mail]>
> > Cc: Al Viro <[e-mail]>
> > Cc: Florian Weimer <[e-mail]>
> > Signed-off-by: Christian Brauner <[e-mail]>
> > ---
> > /* Changelog */
> > v6:
> > - patch introduced
> > v5..v0:
> > - patch not present
> > ---
> >  tools/testing/selftests/Makefile           |   1 +
> >  tools/testing/selftests/pidfd/Makefile     |   6 +
> >  tools/testing/selftests/pidfd/pidfd_test.c | 130 +++++++++++++++++++++
> >  3 files changed, 137 insertions(+)
> >  create mode 100644 tools/testing/selftests/pidfd/Makefile
> >  create mode 100644 tools/testing/selftests/pidfd/pidfd_test.c
> > 
> > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> > index 24b9934fb269..63b0d8a0ebf7 100644
> > --- a/tools/testing/selftests/Makefile
> > +++ b/tools/testing/selftests/Makefile
> > @@ -27,6 +27,7 @@ TARGETS += net
> >  TARGETS += netfilter
> >  TARGETS += networking/timestamping
> >  TARGETS += nsfs
> > +TARGETS += pidfd
> >  TARGETS += powerpc
> >  TARGETS += proc
> >  TARGETS += pstore
> > diff --git a/tools/testing/selftests/pidfd/Makefile b/tools/testing/selftests/pidfd/Makefile
> > new file mode 100644
> > index 000000000000..deaf8073bc06
> > --- /dev/null
> > +++ b/tools/testing/selftests/pidfd/Makefile
> > @@ -0,0 +1,6 @@
> > +CFLAGS += -g -I../../../../usr/include/
> > +
> > +TEST_GEN_PROGS := pidfd_test
> > +
> > +include ../lib.mk
> > +
> > diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
> > new file mode 100644
> > index 000000000000..edcd59979b10
> > --- /dev/null
> > +++ b/tools/testing/selftests/pidfd/pidfd_test.c
> > @@ -0,0 +1,130 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#define _GNU_SOURCE
> > +#include <errno.h>
> > +#include <fcntl.h>
> > +#include <linux/types.h>
> > +#include <signal.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <syscall.h>
> > +#include <sys/wait.h>
> > +#include <unistd.h>
> > +
> > +#include "../kselftest.h"
> > +
> > +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 do_exit_success(int sig)
> > +{
> > +	signal_received = 1;
> > +}
> > +
> > +/*
> > + * Straightforward test to see whether pidfd_send_signal() works is to send
> > + * a signal to ourselves.
> > + */
> > +static int test_pidfd_send_signal_simple_success(void)
> > +{
> > +	int pidfd, ret;
> > +	const char *test_name = "pidfd_send_signal send SIGUSR1";
> > +
> > +	pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC);
> > +	if (pidfd < 0)
> > +		ksft_exit_fail_msg(
> > +			"%s test: Failed to open process file descriptor\n",
> > +			test_name);
> > +
> > +	signal(SIGUSR1, do_exit_success);
> > +
> > +	ret = sys_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0);
> > +	close(pidfd);
> > +	if (ret < 0)
> > +		ksft_exit_fail_msg("%s test: Failed to send signal\n",
> > +				   test_name);
> > +
> > +	if (signal_received != 1)
> > +		ksft_exit_fail_msg("%s test: Failed to receive signal\n",
> > +				   test_name);
> > +
> > +	signal_received = 0;
> > +	ksft_test_result_pass("%s test: Sent signal\n", test_name);
> > +	return 0;
> > +}
> > +
> > +static void wait_for_pid(pid_t pid)
> > +{
> > +	int status, ret;
> > +
> > +again:
> > +	ret = waitpid(pid, &status, 0);
> > +	if (ret == -1) {
> > +		if (errno == EINTR)
> > +			goto again;
> > +
> > +		return;
> > +	}
> > +
> > +	if (ret != pid)
> > +		goto again;
> > +}
> > +
> > +static int test_pidfd_send_signal_exited_fail(void)
> > +{
> > +	int pidfd, ret, saved_errno;
> > +	char buf[256];
> > +	pid_t pid;
> > +	const char *test_name = "pidfd_send_signal signal exited process";
> > +
> > +	pid = fork();
> > +	if (pid < 0)
> > +		ksft_exit_fail_msg("%s test: Failed to create new process\n",
> > +				   test_name);
> > +
> > +	if (pid == 0)
> > +		_exit(EXIT_SUCCESS);
> > +
> > +	snprintf(buf, sizeof(buf), "/proc/%d", pid);
> > +
> > +	pidfd = open(buf, O_DIRECTORY | O_CLOEXEC);
> > +
> > +	wait_for_pid(pid);
> > +
> > +	if (pidfd < 0)
> > +		ksft_exit_fail_msg(
> > +			"%s test: Failed to open process file descriptor\n",
> > +			test_name);
> > +
> > +	ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0);
> > +	saved_errno = errno;
> > +	close(pidfd);
> > +	if (ret == 0)
> > +		ksft_exit_fail_msg(
> > +			"%s test: Managed to send signal to process even though it should have failed\n",
> > +			test_name);
> > +
> > +	if (saved_errno != ESRCH)
> > +		ksft_exit_fail_msg(
> > +			"%s test: Expected to receive ESRCH as errno value but received %d instead\n",
> > +			test_name, saved_errno);
> > +
> > +	ksft_test_result_pass("%s test: Failed to send signal as expected\n",
> > +			      test_name);
> > +	return 0;
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +	ksft_print_header();
> > +
> > +	test_pidfd_send_signal_simple_success();
> > +	test_pidfd_send_signal_exited_fail();
> > +
> > +	return ksft_exit_pass();
> > +}
> > -- 
> > 2.19.1

Christian Brauner Jan. 1, 2019, 3:07 p.m. UTC | #3

On Mon, Dec 31, 2018 at 12:27:13AM +0100, Christian Brauner wrote:
> On Sun, Dec 30, 2018 at 03:02:45PM -0600, Serge Hallyn wrote:
> > On Sat, Dec 29, 2018 at 11:27:56PM +0100, Christian Brauner wrote:
> > > As suggested by Andrew Morton in [1] add selftests for the new
> > > sys_pidfd_send_signal() syscall.
> > > This tests whether we can send a signal to an existing process and whether
> > > sending a signal to a process that has already exited fails with ESRCH.
> > > 
> > > [1]: https://lore.kernel.org/lkml/[e-mail]/
> > > 
> > > Cc: Arnd Bergmann <[e-mail]>
> > > Cc: "Eric W. Biederman" <[e-mail]>
> > > Cc: Kees Cook <[e-mail]>
> > > Cc: Serge Hallyn <[e-mail]>
> > 
> > Acked-by: Serge Hallyn <[e-mail]>
> > 
> > Not saying you need to do this, but it would be neat if you could test
> > sending to a pid which has been recycled :)
> 
> Yeah, I thought about it but it's a little weird code. First of all, we
> can't set /proc/sys/kernel/pid_max to a very low value since this is a
> system wide setting. So we need to recycle a lot via fork(). Something
> along the lines of:
> - unshare pid namespace
> - fork to create pid 1 in new pid namespace
> - cycle with fork() until pid > 300 since pids lower than 300 are
>   reserved by the kernel.
>   (That means if we simply use the first fork() after we created pid 1 we
>   would never be able to recycle the pid since we skip over it. :))
> - get pidfd to the pid > 300 we just created
> - wait on the pid > 300
> - cycle via fork() until we have reached the same pid > 300 again
> - send SIGSTOP to that recycled process
> - test that we cannot send SIGCONT to this SIGSTOPed task via the pidfd we
>   received before
> - send SIGCONT to the SIGSTOPed recycled pid and exit

Ok, I have something like this in my tree now that tests for pid
recycling. I'm going to send it out tomorrow since I reckon Andrew and
others will be off today.
But fwiw it sits in https://github.com/brauner/linux/commits/2018-12-02/procfds

> 
> Christian
> 
> > 
> > > Cc: Jann Horn <[e-mail]>
> > > Cc: Andy Lutomirsky <[e-mail]>
> > > Cc: Andrew Morton <[e-mail]>
> > > Cc: Oleg Nesterov <[e-mail]>
> > > Cc: Aleksa Sarai <[e-mail]>
> > > Cc: Al Viro <[e-mail]>
> > > Cc: Florian Weimer <[e-mail]>
> > > Signed-off-by: Christian Brauner <[e-mail]>
> > > ---
> > > /* Changelog */
> > > v6:
> > > - patch introduced
> > > v5..v0:
> > > - patch not present
> > > ---
> > >  tools/testing/selftests/Makefile           |   1 +
> > >  tools/testing/selftests/pidfd/Makefile     |   6 +
> > >  tools/testing/selftests/pidfd/pidfd_test.c | 130 +++++++++++++++++++++
> > >  3 files changed, 137 insertions(+)
> > >  create mode 100644 tools/testing/selftests/pidfd/Makefile
> > >  create mode 100644 tools/testing/selftests/pidfd/pidfd_test.c
> > > 
> > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> > > index 24b9934fb269..63b0d8a0ebf7 100644
> > > --- a/tools/testing/selftests/Makefile
> > > +++ b/tools/testing/selftests/Makefile
> > > @@ -27,6 +27,7 @@ TARGETS += net
> > >  TARGETS += netfilter
> > >  TARGETS += networking/timestamping
> > >  TARGETS += nsfs
> > > +TARGETS += pidfd
> > >  TARGETS += powerpc
> > >  TARGETS += proc
> > >  TARGETS += pstore
> > > diff --git a/tools/testing/selftests/pidfd/Makefile b/tools/testing/selftests/pidfd/Makefile
> > > new file mode 100644
> > > index 000000000000..deaf8073bc06
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/pidfd/Makefile
> > > @@ -0,0 +1,6 @@
> > > +CFLAGS += -g -I../../../../usr/include/
> > > +
> > > +TEST_GEN_PROGS := pidfd_test
> > > +
> > > +include ../lib.mk
> > > +
> > > diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
> > > new file mode 100644
> > > index 000000000000..edcd59979b10
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/pidfd/pidfd_test.c
> > > @@ -0,0 +1,130 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +#define _GNU_SOURCE
> > > +#include <errno.h>
> > > +#include <fcntl.h>
> > > +#include <linux/types.h>
> > > +#include <signal.h>
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#include <syscall.h>
> > > +#include <sys/wait.h>
> > > +#include <unistd.h>
> > > +
> > > +#include "../kselftest.h"
> > > +
> > > +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 do_exit_success(int sig)
> > > +{
> > > +	signal_received = 1;
> > > +}
> > > +
> > > +/*
> > > + * Straightforward test to see whether pidfd_send_signal() works is to send
> > > + * a signal to ourselves.
> > > + */
> > > +static int test_pidfd_send_signal_simple_success(void)
> > > +{
> > > +	int pidfd, ret;
> > > +	const char *test_name = "pidfd_send_signal send SIGUSR1";
> > > +
> > > +	pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC);
> > > +	if (pidfd < 0)
> > > +		ksft_exit_fail_msg(
> > > +			"%s test: Failed to open process file descriptor\n",
> > > +			test_name);
> > > +
> > > +	signal(SIGUSR1, do_exit_success);
> > > +
> > > +	ret = sys_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0);
> > > +	close(pidfd);
> > > +	if (ret < 0)
> > > +		ksft_exit_fail_msg("%s test: Failed to send signal\n",
> > > +				   test_name);
> > > +
> > > +	if (signal_received != 1)
> > > +		ksft_exit_fail_msg("%s test: Failed to receive signal\n",
> > > +				   test_name);
> > > +
> > > +	signal_received = 0;
> > > +	ksft_test_result_pass("%s test: Sent signal\n", test_name);
> > > +	return 0;
> > > +}
> > > +
> > > +static void wait_for_pid(pid_t pid)
> > > +{
> > > +	int status, ret;
> > > +
> > > +again:
> > > +	ret = waitpid(pid, &status, 0);
> > > +	if (ret == -1) {
> > > +		if (errno == EINTR)
> > > +			goto again;
> > > +
> > > +		return;
> > > +	}
> > > +
> > > +	if (ret != pid)
> > > +		goto again;
> > > +}
> > > +
> > > +static int test_pidfd_send_signal_exited_fail(void)
> > > +{
> > > +	int pidfd, ret, saved_errno;
> > > +	char buf[256];
> > > +	pid_t pid;
> > > +	const char *test_name = "pidfd_send_signal signal exited process";
> > > +
> > > +	pid = fork();
> > > +	if (pid < 0)
> > > +		ksft_exit_fail_msg("%s test: Failed to create new process\n",
> > > +				   test_name);
> > > +
> > > +	if (pid == 0)
> > > +		_exit(EXIT_SUCCESS);
> > > +
> > > +	snprintf(buf, sizeof(buf), "/proc/%d", pid);
> > > +
> > > +	pidfd = open(buf, O_DIRECTORY | O_CLOEXEC);
> > > +
> > > +	wait_for_pid(pid);
> > > +
> > > +	if (pidfd < 0)
> > > +		ksft_exit_fail_msg(
> > > +			"%s test: Failed to open process file descriptor\n",
> > > +			test_name);
> > > +
> > > +	ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0);
> > > +	saved_errno = errno;
> > > +	close(pidfd);
> > > +	if (ret == 0)
> > > +		ksft_exit_fail_msg(
> > > +			"%s test: Managed to send signal to process even though it should have failed\n",
> > > +			test_name);
> > > +
> > > +	if (saved_errno != ESRCH)
> > > +		ksft_exit_fail_msg(
> > > +			"%s test: Expected to receive ESRCH as errno value but received %d instead\n",
> > > +			test_name, saved_errno);
> > > +
> > > +	ksft_test_result_pass("%s test: Failed to send signal as expected\n",
> > > +			      test_name);
> > > +	return 0;
> > > +}
> > > +
> > > +int main(int argc, char **argv)
> > > +{
> > > +	ksft_print_header();
> > > +
> > > +	test_pidfd_send_signal_simple_success();
> > > +	test_pidfd_send_signal_exited_fail();
> > > +
> > > +	return ksft_exit_pass();
> > > +}
> > > -- 
> > > 2.19.1

Serge E. Hallyn Jan. 1, 2019, 5:02 p.m. UTC | #4

On Tue, Jan 01, 2019 at 04:07:44PM +0100, Christian Brauner wrote:
> On Mon, Dec 31, 2018 at 12:27:13AM +0100, Christian Brauner wrote:
> > On Sun, Dec 30, 2018 at 03:02:45PM -0600, Serge Hallyn wrote:
> > > On Sat, Dec 29, 2018 at 11:27:56PM +0100, Christian Brauner wrote:
> > > > As suggested by Andrew Morton in [1] add selftests for the new
> > > > sys_pidfd_send_signal() syscall.
> > > > This tests whether we can send a signal to an existing process and whether
> > > > sending a signal to a process that has already exited fails with ESRCH.
> > > > 
> > > > [1]: https://lore.kernel.org/lkml/[e-mail]/
> > > > 
> > > > Cc: Arnd Bergmann <[e-mail]>
> > > > Cc: "Eric W. Biederman" <[e-mail]>
> > > > Cc: Kees Cook <[e-mail]>
> > > > Cc: Serge Hallyn <[e-mail]>
> > > 
> > > Acked-by: Serge Hallyn <[e-mail]>
> > > 
> > > Not saying you need to do this, but it would be neat if you could test
> > > sending to a pid which has been recycled :)
> > 
> > Yeah, I thought about it but it's a little weird code. First of all, we
> > can't set /proc/sys/kernel/pid_max to a very low value since this is a
> > system wide setting. So we need to recycle a lot via fork(). Something
> > along the lines of:
> > - unshare pid namespace
> > - fork to create pid 1 in new pid namespace
> > - cycle with fork() until pid > 300 since pids lower than 300 are
> >   reserved by the kernel.
> >   (That means if we simply use the first fork() after we created pid 1 we
> >   would never be able to recycle the pid since we skip over it. :))
> > - get pidfd to the pid > 300 we just created
> > - wait on the pid > 300
> > - cycle via fork() until we have reached the same pid > 300 again
> > - send SIGSTOP to that recycled process
> > - test that we cannot send SIGCONT to this SIGSTOPed task via the pidfd we
> >   received before
> > - send SIGCONT to the SIGSTOPed recycled pid and exit
> 
> Ok, I have something like this in my tree now that tests for pid
> recycling. I'm going to send it out tomorrow since I reckon Andrew and
> others will be off today.
> But fwiw it sits in https://github.com/brauner/linux/commits/2018-12-02/procfds

Thanks, that shows off the advantages of the new syscall :)

-serge

Patch

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 24b9934fb269..63b0d8a0ebf7 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -27,6 +27,7 @@  TARGETS += net
 TARGETS += netfilter
 TARGETS += networking/timestamping
 TARGETS += nsfs
+TARGETS += pidfd
 TARGETS += powerpc
 TARGETS += proc
 TARGETS += pstore
diff --git a/tools/testing/selftests/pidfd/Makefile b/tools/testing/selftests/pidfd/Makefile
new file mode 100644
index 000000000000..deaf8073bc06
--- /dev/null
+++ b/tools/testing/selftests/pidfd/Makefile
@@ -0,0 +1,6 @@ 
+CFLAGS += -g -I../../../../usr/include/
+
+TEST_GEN_PROGS := pidfd_test
+
+include ../lib.mk
+
diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
new file mode 100644
index 000000000000..edcd59979b10
--- /dev/null
+++ b/tools/testing/selftests/pidfd/pidfd_test.c
@@ -0,0 +1,130 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/types.h>
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syscall.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "../kselftest.h"
+
+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 do_exit_success(int sig)
+{
+	signal_received = 1;
+}
+
+/*
+ * Straightforward test to see whether pidfd_send_signal() works is to send
+ * a signal to ourselves.
+ */
+static int test_pidfd_send_signal_simple_success(void)
+{
+	int pidfd, ret;
+	const char *test_name = "pidfd_send_signal send SIGUSR1";
+
+	pidfd = open("/proc/self", O_DIRECTORY | O_CLOEXEC);
+	if (pidfd < 0)
+		ksft_exit_fail_msg(
+			"%s test: Failed to open process file descriptor\n",
+			test_name);
+
+	signal(SIGUSR1, do_exit_success);
+
+	ret = sys_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0);
+	close(pidfd);
+	if (ret < 0)
+		ksft_exit_fail_msg("%s test: Failed to send signal\n",
+				   test_name);
+
+	if (signal_received != 1)
+		ksft_exit_fail_msg("%s test: Failed to receive signal\n",
+				   test_name);
+
+	signal_received = 0;
+	ksft_test_result_pass("%s test: Sent signal\n", test_name);
+	return 0;
+}
+
+static void wait_for_pid(pid_t pid)
+{
+	int status, ret;
+
+again:
+	ret = waitpid(pid, &status, 0);
+	if (ret == -1) {
+		if (errno == EINTR)
+			goto again;
+
+		return;
+	}
+
+	if (ret != pid)
+		goto again;
+}
+
+static int test_pidfd_send_signal_exited_fail(void)
+{
+	int pidfd, ret, saved_errno;
+	char buf[256];
+	pid_t pid;
+	const char *test_name = "pidfd_send_signal signal exited process";
+
+	pid = fork();
+	if (pid < 0)
+		ksft_exit_fail_msg("%s test: Failed to create new process\n",
+				   test_name);
+
+	if (pid == 0)
+		_exit(EXIT_SUCCESS);
+
+	snprintf(buf, sizeof(buf), "/proc/%d", pid);
+
+	pidfd = open(buf, O_DIRECTORY | O_CLOEXEC);
+
+	wait_for_pid(pid);
+
+	if (pidfd < 0)
+		ksft_exit_fail_msg(
+			"%s test: Failed to open process file descriptor\n",
+			test_name);
+
+	ret = sys_pidfd_send_signal(pidfd, 0, NULL, 0);
+	saved_errno = errno;
+	close(pidfd);
+	if (ret == 0)
+		ksft_exit_fail_msg(
+			"%s test: Managed to send signal to process even though it should have failed\n",
+			test_name);
+
+	if (saved_errno != ESRCH)
+		ksft_exit_fail_msg(
+			"%s test: Expected to receive ESRCH as errno value but received %d instead\n",
+			test_name, saved_errno);
+
+	ksft_test_result_pass("%s test: Failed to send signal as expected\n",
+			      test_name);
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	ksft_print_header();
+
+	test_pidfd_send_signal_simple_success();
+	test_pidfd_send_signal_exited_fail();
+
+	return ksft_exit_pass();
+}