* [LTP] [PATCH v1 2/4] syscalls/pidfd_send_signal01
2019-05-15 12:01 [LTP] [PATCH v1 1/4] Add Syscall numbers for pidfd_send_signal Christian Amann
@ 2019-05-15 12:01 ` Christian Amann
2019-05-28 11:30 ` Cyril Hrubis
2019-05-15 12:01 ` [LTP] [PATCH v1 3/4] syscalls/pidfd_send_signal02 Christian Amann
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Christian Amann @ 2019-05-15 12:01 UTC (permalink / raw)
To: ltp
Add testcase to check if pidfd_send_signal() can provide
the same functionality as rt_sigqueueinfo().
Signed-off-by: Christian Amann <camann@suse.com>
---
runtest/syscalls | 2 +
.../kernel/syscalls/pidfd_send_signal/.gitignore | 1 +
.../kernel/syscalls/pidfd_send_signal/Makefile | 14 +++
.../syscalls/pidfd_send_signal/pidfd_send_signal.h | 20 ++++
.../pidfd_send_signal/pidfd_send_signal01.c | 110 +++++++++++++++++++++
5 files changed, 147 insertions(+)
create mode 100644 testcases/kernel/syscalls/pidfd_send_signal/.gitignore
create mode 100644 testcases/kernel/syscalls/pidfd_send_signal/Makefile
create mode 100644 testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal.h
create mode 100644 testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal01.c
diff --git a/runtest/syscalls b/runtest/syscalls
index 2b8ca719b..c719b77b2 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -838,6 +838,8 @@ pause03 pause03
personality01 personality01
personality02 personality02
+pidfd_send_signal01 pidfd_send_signal01
+
pipe01 pipe01
pipe02 pipe02
pipe03 pipe03
diff --git a/testcases/kernel/syscalls/pidfd_send_signal/.gitignore b/testcases/kernel/syscalls/pidfd_send_signal/.gitignore
new file mode 100644
index 000000000..7ccbf2d80
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_send_signal/.gitignore
@@ -0,0 +1 @@
+/pidfd_send_signal01
diff --git a/testcases/kernel/syscalls/pidfd_send_signal/Makefile b/testcases/kernel/syscalls/pidfd_send_signal/Makefile
new file mode 100644
index 000000000..23e4ec478
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_send_signal/Makefile
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#
+# Copyright (c) 2019 SUSE LLC
+#
+# Author: Christian Amann <camann@suse.com>
+#
+
+top_srcdir ?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
+
+pidfd_send_signal01: CFLAGS += -pthread
diff --git a/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal.h b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal.h
new file mode 100644
index 000000000..3553c5464
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal.h
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 SUSE LLC
+ * Author: Christian Amann <camann@suse.com>
+ */
+
+#ifndef __PIDFD_SEND_SIGNAL_H__
+#define __PIDFD_SEND_SIGNAL_H__
+
+#include "tst_test.h"
+#include "lapi/syscalls.h"
+
+static int tst_pidfd_send_signal(int pidfd, int sig, siginfo_t *info,
+ unsigned int flags)
+{
+ return tst_syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags);
+}
+
+
+#endif /* __PIDFD_SEND_SIGNAL_H__ */
diff --git a/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal01.c b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal01.c
new file mode 100644
index 000000000..9ab1971bf
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal01.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 SUSE LLC
+ * Author: Christian Amann <camann@suse.com>
+ */
+/*
+ * Tests if the pidfd_send_signal systemcall behaves
+ * like rt_sigqueueinfo when a pointer to a siginfo_t
+ * struct is passed.
+ */
+
+#define _GNU_SOURCE
+
+#include <signal.h>
+#include <stdlib.h>
+#include "tst_safe_pthread.h"
+#include "pidfd_send_signal.h"
+
+#define SIGNAL SIGUSR1
+#define DATA 777
+
+static struct sigaction *sig_action;
+static int sig_rec;
+static siginfo_t *uinfo;
+static int pidfd;
+
+static void received_signal(int sig, siginfo_t *info, void *ucontext)
+{
+ if (info && ucontext) {
+ if (sig == SIGNAL && uinfo->si_value.sival_int == DATA) {
+ tst_res(TPASS, "Received correct signal and data!");
+ sig_rec = 1;
+ } else
+ tst_res(TFAIL, "Received wrong signal and/or data!");
+ } else
+ tst_res(TFAIL, "Signal handling went wrong!");
+}
+
+static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
+{
+ int ret;
+
+ ret = sigaction(SIGNAL, sig_action, NULL);
+ if (ret)
+ tst_brk(TBROK | TTERRNO, "Failed to set sigaction");
+
+ TST_CHECKPOINT_WAKE(0);
+ TST_CHECKPOINT_WAIT(1);
+ return arg;
+}
+
+static void verify_pidfd_send_signal(void)
+{
+ pthread_t thr;
+
+ SAFE_PTHREAD_CREATE(&thr, NULL, handle_thread, NULL);
+
+ TST_CHECKPOINT_WAIT(0);
+
+ TEST(tst_pidfd_send_signal(pidfd, SIGNAL, uinfo, 0));
+ if (TST_RET != 0) {
+ tst_res(TFAIL | TTERRNO, "pidfd_send_signal() failed");
+ return;
+ }
+
+ TST_CHECKPOINT_WAKE(1);
+ SAFE_PTHREAD_JOIN(thr, NULL);
+
+ if (sig_rec)
+ tst_res(TPASS,
+ "pidfd_send_signal() behaved like rt_sigqueueinfo()");
+}
+
+static void setup(void)
+{
+ pidfd = SAFE_OPEN("/proc/self", O_DIRECTORY | O_CLOEXEC);
+
+ sig_action = SAFE_MALLOC(sizeof(struct sigaction));
+
+ memset(sig_action, 0, sizeof(*sig_action));
+ sig_action->sa_sigaction = received_signal;
+ sig_action->sa_flags = SA_SIGINFO;
+
+ uinfo = SAFE_MALLOC(sizeof(siginfo_t));
+
+ memset(uinfo, 0, sizeof(*uinfo));
+ uinfo->si_signo = SIGNAL;
+ uinfo->si_code = SI_QUEUE;
+ uinfo->si_pid = getpid();
+ uinfo->si_uid = getuid();
+ uinfo->si_value.sival_int = DATA;
+
+ sig_rec = 0;
+}
+
+static void cleanup(void)
+{
+ free(uinfo);
+ free(sig_action);
+ if (pidfd > 0)
+ SAFE_CLOSE(pidfd);
+}
+
+static struct tst_test test = {
+ .test_all = verify_pidfd_send_signal,
+ .setup = setup,
+ .cleanup = cleanup,
+ .needs_checkpoints = 1,
+ .timeout = 20,
+};
--
2.16.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [LTP] [PATCH v1 2/4] syscalls/pidfd_send_signal01
2019-05-15 12:01 ` [LTP] [PATCH v1 2/4] syscalls/pidfd_send_signal01 Christian Amann
@ 2019-05-28 11:30 ` Cyril Hrubis
0 siblings, 0 replies; 8+ messages in thread
From: Cyril Hrubis @ 2019-05-28 11:30 UTC (permalink / raw)
To: ltp
Hi!
> new file mode 100644
> index 000000000..3553c5464
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal.h
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 SUSE LLC
> + * Author: Christian Amann <camann@suse.com>
> + */
> +
> +#ifndef __PIDFD_SEND_SIGNAL_H__
> +#define __PIDFD_SEND_SIGNAL_H__
^
Plese don't use undescores at the beginning of identifiers.
Identifiers starting with _ and __ are reserved for the system, i.e.
libc and compiler.
> +#include "tst_test.h"
> +#include "lapi/syscalls.h"
> +
> +static int tst_pidfd_send_signal(int pidfd, int sig, siginfo_t *info,
> + unsigned int flags)
We probably should not use the tst_ prefix here as this is not a test
library code.
I guess that we can make this future proof with a configure check for
pidfd_send_signal() that will get included in glibc later on.
> +{
> + return tst_syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags);
> +}
> +
> +
> +#endif /* __PIDFD_SEND_SIGNAL_H__ */
> diff --git a/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal01.c b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal01.c
> new file mode 100644
> index 000000000..9ab1971bf
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal01.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 SUSE LLC
> + * Author: Christian Amann <camann@suse.com>
> + */
> +/*
> + * Tests if the pidfd_send_signal systemcall behaves
^
Should either be one of:
* syscall
* system call
> + * like rt_sigqueueinfo when a pointer to a siginfo_t
> + * struct is passed.
> + */
> +
> +#define _GNU_SOURCE
> +
> +#include <signal.h>
> +#include <stdlib.h>
> +#include "tst_safe_pthread.h"
> +#include "pidfd_send_signal.h"
> +
> +#define SIGNAL SIGUSR1
> +#define DATA 777
> +
> +static struct sigaction *sig_action;
> +static int sig_rec;
> +static siginfo_t *uinfo;
> +static int pidfd;
> +
> +static void received_signal(int sig, siginfo_t *info, void *ucontext)
> +{
> + if (info && ucontext) {
> + if (sig == SIGNAL && uinfo->si_value.sival_int == DATA) {
> + tst_res(TPASS, "Received correct signal and data!");
> + sig_rec = 1;
> + } else
> + tst_res(TFAIL, "Received wrong signal and/or data!");
> + } else
> + tst_res(TFAIL, "Signal handling went wrong!");
This is a very minor however LKML coding style prefers to have curly
braces around both blocks if they have to be over one of them.
> +}
> +
> +static void *handle_thread(void *arg LTP_ATTRIBUTE_UNUSED)
^
You do return arg at the end of this function,
there is no point in the ATTRIBUTE_UNUSED
> +{
> + int ret;
> +
> + ret = sigaction(SIGNAL, sig_action, NULL);
> + if (ret)
> + tst_brk(TBROK | TTERRNO, "Failed to set sigaction");
Why not SAFE_SIGACTION() ?
> + TST_CHECKPOINT_WAKE(0);
> + TST_CHECKPOINT_WAIT(1);
We do have TST_CHECKPOINT_WAKE_AND_WAIT().
> + return arg;
> +}
> +
> +static void verify_pidfd_send_signal(void)
> +{
> + pthread_t thr;
> +
> + SAFE_PTHREAD_CREATE(&thr, NULL, handle_thread, NULL);
> +
> + TST_CHECKPOINT_WAIT(0);
> +
> + TEST(tst_pidfd_send_signal(pidfd, SIGNAL, uinfo, 0));
> + if (TST_RET != 0) {
> + tst_res(TFAIL | TTERRNO, "pidfd_send_signal() failed");
> + return;
> + }
> +
> + TST_CHECKPOINT_WAKE(1);
> + SAFE_PTHREAD_JOIN(thr, NULL);
> +
> + if (sig_rec)
> + tst_res(TPASS,
> + "pidfd_send_signal() behaved like rt_sigqueueinfo()");
Very minor as well, LKML coding style prefers curly braces around
multiline statements like this one.
> +}
> +
> +static void setup(void)
> +{
> + pidfd = SAFE_OPEN("/proc/self", O_DIRECTORY | O_CLOEXEC);
> +
> + sig_action = SAFE_MALLOC(sizeof(struct sigaction));
> +
> + memset(sig_action, 0, sizeof(*sig_action));
> + sig_action->sa_sigaction = received_signal;
> + sig_action->sa_flags = SA_SIGINFO;
> +
> + uinfo = SAFE_MALLOC(sizeof(siginfo_t));
> +
> + memset(uinfo, 0, sizeof(*uinfo));
> + uinfo->si_signo = SIGNAL;
> + uinfo->si_code = SI_QUEUE;
> + uinfo->si_pid = getpid();
> + uinfo->si_uid = getuid();
> + uinfo->si_value.sival_int = DATA;
> +
> + sig_rec = 0;
> +}
> +
> +static void cleanup(void)
> +{
> + free(uinfo);
> + free(sig_action);
> + if (pidfd > 0)
> + SAFE_CLOSE(pidfd);
> +}
> +
> +static struct tst_test test = {
> + .test_all = verify_pidfd_send_signal,
> + .setup = setup,
> + .cleanup = cleanup,
> + .needs_checkpoints = 1,
> + .timeout = 20,
Please do not change the default timeout unless it's needed.
There are actually only two situations where this makese sense:
* The test is expected to run for a long time
=> timeout needs to be increased
* The test goes into an infinit loop on a buggy kernel
=> timeout has to be set low, so that we do not waste time
> +};
> --
> 2.16.4
Other than the minor comments, the test looks good.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v1 3/4] syscalls/pidfd_send_signal02
2019-05-15 12:01 [LTP] [PATCH v1 1/4] Add Syscall numbers for pidfd_send_signal Christian Amann
2019-05-15 12:01 ` [LTP] [PATCH v1 2/4] syscalls/pidfd_send_signal01 Christian Amann
@ 2019-05-15 12:01 ` Christian Amann
2019-05-28 11:38 ` Cyril Hrubis
2019-05-15 12:01 ` [LTP] [PATCH v1 4/4] syscalls/pidfd_send_signal03 Christian Amann
2019-05-15 12:44 ` [LTP] [PATCH v1 1/4] Add Syscall numbers for pidfd_send_signal Petr Vorel
3 siblings, 1 reply; 8+ messages in thread
From: Christian Amann @ 2019-05-15 12:01 UTC (permalink / raw)
To: ltp
Add test to check basic error handling of the syscall.
Signed-off-by: Christian Amann <camann@suse.com>
---
runtest/syscalls | 1 +
.../kernel/syscalls/pidfd_send_signal/.gitignore | 1 +
.../pidfd_send_signal/pidfd_send_signal02.c | 104 +++++++++++++++++++++
3 files changed, 106 insertions(+)
create mode 100644 testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal02.c
diff --git a/runtest/syscalls b/runtest/syscalls
index c719b77b2..fd57e2ee7 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -839,6 +839,7 @@ personality01 personality01
personality02 personality02
pidfd_send_signal01 pidfd_send_signal01
+pidfd_send_signal02 pidfd_send_signal02
pipe01 pipe01
pipe02 pipe02
diff --git a/testcases/kernel/syscalls/pidfd_send_signal/.gitignore b/testcases/kernel/syscalls/pidfd_send_signal/.gitignore
index 7ccbf2d80..6ea6401a8 100644
--- a/testcases/kernel/syscalls/pidfd_send_signal/.gitignore
+++ b/testcases/kernel/syscalls/pidfd_send_signal/.gitignore
@@ -1 +1,2 @@
/pidfd_send_signal01
+/pidfd_send_signal02
diff --git a/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal02.c b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal02.c
new file mode 100644
index 000000000..25e66a214
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal02.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 SUSE LLC
+ * Author: Christian Amann <camann@suse.com>
+ */
+/*
+ * Tests basic error handling of the pidfd_send_signal
+ * system call.
+ *
+ * 1) Pass invalid flag value to syscall (value chosen
+ * to be unlikely to collide with future extensions)
+ * -> EINVAL
+ * 2) Pass a file descriptor that is corresponding to a
+ * regular file instead of a pid directory
+ * -> EBADF
+ * 3) Pass a signal that is different from the one used
+ * to initialize the siginfo_t struct
+ * -> EINVAL
+ * 4) Try to send signal to other process (init) with
+ * missing privileges
+ * -> EPERM
+ */
+
+#define _GNU_SOURCE
+
+#include <signal.h>
+#include "pwd.h"
+#include "tst_safe_pthread.h"
+#include "pidfd_send_signal.h"
+
+#define CORRECT_SIGNAL SIGUSR1
+#define DIFFERENT_SIGNAL SIGUSR2
+
+static siginfo_t info;
+static int pidfd;
+static int init_pidfd;
+static int dummyfd;
+
+static struct tcase {
+ int *fd;
+ siginfo_t *siginf;
+ int signal;
+ int flags;
+ int exp_err;
+} tcases[] = {
+ {&pidfd, &info, CORRECT_SIGNAL, 99999, EINVAL},
+ {&dummyfd, &info, CORRECT_SIGNAL, 0, EBADF},
+ {&pidfd, &info, DIFFERENT_SIGNAL, 0, EINVAL},
+ {&init_pidfd, &info, CORRECT_SIGNAL, 0, EPERM},
+};
+
+static void verify_pidfd_send_signal(unsigned int n)
+{
+ struct tcase *tc = &tcases[n];
+
+ TEST(tst_pidfd_send_signal(*tc->fd, tc->signal, tc->siginf, tc->flags));
+ if (tc->exp_err != TST_ERR) {
+ tst_res(TFAIL | TTERRNO,
+ "pidfd_send_signal() did not fail with %s but",
+ tst_strerrno(tc->exp_err));
+ return;
+ }
+
+ tst_res(TPASS,
+ "pidfd_send_signal() failed as expected!");
+}
+
+static void setup(void)
+{
+ struct passwd *pw;
+
+ pidfd = SAFE_OPEN("/proc/self", O_DIRECTORY | O_CLOEXEC);
+ init_pidfd = SAFE_OPEN("/proc/1", O_DIRECTORY | O_CLOEXEC);
+ dummyfd = SAFE_OPEN("dummy_file", O_RDWR | O_CREAT, 0664);
+
+ if (getuid() == 0) {
+ pw = SAFE_GETPWNAM("nobody");
+ SAFE_SETUID(pw->pw_uid);
+ }
+
+ memset(&info, 0, sizeof(info));
+ info.si_signo = CORRECT_SIGNAL;
+ info.si_code = SI_QUEUE;
+ info.si_pid = getpid();
+ info.si_uid = getuid();
+}
+
+static void cleanup(void)
+{
+ if (dummyfd > 0)
+ SAFE_CLOSE(dummyfd);
+ if (init_pidfd > 0)
+ SAFE_CLOSE(init_pidfd);
+ if (pidfd > 0)
+ SAFE_CLOSE(pidfd);
+}
+
+static struct tst_test test = {
+ .test = verify_pidfd_send_signal,
+ .tcnt = ARRAY_SIZE(tcases),
+ .setup = setup,
+ .cleanup = cleanup,
+ .needs_tmpdir = 1,
+};
--
2.16.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [LTP] [PATCH v1 3/4] syscalls/pidfd_send_signal02
2019-05-15 12:01 ` [LTP] [PATCH v1 3/4] syscalls/pidfd_send_signal02 Christian Amann
@ 2019-05-28 11:38 ` Cyril Hrubis
0 siblings, 0 replies; 8+ messages in thread
From: Cyril Hrubis @ 2019-05-28 11:38 UTC (permalink / raw)
To: ltp
Hi!
> +static void verify_pidfd_send_signal(unsigned int n)
> +{
> + struct tcase *tc = &tcases[n];
> +
> + TEST(tst_pidfd_send_signal(*tc->fd, tc->signal, tc->siginf, tc->flags));
> + if (tc->exp_err != TST_ERR) {
> + tst_res(TFAIL | TTERRNO,
> + "pidfd_send_signal() did not fail with %s but",
> + tst_strerrno(tc->exp_err));
> + return;
> + }
> +
> + tst_res(TPASS,
> + "pidfd_send_signal() failed as expected!");
I tend to pass TTERRNO to the passing message for negative testcases
like this one so that the output has some information about what has
been tested.
I would have done something as here:
tst_res(TPASS | TTERRNO, "pidfd_send_signal() failed");
Other than this the test looks good.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v1 4/4] syscalls/pidfd_send_signal03
2019-05-15 12:01 [LTP] [PATCH v1 1/4] Add Syscall numbers for pidfd_send_signal Christian Amann
2019-05-15 12:01 ` [LTP] [PATCH v1 2/4] syscalls/pidfd_send_signal01 Christian Amann
2019-05-15 12:01 ` [LTP] [PATCH v1 3/4] syscalls/pidfd_send_signal02 Christian Amann
@ 2019-05-15 12:01 ` Christian Amann
2019-05-28 13:22 ` Cyril Hrubis
2019-05-15 12:44 ` [LTP] [PATCH v1 1/4] Add Syscall numbers for pidfd_send_signal Petr Vorel
3 siblings, 1 reply; 8+ messages in thread
From: Christian Amann @ 2019-05-15 12:01 UTC (permalink / raw)
To: ltp
Add testcase to check if the syscall will send a signal
to a process with the same PID as the targeted process.
Signed-off-by: Christian Amann <camann@suse.com>
---
runtest/syscalls | 1 +
.../kernel/syscalls/pidfd_send_signal/.gitignore | 1 +
.../pidfd_send_signal/pidfd_send_signal03.c | 118 +++++++++++++++++++++
3 files changed, 120 insertions(+)
create mode 100644 testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal03.c
diff --git a/runtest/syscalls b/runtest/syscalls
index fd57e2ee7..a534a663b 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -840,6 +840,7 @@ personality02 personality02
pidfd_send_signal01 pidfd_send_signal01
pidfd_send_signal02 pidfd_send_signal02
+pidfd_send_signal03 pidfd_send_signal03
pipe01 pipe01
pipe02 pipe02
diff --git a/testcases/kernel/syscalls/pidfd_send_signal/.gitignore b/testcases/kernel/syscalls/pidfd_send_signal/.gitignore
index 6ea6401a8..cba7d50a4 100644
--- a/testcases/kernel/syscalls/pidfd_send_signal/.gitignore
+++ b/testcases/kernel/syscalls/pidfd_send_signal/.gitignore
@@ -1,2 +1,3 @@
/pidfd_send_signal01
/pidfd_send_signal02
+/pidfd_send_signal03
diff --git a/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal03.c b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal03.c
new file mode 100644
index 000000000..ee17e714e
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_send_signal/pidfd_send_signal03.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 SUSE LLC
+ * Author: Christian Amann <camann@suse.com>
+ */
+/*
+ * This test checks if the pidfd_send_signal syscall wrongfully sends
+ * a signal to a new process which inherited the PID of the actual
+ * target process.
+ * In order to do so it is necessary to start a process with a pre-
+ * determined PID. This is accomplished by writing to the
+ * /proc/sys/kernel/ns_last_pid file.
+ * By utilizing this, this test forks two children with the same PID.
+ * It is then checked, if the syscall will send a signal to the second
+ * child using the pidfd of the first one.
+ */
+
+#define _GNU_SOURCE
+
+#include <signal.h>
+#include <stdio.h>
+#include <unistd.h>
+#include "pidfd_send_signal.h"
+#include "tst_safe_pthread.h"
+
+#define PIDTRIES 3
+
+static char *last_pid_file;
+static int pidfd, last_pidfd;
+
+static void verify_pidfd_send_signal(void)
+{
+ pid_t pid, new_pid;
+ char pid_filename[32];
+ char pid_str[16];
+ int i, fail;
+
+ fail = 1;
+ for (i = 1; i <= PIDTRIES; i++) {
+ pid = SAFE_FORK();
+ if (pid == 0) {
+ TST_CHECKPOINT_WAIT(0);
+ return;
+ }
+
+ sprintf(pid_filename, "/proc/%d", pid);
+ pidfd = SAFE_OPEN(pid_filename, O_DIRECTORY | O_CLOEXEC);
+
+ TST_CHECKPOINT_WAKE(0);
+ tst_reap_children();
+
+ /* Manipulate PID for next process */
+ sprintf(pid_str, "%d", pid - 1);
+ SAFE_LSEEK(last_pidfd, 0, SEEK_SET);
+ SAFE_WRITE(1, last_pidfd, pid_str, strlen(pid_str));
+
+ new_pid = SAFE_FORK();
+ if (new_pid == 0) {
+ TST_CHECKPOINT_WAIT(i);
+ return;
+ } else if (new_pid == pid) {
+ fail = 0;
+ break;
+ }
+
+ if (i < PIDTRIES)
+ tst_res(TINFO,
+ "Failed to set correct PID, trying again...");
+ SAFE_CLOSE(pidfd);
+ TST_CHECKPOINT_WAKE(i);
+ tst_reap_children();
+ }
+ if (fail)
+ tst_brk(TBROK,
+ "Could not set new child to same PID as the old one!");
+
+ TEST(tst_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0));
+ if (TST_RET == -1 && TST_ERR == ESRCH) {
+ tst_res(TPASS,
+ "Did not send signal to wrong process with same PID!");
+ } else
+ tst_res(TFAIL | TTERRNO,
+ "pidf_send_signal() ended unexpectedly - return value: %ld, error",
+ TST_RET);
+
+ TST_CHECKPOINT_WAKE(i);
+ tst_reap_children();
+
+ SAFE_CLOSE(pidfd);
+}
+
+static void setup(void)
+{
+ last_pid_file = "/proc/sys/kernel/ns_last_pid";
+ if (access(last_pid_file, F_OK) == -1)
+ tst_brk(TCONF, "%s does not exist, cannot set PIDs",
+ last_pid_file);
+ last_pidfd = SAFE_OPEN(last_pid_file, O_RDWR);
+}
+
+static void cleanup(void)
+{
+ tst_reap_children();
+ if (pidfd > 0)
+ SAFE_CLOSE(pidfd);
+ if (last_pidfd > 0)
+ SAFE_CLOSE(last_pidfd);
+}
+
+static struct tst_test test = {
+ .test_all = verify_pidfd_send_signal,
+ .setup = setup,
+ .cleanup = cleanup,
+ .needs_root = 1,
+ .needs_checkpoints = 1,
+ .forks_child = 1,
+ .timeout = 20,
+};
--
2.16.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [LTP] [PATCH v1 4/4] syscalls/pidfd_send_signal03
2019-05-15 12:01 ` [LTP] [PATCH v1 4/4] syscalls/pidfd_send_signal03 Christian Amann
@ 2019-05-28 13:22 ` Cyril Hrubis
0 siblings, 0 replies; 8+ messages in thread
From: Cyril Hrubis @ 2019-05-28 13:22 UTC (permalink / raw)
To: ltp
Hi!
> +#define _GNU_SOURCE
> +
> +#include <signal.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include "pidfd_send_signal.h"
> +#include "tst_safe_pthread.h"
> +
> +#define PIDTRIES 3
> +
> +static char *last_pid_file;
> +static int pidfd, last_pidfd;
> +
> +static void verify_pidfd_send_signal(void)
> +{
> + pid_t pid, new_pid;
> + char pid_filename[32];
> + char pid_str[16];
> + int i, fail;
> +
> + fail = 1;
> + for (i = 1; i <= PIDTRIES; i++) {
> + pid = SAFE_FORK();
> + if (pid == 0) {
> + TST_CHECKPOINT_WAIT(0);
> + return;
> + }
> +
> + sprintf(pid_filename, "/proc/%d", pid);
> + pidfd = SAFE_OPEN(pid_filename, O_DIRECTORY | O_CLOEXEC);
> +
> + TST_CHECKPOINT_WAKE(0);
> + tst_reap_children();
> +
> + /* Manipulate PID for next process */
> + sprintf(pid_str, "%d", pid - 1);
> + SAFE_LSEEK(last_pidfd, 0, SEEK_SET);
> + SAFE_WRITE(1, last_pidfd, pid_str, strlen(pid_str));
We do have SAFE_FILE_PRINTF() for this purpose.
> + new_pid = SAFE_FORK();
> + if (new_pid == 0) {
> + TST_CHECKPOINT_WAIT(i);
> + return;
> + } else if (new_pid == pid) {
No need for else here if you do return in the if above.
> + fail = 0;
> + break;
> + }
> +
> + if (i < PIDTRIES)
> + tst_res(TINFO,
> + "Failed to set correct PID, trying again...");
> + SAFE_CLOSE(pidfd);
> + TST_CHECKPOINT_WAKE(i);
Do we really need to use checkpoint i here? The checkpoint 0 should be
unused at this point...
> + tst_reap_children();
> + }
> + if (fail)
> + tst_brk(TBROK,
> + "Could not set new child to same PID as the old one!");
> +
> + TEST(tst_pidfd_send_signal(pidfd, SIGUSR1, NULL, 0));
> + if (TST_RET == -1 && TST_ERR == ESRCH) {
> + tst_res(TPASS,
> + "Did not send signal to wrong process with same PID!");
> + } else
> + tst_res(TFAIL | TTERRNO,
> + "pidf_send_signal() ended unexpectedly - return value: %ld, error",
> + TST_RET);
Other obvious test would be opening the pidfd of the new pid and
comparing if they point out to the same file by comparing i-nodes, see
ioctl_ns05.c.
> + TST_CHECKPOINT_WAKE(i);
> + tst_reap_children();
> +
> + SAFE_CLOSE(pidfd);
> +}
> +
> +static void setup(void)
> +{
> + last_pid_file = "/proc/sys/kernel/ns_last_pid";
> + if (access(last_pid_file, F_OK) == -1)
> + tst_brk(TCONF, "%s does not exist, cannot set PIDs",
> + last_pid_file);
> + last_pidfd = SAFE_OPEN(last_pid_file, O_RDWR);
> +}
> +
> +static void cleanup(void)
> +{
> + tst_reap_children();
> + if (pidfd > 0)
> + SAFE_CLOSE(pidfd);
> + if (last_pidfd > 0)
> + SAFE_CLOSE(last_pidfd);
> +}
> +
> +static struct tst_test test = {
> + .test_all = verify_pidfd_send_signal,
> + .setup = setup,
> + .cleanup = cleanup,
> + .needs_root = 1,
> + .needs_checkpoints = 1,
> + .forks_child = 1,
> + .timeout = 20,
> +};
> --
> 2.16.4
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 8+ messages in thread
* [LTP] [PATCH v1 1/4] Add Syscall numbers for pidfd_send_signal
2019-05-15 12:01 [LTP] [PATCH v1 1/4] Add Syscall numbers for pidfd_send_signal Christian Amann
` (2 preceding siblings ...)
2019-05-15 12:01 ` [LTP] [PATCH v1 4/4] syscalls/pidfd_send_signal03 Christian Amann
@ 2019-05-15 12:44 ` Petr Vorel
3 siblings, 0 replies; 8+ messages in thread
From: Petr Vorel @ 2019-05-15 12:44 UTC (permalink / raw)
To: ltp
Hi Christian,
> Signed-off-by: Christian Amann <camann@suse.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
^ permalink raw reply [flat|nested] 8+ messages in thread