[LTP] [PATCH 1/2] Add Syscall numbers for pidfd_open

[LTP] [PATCH 1/2] Add Syscall numbers for pidfd_open

[Viresh Kumar]

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 include/lapi/syscalls/aarch64.in   | 1 +
 include/lapi/syscalls/arm.in       | 1 +
 include/lapi/syscalls/hppa.in      | 1 +
 include/lapi/syscalls/i386.in      | 1 +
 include/lapi/syscalls/ia64.in      | 1 +
 include/lapi/syscalls/powerpc.in   | 1 +
 include/lapi/syscalls/powerpc64.in | 1 +
 include/lapi/syscalls/s390.in      | 1 +
 include/lapi/syscalls/s390x.in     | 1 +
 include/lapi/syscalls/sh.in        | 1 +
 include/lapi/syscalls/sparc.in     | 1 +
 include/lapi/syscalls/sparc64.in   | 1 +
 include/lapi/syscalls/x86_64.in    | 1 +
 13 files changed, 13 insertions(+)

diff --git a/include/lapi/syscalls/aarch64.in b/include/lapi/syscalls/aarch64.in
index 0e00641bcf4d..19f3aa4475bd 100644
--- a/include/lapi/syscalls/aarch64.in
+++ b/include/lapi/syscalls/aarch64.in
@@ -270,4 +270,5 @@ pkey_mprotect 288
 pkey_alloc 289
 pkey_free 290
 pidfd_send_signal 424
+pidfd_open 434
 _sysctl 1078
diff --git a/include/lapi/syscalls/arm.in b/include/lapi/syscalls/arm.in
index 00e99f2b99b8..29538ba50d3b 100644
--- a/include/lapi/syscalls/arm.in
+++ b/include/lapi/syscalls/arm.in
@@ -354,3 +354,4 @@ pkey_alloc (__NR_SYSCALL_BASE+395)
 pkey_free (__NR_SYSCALL_BASE+396)
 statx (__NR_SYSCALL_BASE+397)
 pidfd_send_signal (__NR_SYSCALL_BASE+424)
+pidfd_open (__NR_SYSCALL_BASE+434)
diff --git a/include/lapi/syscalls/hppa.in b/include/lapi/syscalls/hppa.in
index 4cdd109fb50e..b578d886516d 100644
--- a/include/lapi/syscalls/hppa.in
+++ b/include/lapi/syscalls/hppa.in
@@ -26,3 +26,4 @@ copy_file_range 346
 preadv2 347
 pwritev2 348
 pidfd_send_signal 424
+pidfd_open 434
diff --git a/include/lapi/syscalls/i386.in b/include/lapi/syscalls/i386.in
index 87ab4693343e..696563ebba48 100644
--- a/include/lapi/syscalls/i386.in
+++ b/include/lapi/syscalls/i386.in
@@ -353,3 +353,4 @@ pkey_alloc 381
 pkey_free 382
 statx 383
 pidfd_send_signal 424
+pidfd_open 434
diff --git a/include/lapi/syscalls/ia64.in b/include/lapi/syscalls/ia64.in
index cf9f73e8529b..11b236c7c507 100644
--- a/include/lapi/syscalls/ia64.in
+++ b/include/lapi/syscalls/ia64.in
@@ -309,3 +309,4 @@ pkey_mprotect 1354
 pkey_alloc 1355
 pkey_free 1356
 pidfd_send_signal 1448
+pidfd_open 1458
diff --git a/include/lapi/syscalls/powerpc.in b/include/lapi/syscalls/powerpc.in
index 660165d7a37b..293d86ba54c4 100644
--- a/include/lapi/syscalls/powerpc.in
+++ b/include/lapi/syscalls/powerpc.in
@@ -356,6 +356,7 @@ preadv2 380
 pwritev2 381
 statx 383
 pidfd_send_signal 424
+pidfd_open 434
 pkey_mprotect 386
 pkey_alloc 384
 pkey_free 385
diff --git a/include/lapi/syscalls/powerpc64.in b/include/lapi/syscalls/powerpc64.in
index 660165d7a37b..293d86ba54c4 100644
--- a/include/lapi/syscalls/powerpc64.in
+++ b/include/lapi/syscalls/powerpc64.in
@@ -356,6 +356,7 @@ preadv2 380
 pwritev2 381
 statx 383
 pidfd_send_signal 424
+pidfd_open 434
 pkey_mprotect 386
 pkey_alloc 384
 pkey_free 385
diff --git a/include/lapi/syscalls/s390.in b/include/lapi/syscalls/s390.in
index d3f7eb1f60f7..4f4b2dbed5ef 100644
--- a/include/lapi/syscalls/s390.in
+++ b/include/lapi/syscalls/s390.in
@@ -343,3 +343,4 @@ pkey_mprotect 384
 pkey_alloc 385
 pkey_free 386
 pidfd_send_signal 424
+pidfd_open 434
diff --git a/include/lapi/syscalls/s390x.in b/include/lapi/syscalls/s390x.in
index 7d632d1dc0bb..673538a7217e 100644
--- a/include/lapi/syscalls/s390x.in
+++ b/include/lapi/syscalls/s390x.in
@@ -341,3 +341,4 @@ pkey_mprotect 384
 pkey_alloc 385
 pkey_free 386
 pidfd_send_signal 424
+pidfd_open 434
diff --git a/include/lapi/syscalls/sh.in b/include/lapi/syscalls/sh.in
index 132492922659..ef877fa9c25f 100644
--- a/include/lapi/syscalls/sh.in
+++ b/include/lapi/syscalls/sh.in
@@ -370,3 +370,4 @@ copy_file_range 391
 preadv2 392
 pwritev2 393
 pidfd_send_signal 424
+pidfd_open 434
diff --git a/include/lapi/syscalls/sparc.in b/include/lapi/syscalls/sparc.in
index 94a672428973..2b33c4983088 100644
--- a/include/lapi/syscalls/sparc.in
+++ b/include/lapi/syscalls/sparc.in
@@ -348,3 +348,4 @@ pkey_mprotect 362
 pkey_alloc 363
 pkey_free 364
 pidfd_send_signal 424
+pidfd_open 434
diff --git a/include/lapi/syscalls/sparc64.in b/include/lapi/syscalls/sparc64.in
index d17dce5cd9a3..a6157adef7b3 100644
--- a/include/lapi/syscalls/sparc64.in
+++ b/include/lapi/syscalls/sparc64.in
@@ -323,3 +323,4 @@ pkey_mprotect 362
 pkey_alloc 363
 pkey_free 364
 pidfd_send_signal 424
+pidfd_open 434
diff --git a/include/lapi/syscalls/x86_64.in b/include/lapi/syscalls/x86_64.in
index b1cbd4f2fb04..8bd6b66d3bef 100644
--- a/include/lapi/syscalls/x86_64.in
+++ b/include/lapi/syscalls/x86_64.in
@@ -320,3 +320,4 @@ pkey_alloc 330
 pkey_free 331
 statx 332
 pidfd_send_signal 424
+pidfd_open 434
-- 
2.21.0.rc0.269.g1a574e7a288b

[LTP] [PATCH 2/2] syscalls/pidfd_open

[Viresh Kumar]

Add tests to check working of pidfd_open() syscall.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 configure.ac                                  |  1 +
 include/lapi/pidfd_open.h                     | 21 ++++++
 runtest/syscalls                              |  3 +
 .../kernel/syscalls/pidfd_open/.gitignore     |  2 +
 testcases/kernel/syscalls/pidfd_open/Makefile |  6 ++
 .../kernel/syscalls/pidfd_open/pidfd_open01.c | 38 +++++++++++
 .../kernel/syscalls/pidfd_open/pidfd_open02.c | 68 +++++++++++++++++++
 7 files changed, 139 insertions(+)
 create mode 100644 include/lapi/pidfd_open.h
 create mode 100644 testcases/kernel/syscalls/pidfd_open/.gitignore
 create mode 100644 testcases/kernel/syscalls/pidfd_open/Makefile
 create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
 create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open02.c

diff --git a/configure.ac b/configure.ac
index 50d14967d3c6..1bf0911d88ad 100644
--- a/configure.ac
+++ b/configure.ac
@@ -79,6 +79,7 @@ AC_CHECK_FUNCS([ \
     mknodat \
     name_to_handle_at \
     openat \
+    pidfd_open \
     pidfd_send_signal \
     pkey_mprotect \
     preadv \
diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h
new file mode 100644
index 000000000000..ced163be83bf
--- /dev/null
+++ b/include/lapi/pidfd_open.h
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Linaro Limited. All rights reserved.
+ * Author: Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+#ifndef PIDFD_OPEN_H
+#define PIDFD_OPEN_H
+
+#include "config.h"
+#include <sys/types.h>
+#include "lapi/syscalls.h"
+
+#ifndef HAVE_PIDFD_OPEN
+int pidfd_open(pid_t pid, unsigned int flags)
+{
+	return tst_syscall(__NR_pidfd_open, pid, flags);
+}
+#endif
+
+#endif /* PIDFD_OPEN_H */
diff --git a/runtest/syscalls b/runtest/syscalls
index fa87ef63fbc1..9d6d288780a3 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -846,6 +846,9 @@ pause03 pause03
 personality01 personality01
 personality02 personality02
 
+pidfd_open01 pidfd_open01
+pidfd_open02 pidfd_open02
+
 pidfd_send_signal01 pidfd_send_signal01
 pidfd_send_signal02 pidfd_send_signal02
 pidfd_send_signal03 pidfd_send_signal03
diff --git a/testcases/kernel/syscalls/pidfd_open/.gitignore b/testcases/kernel/syscalls/pidfd_open/.gitignore
new file mode 100644
index 000000000000..be218f88647d
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/.gitignore
@@ -0,0 +1,2 @@
+pidfd_open01
+pidfd_open02
diff --git a/testcases/kernel/syscalls/pidfd_open/Makefile b/testcases/kernel/syscalls/pidfd_open/Makefile
new file mode 100644
index 000000000000..5ea7d67db123
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
new file mode 100644
index 000000000000..4e3417c487a6
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Description:
+ * Basic pidfd_open() test, fetches the PID of the current process and tries to
+ * get its file descriptor.
+ */
+#include <sys/types.h>
+#include <sys/syscall.h>
+
+#include "tst_test.h"
+#include "lapi/pidfd_open.h"
+#include "lapi/syscalls.h"
+
+static void run(void)
+{
+	int p_id, fd;
+
+	p_id = getpid();
+
+	TEST(pidfd_open(p_id, 0));
+
+	fd = TST_RET;
+	if (fd == -1) {
+		tst_res(TFAIL, "Cannot retrieve file descriptor to the current process");
+		return;
+	}
+
+	SAFE_CLOSE(fd);
+
+	tst_res(TPASS, "Retrieved file descriptor to the current process");
+}
+
+static struct tst_test test = {
+	.min_kver = "5.3",
+	.test_all = run,
+};
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
new file mode 100644
index 000000000000..b67393bcafa2
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Description:
+ * This program opens the PID file descriptor of the child process created with
+ * fork(). It then uses poll to monitor the file descriptor for process exit, as
+ * indicated by an EPOLLIN event.
+ */
+#include <poll.h>
+#include <sys/types.h>
+#include <sys/syscall.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include "tst_test.h"
+#include "lapi/pidfd_open.h"
+#include "lapi/syscalls.h"
+
+static void run(void)
+{
+	struct pollfd pollfd;
+	int p_id, fd, ready;
+
+	TEST(fork());
+
+	if (TST_RET == -1) {
+		tst_res(TFAIL, "fork() Failed");
+		tst_res(TBROK, "unable to continue");
+	}
+
+	if (TST_RET == 0) {
+		/* child */
+		usleep(1000);
+		exit(EXIT_SUCCESS);
+	} else {
+		/* parent */
+		p_id = TST_RET;
+
+		TEST(pidfd_open(p_id, 0));
+
+		fd = TST_RET;
+		if (fd == -1) {
+			tst_res(TFAIL, "Cannot retrieve file descriptor to the child process");
+			return;
+		}
+
+		pollfd.fd = fd;
+		pollfd.events = POLLIN;
+
+		ready = poll(&pollfd, 1, -1);
+		if (ready == -1) {
+			tst_res(TFAIL, "poll() Failed");
+			tst_res(TBROK, "unable to continue");
+		}
+
+		printf("Events (0x%x): POLLIN is %sset\n", pollfd.revents,
+				(pollfd.revents & POLLIN) ? "" : "not ");
+
+		SAFE_CLOSE(fd);
+	}
+}
+
+static struct tst_test test = {
+	.min_kver = "5.3",
+	.test_all = run,
+};
-- 
2.21.0.rc0.269.g1a574e7a288b

[LTP] [PATCH 2/2] syscalls/pidfd_open

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> new file mode 100644
> index 000000000000..b67393bcafa2
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * Description:
> + * This program opens the PID file descriptor of the child process created with
> + * fork(). It then uses poll to monitor the file descriptor for process exit, as
> + * indicated by an EPOLLIN event.
> + */
> +#include <poll.h>
> +#include <sys/types.h>
> +#include <sys/syscall.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +#include "tst_test.h"
> +#include "lapi/pidfd_open.h"
> +#include "lapi/syscalls.h"
> +
> +static void run(void)
> +{
> +	struct pollfd pollfd;
> +	int p_id, fd, ready;
> +
> +	TEST(fork());
> +
> +	if (TST_RET == -1) {
> +		tst_res(TFAIL, "fork() Failed");
> +		tst_res(TBROK, "unable to continue");
> +	}

We do have SAFE_FORK() make use of that one instead.


> +	if (TST_RET == 0) {
> +		/* child */
> +		usleep(1000);

This is inherently racy, the process should not exit until the parent
has called pidfd_open().

We do have a checkpoints in the test library that can synchronize
between processes, generally it should look like:

child:
	checkpoint_wait()
	exit()

parent:
	pidfd_open()
	checkpoint_wake()

> +		exit(EXIT_SUCCESS);
> +	} else {
> +		/* parent */
> +		p_id = TST_RET;
> +
> +		TEST(pidfd_open(p_id, 0));
> +
> +		fd = TST_RET;
> +		if (fd == -1) {
> +			tst_res(TFAIL, "Cannot retrieve file descriptor to the child process");
                                  ^
			tst_res(TFAIL | TTERRNO, "pidfd_open() failed");
> +			return;
> +		}
> +
> +		pollfd.fd = fd;
> +		pollfd.events = POLLIN;
> +
> +		ready = poll(&pollfd, 1, -1);
> +		if (ready == -1) {
> +			tst_res(TFAIL, "poll() Failed");
> +			tst_res(TBROK, "unable to continue");

There is absolutely no need to print two messages on a failure, so this
should just be tst_brk(TBROK | TERRNO, "poll() failed");

Also note that tst_brk() _EXITS_ the test, which is what you want here.

> +		}
> +
> +		printf("Events (0x%x): POLLIN is %sset\n", pollfd.revents,
> +				(pollfd.revents & POLLIN) ? "" : "not ");

No printf in tests, use tst_res(TINFO, ...) instead.

Also we should check here that we got the event and that the child is
dead at this point, or something along these lines.

There does not seem to be single TPASS in this tests, which itself will
report that the test is broken when it's executed.

> +		SAFE_CLOSE(fd);
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.min_kver = "5.3",
> +	.test_all = run,
> +};

And we are also missing third test here, that checks various error
condigitions such as flags != 0, invalid pid, etc.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] syscalls/pidfd_open

[Viresh Kumar]

On 21-01-20, 16:39, Cyril Hrubis wrote:
> And we are also missing third test here, that checks various error
> condigitions such as flags != 0, invalid pid, etc.

Hi Cyril,

Thanks for the feedback and sorry about all the mistakes as it was my
very first attempt at running/updating ltp code :)

Please have a look at the updated patch pasted below and let me know
if anything is still missing.

-- 
viresh

-------------------------8<-------------------------
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Thu, 16 Jan 2020 16:47:01 +0530
Subject: [PATCH] syscalls/pidfd_open

Add tests to check working of pidfd_open() syscall.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 configure.ac                                  |  1 +
 include/lapi/pidfd_open.h                     | 21 ++++++
 runtest/syscalls                              |  3 +
 .../kernel/syscalls/pidfd_open/.gitignore     |  2 +
 testcases/kernel/syscalls/pidfd_open/Makefile |  6 ++
 .../kernel/syscalls/pidfd_open/pidfd_open01.c | 38 +++++++++++
 .../kernel/syscalls/pidfd_open/pidfd_open02.c | 45 +++++++++++++
 .../kernel/syscalls/pidfd_open/pidfd_open03.c | 64 +++++++++++++++++++
 8 files changed, 180 insertions(+)
 create mode 100644 include/lapi/pidfd_open.h
 create mode 100644 testcases/kernel/syscalls/pidfd_open/.gitignore
 create mode 100644 testcases/kernel/syscalls/pidfd_open/Makefile
 create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
 create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
 create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open03.c

diff --git a/configure.ac b/configure.ac
index 50d14967d3c6..1bf0911d88ad 100644
--- a/configure.ac
+++ b/configure.ac
@@ -79,6 +79,7 @@ AC_CHECK_FUNCS([ \
     mknodat \
     name_to_handle_at \
     openat \
+    pidfd_open \
     pidfd_send_signal \
     pkey_mprotect \
     preadv \
diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h
new file mode 100644
index 000000000000..ced163be83bf
--- /dev/null
+++ b/include/lapi/pidfd_open.h
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Linaro Limited. All rights reserved.
+ * Author: Viresh Kumar <viresh.kumar@linaro.org>
+ */
+
+#ifndef PIDFD_OPEN_H
+#define PIDFD_OPEN_H
+
+#include "config.h"
+#include <sys/types.h>
+#include "lapi/syscalls.h"
+
+#ifndef HAVE_PIDFD_OPEN
+int pidfd_open(pid_t pid, unsigned int flags)
+{
+	return tst_syscall(__NR_pidfd_open, pid, flags);
+}
+#endif
+
+#endif /* PIDFD_OPEN_H */
diff --git a/runtest/syscalls b/runtest/syscalls
index fa87ef63fbc1..9d6d288780a3 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -846,6 +846,9 @@ pause03 pause03
 personality01 personality01
 personality02 personality02
 
+pidfd_open01 pidfd_open01
+pidfd_open02 pidfd_open02
+
 pidfd_send_signal01 pidfd_send_signal01
 pidfd_send_signal02 pidfd_send_signal02
 pidfd_send_signal03 pidfd_send_signal03
diff --git a/testcases/kernel/syscalls/pidfd_open/.gitignore b/testcases/kernel/syscalls/pidfd_open/.gitignore
new file mode 100644
index 000000000000..be218f88647d
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/.gitignore
@@ -0,0 +1,2 @@
+pidfd_open01
+pidfd_open02
diff --git a/testcases/kernel/syscalls/pidfd_open/Makefile b/testcases/kernel/syscalls/pidfd_open/Makefile
new file mode 100644
index 000000000000..5ea7d67db123
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir		?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
new file mode 100644
index 000000000000..2ca22ae3fb92
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Description:
+ * Basic pidfd_open() test, fetches the PID of the current process and tries to
+ * get its file descriptor.
+ */
+#include <sys/types.h>
+#include <sys/syscall.h>
+
+#include "tst_test.h"
+#include "lapi/pidfd_open.h"
+#include "lapi/syscalls.h"
+
+static void run(void)
+{
+	int pid, fd;
+
+	pid = getpid();
+
+	TEST(pidfd_open(pid, 0));
+
+	fd = TST_RET;
+	if (fd == -1) {
+		tst_res(TFAIL, "Cannot retrieve file descriptor to the current process");
+		return;
+	}
+
+	SAFE_CLOSE(fd);
+
+	tst_res(TPASS, "Retrieved file descriptor to the current process");
+}
+
+static struct tst_test test = {
+	.min_kver = "5.3",
+	.test_all = run,
+};
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
new file mode 100644
index 000000000000..ab2f86a31392
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Description:
+ * Basic pidfd_open() test to test invalid arguments.
+ */
+#include <sys/types.h>
+#include <sys/syscall.h>
+
+#include "tst_test.h"
+#include "lapi/pidfd_open.h"
+#include "lapi/syscalls.h"
+
+static void run(void)
+{
+	int pid, fd;
+
+	/* Invalid pid */
+	pid = -1;
+
+	fd = pidfd_open(pid, 0);
+	if (fd != -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TFAIL, "pidfd_open() didn't recognize invalid pid");
+		return;
+	}
+
+	pid = getpid();
+
+	/* Invalid flags */
+	fd = pidfd_open(pid, 1);
+	if (fd != -1) {
+		SAFE_CLOSE(fd);
+		tst_res(TFAIL, "pidfd_open() didn't recognize invalid flags");
+		return;
+	}
+
+	tst_res(TPASS, "pidfd_open() failed for invalid arguments");
+}
+
+static struct tst_test test = {
+	.min_kver = "5.3",
+	.test_all = run,
+};
diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
new file mode 100644
index 000000000000..56861dfc014a
--- /dev/null
+++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
+ *
+ * Description:
+ * This program opens the PID file descriptor of the child process created with
+ * fork(). It then uses poll to monitor the file descriptor for process exit, as
+ * indicated by an EPOLLIN event.
+ */
+#include <poll.h>
+#include <sys/types.h>
+#include <sys/syscall.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "tst_test.h"
+#include "lapi/pidfd_open.h"
+#include "lapi/syscalls.h"
+
+static void run(void)
+{
+	struct pollfd pollfd;
+	int pid, fd, ready;
+
+	pid = SAFE_FORK();
+
+	if (!pid) {
+		/* child */
+		TST_CHECKPOINT_WAIT(0);
+		exit(EXIT_SUCCESS);
+	} else {
+		/* parent */
+		TEST(pidfd_open(pid, 0));
+
+		fd = TST_RET;
+		if (fd == -1) {
+			tst_res(TFAIL | TTERRNO, "pidfd_open() failed");
+			return;
+		}
+
+		TST_CHECKPOINT_WAKE(0);
+
+		pollfd.fd = fd;
+		pollfd.events = POLLIN;
+
+		ready = poll(&pollfd, 1, -1);
+
+		SAFE_CLOSE(fd);
+		SAFE_WAITPID(pid, NULL, 0);
+
+		if (ready == -1)
+			tst_brk(TBROK | TERRNO, "poll() failed");
+
+		tst_res(TPASS, "Events (0x%x): POLLIN is %sset\n",
+			pollfd.revents, (pollfd.revents & POLLIN) ? "" : "not");
+	}
+}
+
+static struct tst_test test = {
+	.min_kver = "5.3",
+	.test_all = run,
+	.forks_child = 1,
+	.needs_checkpoints = 1,
+};

[LTP] [PATCH 2/2] syscalls/pidfd_open

[Viresh Kumar]

On 22-01-20, 10:42, Viresh Kumar wrote:
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Date: Thu, 16 Jan 2020 16:47:01 +0530
> Subject: [PATCH] syscalls/pidfd_open
> 
> Add tests to check working of pidfd_open() syscall.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  configure.ac                                  |  1 +
>  include/lapi/pidfd_open.h                     | 21 ++++++
>  runtest/syscalls                              |  3 +
>  .../kernel/syscalls/pidfd_open/.gitignore     |  2 +
>  testcases/kernel/syscalls/pidfd_open/Makefile |  6 ++
>  .../kernel/syscalls/pidfd_open/pidfd_open01.c | 38 +++++++++++
>  .../kernel/syscalls/pidfd_open/pidfd_open02.c | 45 +++++++++++++
>  .../kernel/syscalls/pidfd_open/pidfd_open03.c | 64 +++++++++++++++++++
>  8 files changed, 180 insertions(+)
>  create mode 100644 include/lapi/pidfd_open.h
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/.gitignore
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/Makefile
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open03.c

+ this as well (I will resent the patches properly again after you
have had a chance to look at them once).

diff --git a/runtest/syscalls b/runtest/syscalls
index 9d6d288780a3..a2d749d526a8 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -848,6 +848,7 @@ personality02 personality02
 
 pidfd_open01 pidfd_open01
 pidfd_open02 pidfd_open02
+pidfd_open03 pidfd_open03
 
 pidfd_send_signal01 pidfd_send_signal01
 pidfd_send_signal02 pidfd_send_signal02
diff --git a/testcases/kernel/syscalls/pidfd_open/.gitignore b/testcases/kernel/syscalls/pidfd_open/.gitignore
index be218f88647d..e0b8900c1c33 100644
--- a/testcases/kernel/syscalls/pidfd_open/.gitignore
+++ b/testcases/kernel/syscalls/pidfd_open/.gitignore
@@ -1,2 +1,3 @@
 pidfd_open01
 pidfd_open02
+pidfd_open03

-- 
viresh

[LTP] [PATCH 2/2] syscalls/pidfd_open

[Cyril Hrubis]

Hi!
> > And we are also missing third test here, that checks various error
> > condigitions such as flags != 0, invalid pid, etc.
> 
> Hi Cyril,
> 
> Thanks for the feedback and sorry about all the mistakes as it was my
> very first attempt at running/updating ltp code :)

It takes time to learn... Also welcome :-).

> Please have a look at the updated patch pasted below and let me know
> if anything is still missing.

It's usuall to send updated patches as new version, i.e. this should be
called [PATCH v2] and just send to the ML as the previous one. So the
next one should be [PATCH v3] ...

> -------------------------8<-------------------------
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Date: Thu, 16 Jan 2020 16:47:01 +0530
> Subject: [PATCH] syscalls/pidfd_open
> 
> Add tests to check working of pidfd_open() syscall.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  configure.ac                                  |  1 +
>  include/lapi/pidfd_open.h                     | 21 ++++++
>  runtest/syscalls                              |  3 +
>  .../kernel/syscalls/pidfd_open/.gitignore     |  2 +
>  testcases/kernel/syscalls/pidfd_open/Makefile |  6 ++
>  .../kernel/syscalls/pidfd_open/pidfd_open01.c | 38 +++++++++++
>  .../kernel/syscalls/pidfd_open/pidfd_open02.c | 45 +++++++++++++
>  .../kernel/syscalls/pidfd_open/pidfd_open03.c | 64 +++++++++++++++++++
>  8 files changed, 180 insertions(+)
>  create mode 100644 include/lapi/pidfd_open.h
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/.gitignore
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/Makefile
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
>  create mode 100644 testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> 
> diff --git a/configure.ac b/configure.ac
> index 50d14967d3c6..1bf0911d88ad 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -79,6 +79,7 @@ AC_CHECK_FUNCS([ \
>      mknodat \
>      name_to_handle_at \
>      openat \
> +    pidfd_open \
>      pidfd_send_signal \
>      pkey_mprotect \
>      preadv \
> diff --git a/include/lapi/pidfd_open.h b/include/lapi/pidfd_open.h
> new file mode 100644
> index 000000000000..ced163be83bf
> --- /dev/null
> +++ b/include/lapi/pidfd_open.h
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Linaro Limited. All rights reserved.
> + * Author: Viresh Kumar <viresh.kumar@linaro.org>
> + */
> +
> +#ifndef PIDFD_OPEN_H
> +#define PIDFD_OPEN_H
> +
> +#include "config.h"
> +#include <sys/types.h>
> +#include "lapi/syscalls.h"
> +
> +#ifndef HAVE_PIDFD_OPEN
> +int pidfd_open(pid_t pid, unsigned int flags)
> +{
> +	return tst_syscall(__NR_pidfd_open, pid, flags);
> +}
> +#endif
> +
> +#endif /* PIDFD_OPEN_H */
> diff --git a/runtest/syscalls b/runtest/syscalls
> index fa87ef63fbc1..9d6d288780a3 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -846,6 +846,9 @@ pause03 pause03
>  personality01 personality01
>  personality02 personality02
>  
> +pidfd_open01 pidfd_open01
> +pidfd_open02 pidfd_open02
> +
>  pidfd_send_signal01 pidfd_send_signal01
>  pidfd_send_signal02 pidfd_send_signal02
>  pidfd_send_signal03 pidfd_send_signal03
> diff --git a/testcases/kernel/syscalls/pidfd_open/.gitignore b/testcases/kernel/syscalls/pidfd_open/.gitignore
> new file mode 100644
> index 000000000000..be218f88647d
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_open/.gitignore
> @@ -0,0 +1,2 @@
> +pidfd_open01
> +pidfd_open02

These two files now miss pidfd_open03

> diff --git a/testcases/kernel/syscalls/pidfd_open/Makefile b/testcases/kernel/syscalls/pidfd_open/Makefile
> new file mode 100644
> index 000000000000..5ea7d67db123
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_open/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir		?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> new file mode 100644
> index 000000000000..2ca22ae3fb92
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open01.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * Description:
> + * Basic pidfd_open() test, fetches the PID of the current process and tries to
> + * get its file descriptor.
> + */
> +#include <sys/types.h>
> +#include <sys/syscall.h>
> +
> +#include "tst_test.h"
> +#include "lapi/pidfd_open.h"
> +#include "lapi/syscalls.h"
> +
> +static void run(void)
> +{
> +	int pid, fd;
> +
> +	pid = getpid();
> +
> +	TEST(pidfd_open(pid, 0));
> +
> +	fd = TST_RET;
> +	if (fd == -1) {
> +		tst_res(TFAIL, "Cannot retrieve file descriptor to the current process");
                        ^
			This is still missing the | TTERRNO bitflag so
			that the message prints errno as well

Also the whole message could be shorter and to the point:

"pidfd_open(getpid(), 0) failed"

> +		return;
> +	}
> +
> +	SAFE_CLOSE(fd);
> +
> +	tst_res(TPASS, "Retrieved file descriptor to the current process");

Here as well.

> +}
> +
> +static struct tst_test test = {
> +	.min_kver = "5.3",
> +	.test_all = run,
> +};
> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> new file mode 100644
> index 000000000000..ab2f86a31392
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open02.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * Description:
> + * Basic pidfd_open() test to test invalid arguments.
> + */
> +#include <sys/types.h>
> +#include <sys/syscall.h>
> +
> +#include "tst_test.h"
> +#include "lapi/pidfd_open.h"
> +#include "lapi/syscalls.h"
> +
> +static void run(void)
> +{
> +	int pid, fd;
> +
> +	/* Invalid pid */
> +	pid = -1;
> +
> +	fd = pidfd_open(pid, 0);
> +	if (fd != -1) {

We have to check if the errno is correct here as well, also we usually
use the TEST() macro that saves return value and errno at the same time.

> +		SAFE_CLOSE(fd);
> +		tst_res(TFAIL, "pidfd_open() didn't recognize invalid pid");
> +		return;
> +	}
> +
> +	pid = getpid();
> +
> +	/* Invalid flags */
> +	fd = pidfd_open(pid, 1);
> +	if (fd != -1) {
> +		SAFE_CLOSE(fd);
> +		tst_res(TFAIL, "pidfd_open() didn't recognize invalid flags");
> +		return;
> +	}
> +
> +	tst_res(TPASS, "pidfd_open() failed for invalid arguments");
> +}
> +
> +static struct tst_test test = {
> +	.min_kver = "5.3",
> +	.test_all = run,
> +};

These kid of tests are usually written in a way that the parameters are
stored in a structure and the test function is called with increasing
index to loop over all tests. Have a look at open/open02.c for example.

But looking at the pidfd_open() manual, the only error that is
missing here and reasonable to test here is ESRCH.

	Hint: see tst_get_unused_pid()

> diff --git a/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> new file mode 100644
> index 000000000000..56861dfc014a
> --- /dev/null
> +++ b/testcases/kernel/syscalls/pidfd_open/pidfd_open03.c
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2020 Viresh Kumar <viresh.kumar@linaro.org>
> + *
> + * Description:
> + * This program opens the PID file descriptor of the child process created with
> + * fork(). It then uses poll to monitor the file descriptor for process exit, as
> + * indicated by an EPOLLIN event.
> + */
> +#include <poll.h>
> +#include <sys/types.h>
> +#include <sys/syscall.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include "tst_test.h"
> +#include "lapi/pidfd_open.h"
> +#include "lapi/syscalls.h"
> +
> +static void run(void)
> +{
> +	struct pollfd pollfd;
> +	int pid, fd, ready;
> +
> +	pid = SAFE_FORK();
> +
> +	if (!pid) {
> +		/* child */
> +		TST_CHECKPOINT_WAIT(0);
> +		exit(EXIT_SUCCESS);
> +	} else {
> +		/* parent */

These /* parent */ and /* child */ comments fall into "commenting the
obvious" categrogy please avoid doing so.

> +		TEST(pidfd_open(pid, 0));
> +
> +		fd = TST_RET;
> +		if (fd == -1) {
> +			tst_res(TFAIL | TTERRNO, "pidfd_open() failed");
> +			return;
> +		}
> +
> +		TST_CHECKPOINT_WAKE(0);
> +
> +		pollfd.fd = fd;
> +		pollfd.events = POLLIN;
> +
> +		ready = poll(&pollfd, 1, -1);
> +
> +		SAFE_CLOSE(fd);
> +		SAFE_WAITPID(pid, NULL, 0);
> +
> +		if (ready == -1)
> +			tst_brk(TBROK | TERRNO, "poll() failed");

I guess that we may as well check that the ready is exactly 1 here as
well, but that's a minor thing.

> +		tst_res(TPASS, "Events (0x%x): POLLIN is %sset\n",
> +			pollfd.revents, (pollfd.revents & POLLIN) ? "" : "not");
> +	}
> +}
> +
> +static struct tst_test test = {
> +	.min_kver = "5.3",
> +	.test_all = run,
> +	.forks_child = 1,
> +	.needs_checkpoints = 1,
> +};

-- 
Cyril Hrubis
chrubis@suse.cz