[PATCH v3 0/6] Update clone3 self-tests

[PATCH v3 0/6] Update clone3 self-tests

[Eugene Syromiatnikov]

Hello.

This patch set updates clone3 selftest in several aspects:
 - adding checks for exit_signal invalid values handling;
 - adding clone3 to selftests targets;
 - enabling clone3 tests on all architectures;
 - minor cleanups of the clone3 test.

This respin alignes additional clone3 self-tests with v3 of the
exit_signal checking patch[1].

Applied on top of brauer/linux.git/for-next.

Changes since v2[2]:
 - CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG check is now expected to fail.

Changes since v1[3]:
 - exit_signal check extended to cover more cases of invalid
   exit_signal value.

[1] https://lkml.org/lkml/2019/9/11/677
[2] https://lkml.org/lkml/2019/9/10/768
[3] https://lkml.org/lkml/2019/9/10/416

Eugene Syromiatnikov (6):
  selftests/clone3: convert test modes into an enum
  selftests/clone3: add a check for invalid exit_signal
  selftests/clone3: use uint64_t for flags parameter
  selftests/clone3: fix up format strings
  selftests/clone3: enable clone3 self-tests on all architectures
  selftests: add clone3 to TARGETS

 tools/testing/selftests/Makefile        |  1 +
 tools/testing/selftests/clone3/Makefile |  4 +--
 tools/testing/selftests/clone3/clone3.c | 64 ++++++++++++++++++++++++++++-----
 3 files changed, 57 insertions(+), 12 deletions(-)

-- 
2.1.4

[PATCH v3 1/6] selftests/clone3: convert test modes into an enum

[Eugene Syromiatnikov]

* tools/testing/selftests/clone3/clone3.c (CLONE3_ARGS_NO_TEST,
CLONE3_ARGS_ALL_0, CLONE3_ARGS_ALL_1): Change into an enum.
(call_clone3): Change test_mode parameter type to enum test_mode;
use switch statement for actions that dependent on test_mode selection.
(test_clone3): Change test_mode parameter type to enum test_mode.

Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 tools/testing/selftests/clone3/clone3.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c
index a0f1989..7b65ee5 100644
--- a/tools/testing/selftests/clone3/clone3.c
+++ b/tools/testing/selftests/clone3/clone3.c
@@ -24,16 +24,18 @@
 /* V1 includes set_tid */
 #define CLONE3_ARGS_SIZE_V1 72
 
-#define CLONE3_ARGS_NO_TEST 0
-#define CLONE3_ARGS_ALL_0 1
-#define CLONE3_ARGS_ALL_1 2
+enum test_mode {
+	CLONE3_ARGS_NO_TEST,
+	CLONE3_ARGS_ALL_0,
+	CLONE3_ARGS_ALL_1,
+};
 
 static pid_t raw_clone(struct clone_args *args, size_t size)
 {
 	return syscall(__NR_clone3, args, size);
 }
 
-static int call_clone3(int flags, size_t size, int test_mode)
+static int call_clone3(int flags, size_t size, enum test_mode test_mode)
 {
 	struct clone_args args = {0};
 	pid_t ppid = -1;
@@ -46,7 +48,8 @@ static int call_clone3(int flags, size_t size, int test_mode)
 	if (size == 0)
 		size = sizeof(struct clone_args);
 
-	if (test_mode == CLONE3_ARGS_ALL_0) {
+	switch (test_mode) {
+	case CLONE3_ARGS_ALL_0:
 		args.flags = 0;
 		args.pidfd = 0;
 		args.child_tid = 0;
@@ -56,7 +59,9 @@ static int call_clone3(int flags, size_t size, int test_mode)
 		args. stack_size = 0;
 		args.tls = 0;
 		args.set_tid = 0;
-	} else if (test_mode == CLONE3_ARGS_ALL_1) {
+		break;
+
+	case CLONE3_ARGS_ALL_1:
 		args.flags = 1;
 		args.pidfd = 1;
 		args.child_tid = 1;
@@ -66,6 +71,7 @@ static int call_clone3(int flags, size_t size, int test_mode)
 		args. stack_size = 1;
 		args.tls = 1;
 		args.set_tid = 1;
+		break;
 	}
 
 	pid = raw_clone(&args, size);
@@ -91,7 +97,8 @@ static int call_clone3(int flags, size_t size, int test_mode)
 	return 0;
 }
 
-static int test_clone3(int flags, size_t size, int expected, int test_mode)
+static int test_clone3(int flags, size_t size, int expected,
+		       enum test_mode test_mode)
 {
 	int ret;
 
-- 
2.1.4

[PATCH v3 2/6] selftests/clone3: add a check for invalid exit_signal

[Eugene Syromiatnikov]

Check that the kernel fails calls with exit_signal with non-zero highest
32 bits, negative 32-bit exit_signal, and invalid exit_signal withing
CSIGNAL mask, like legacy clone syscalls do.

* tools/testing/selftests/clone3/clone3.c (enum test_mode): Add
CLONE3_ARGS_INVAL_EXIT_SIGNAL_BIG, CLONE3_ARGS_INVAL_EXIT_SIGNAL_NEG,
CLONE3_ARGS_INVAL_EXIT_SIGNAL_CSIG, CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG.
(call_clone3): Add args.exit_signal initialisation in case test_mode
is equal to one of the added enum test_mode values.
(main): Add test_clone3 clone check with test_mode equal to the added
enum test_mode values.

Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 tools/testing/selftests/clone3/clone3.c | 36 +++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c
index 7b65ee5..4837865 100644
--- a/tools/testing/selftests/clone3/clone3.c
+++ b/tools/testing/selftests/clone3/clone3.c
@@ -28,6 +28,10 @@ enum test_mode {
 	CLONE3_ARGS_NO_TEST,
 	CLONE3_ARGS_ALL_0,
 	CLONE3_ARGS_ALL_1,
+	CLONE3_ARGS_INVAL_EXIT_SIGNAL_BIG,
+	CLONE3_ARGS_INVAL_EXIT_SIGNAL_NEG,
+	CLONE3_ARGS_INVAL_EXIT_SIGNAL_CSIG,
+	CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG,
 };
 
 static pid_t raw_clone(struct clone_args *args, size_t size)
@@ -72,6 +76,22 @@ static int call_clone3(int flags, size_t size, enum test_mode test_mode)
 		args.tls = 1;
 		args.set_tid = 1;
 		break;
+
+	case CLONE3_ARGS_INVAL_EXIT_SIGNAL_BIG:
+		args.exit_signal = 0xbadc0ded00000000ULL;
+		break;
+
+	case CLONE3_ARGS_INVAL_EXIT_SIGNAL_NEG:
+		args.exit_signal = 0x0000000080000000ULL;
+		break;
+
+	case CLONE3_ARGS_INVAL_EXIT_SIGNAL_CSIG:
+		args.exit_signal = 0x0000000000000100ULL;
+		break;
+
+	case CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG:
+		args.exit_signal = 0x00000000000000f0ULL;
+		break;
 	}
 
 	pid = raw_clone(&args, size);
@@ -146,6 +166,22 @@ int main(int argc, char *argv[])
 	/* Do a clone3() with all members set to 1 */
 	if (test_clone3(0, CLONE3_ARGS_SIZE_V0, -EINVAL, CLONE3_ARGS_ALL_1))
 		goto on_error;
+	/* Do a clone3() with exit_signal having highest 32 bits non-zero */
+	if (test_clone3(0, CLONE3_ARGS_SIZE_V0, -EINVAL,
+			CLONE3_ARGS_INVAL_EXIT_SIGNAL_BIG))
+		goto on_error;
+	/* Do a clone3() with negative 32-bit exit_signal */
+	if (test_clone3(0, CLONE3_ARGS_SIZE_V0, -EINVAL,
+			CLONE3_ARGS_INVAL_EXIT_SIGNAL_NEG))
+		goto on_error;
+	/* Do a clone3() with exit_signal not fitting into CSIGNAL mask */
+	if (test_clone3(0, CLONE3_ARGS_SIZE_V0, -EINVAL,
+			CLONE3_ARGS_INVAL_EXIT_SIGNAL_CSIG))
+		goto on_error;
+	/* Do a clone3() with NSIG < exit_signal < CSIG */
+	if (test_clone3(0, CLONE3_ARGS_SIZE_V0, -EINVAL,
+			CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG))
+		goto on_error;
 	/*
 	 * Do a clone3() with sizeof(struct clone_args) + 8
 	 * and all members set to 0.
-- 
2.1.4

[PATCH v3 3/6] selftests/clone3: use uint64_t for flags parameter

[Eugene Syromiatnikov]

Flags parameter in both userspace and kernel clone args is 64-bit wide,
there's little reason to have it signed and 32-bit in tests.

* tools/testing/selftests/clone3/clone3.c: Include <inttypes.h> and
<stdint.h>.
(call_clone3): Change flags parameter type from int to uint64_t.
(test_clone3): Change flags parameter type from int to uint64_t; change
the format string that prints it accordingly.

Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 tools/testing/selftests/clone3/clone3.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c
index 4837865..264d03a 100644
--- a/tools/testing/selftests/clone3/clone3.c
+++ b/tools/testing/selftests/clone3/clone3.c
@@ -4,8 +4,10 @@
 
 #define _GNU_SOURCE
 #include <errno.h>
+#include <inttypes.h>
 #include <linux/types.h>
 #include <linux/sched.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/syscall.h>
@@ -39,7 +41,7 @@ static pid_t raw_clone(struct clone_args *args, size_t size)
 	return syscall(__NR_clone3, args, size);
 }
 
-static int call_clone3(int flags, size_t size, enum test_mode test_mode)
+static int call_clone3(uint64_t flags, size_t size, enum test_mode test_mode)
 {
 	struct clone_args args = {0};
 	pid_t ppid = -1;
@@ -117,12 +119,13 @@ static int call_clone3(int flags, size_t size, enum test_mode test_mode)
 	return 0;
 }
 
-static int test_clone3(int flags, size_t size, int expected,
+static int test_clone3(uint64_t flags, size_t size, int expected,
 		       enum test_mode test_mode)
 {
 	int ret;
 
-	ksft_print_msg("[%d] Trying clone3() with flags 0x%x (size %d)\n",
+	ksft_print_msg("[%d] Trying clone3() with flags %#" PRIx64 " (size %d)"
+			"\n",
 			getpid(), flags, size);
 	ret = call_clone3(flags, size, test_mode);
 	ksft_print_msg("[%d] clone3() with flags says :%d expected %d\n",
-- 
2.1.4

[PATCH v3 4/6] selftests/clone3: fix up format strings

[Eugene Syromiatnikov]

* tools/testing/selftests/clone3/clone3.c (test_clone3): Change format
qualifier for printing size field from %d to %zu; place colon right
after the word "says".

Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 tools/testing/selftests/clone3/clone3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c
index 264d03a..86f888b 100644
--- a/tools/testing/selftests/clone3/clone3.c
+++ b/tools/testing/selftests/clone3/clone3.c
@@ -124,11 +124,11 @@ static int test_clone3(uint64_t flags, size_t size, int expected,
 {
 	int ret;
 
-	ksft_print_msg("[%d] Trying clone3() with flags %#" PRIx64 " (size %d)"
+	ksft_print_msg("[%d] Trying clone3() with flags %#" PRIx64 " (size %zu)"
 			"\n",
 			getpid(), flags, size);
 	ret = call_clone3(flags, size, test_mode);
-	ksft_print_msg("[%d] clone3() with flags says :%d expected %d\n",
+	ksft_print_msg("[%d] clone3() with flags says: %d expected %d\n",
 			getpid(), ret, expected);
 	if (ret != expected)
 		ksft_exit_fail_msg(
-- 
2.1.4

[PATCH v3 5/6] selftests/clone3: enable clone3 self-tests on all architectures

[Eugene Syromiatnikov]

clone3() is available on most architectures, so there's no reason to
restrict the respective self-tests to x86_64.

* tools/testing/selftests/clone3/Makefile (TEST_GEN_PROGS): Set always,
not only ifeq ($(ARCH),x86_64).

Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 tools/testing/selftests/clone3/Makefile | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/testing/selftests/clone3/Makefile b/tools/testing/selftests/clone3/Makefile
index 4efcf45..faa069c 100644
--- a/tools/testing/selftests/clone3/Makefile
+++ b/tools/testing/selftests/clone3/Makefile
@@ -4,8 +4,6 @@ ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/)
 
 CFLAGS += -I../../../../usr/include/
 
-ifeq ($(ARCH),x86_64)
-	TEST_GEN_PROGS := clone3 clone3_set_tid
-endif
+TEST_GEN_PROGS := clone3 clone3_set_tid
 
 include ../lib.mk
-- 
2.1.4

[PATCH v3 6/6] selftests: add clone3 to TARGETS

[Eugene Syromiatnikov]

* tools/testing/selftests/Makefile (TARGETS): Add clone3.

Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 tools/testing/selftests/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 25b43a8c..05163e4 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -4,6 +4,7 @@ TARGETS += bpf
 TARGETS += breakpoints
 TARGETS += capabilities
 TARGETS += cgroup
+TARGETS += clone3
 TARGETS += cpufreq
 TARGETS += cpu-hotplug
 TARGETS += drivers/dma-buf
-- 
2.1.4

Re: [PATCH v3 0/6] Update clone3 self-tests

[Anders Roxell]

On Wed, 11 Sep 2019 at 20:02, Eugene Syromiatnikov <esyr@redhat.com> wrote:
>
> Hello.
>
> This patch set updates clone3 selftest in several aspects:
>  - adding checks for exit_signal invalid values handling;
>  - adding clone3 to selftests targets;
>  - enabling clone3 tests on all architectures;
>  - minor cleanups of the clone3 test.
>
> This respin alignes additional clone3 self-tests with v3 of the
> exit_signal checking patch[1].
>
> Applied on top of brauer/linux.git/for-next.
>
> Changes since v2[2]:
>  - CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG check is now expected to fail.
>
> Changes since v1[3]:
>  - exit_signal check extended to cover more cases of invalid
>    exit_signal value.
>
> [1] https://lkml.org/lkml/2019/9/11/677
> [2] https://lkml.org/lkml/2019/9/10/768
> [3] https://lkml.org/lkml/2019/9/10/416
>
> Eugene Syromiatnikov (6):
>   selftests/clone3: convert test modes into an enum
>   selftests/clone3: add a check for invalid exit_signal
>   selftests/clone3: use uint64_t for flags parameter
>   selftests/clone3: fix up format strings
>   selftests/clone3: enable clone3 self-tests on all architectures
>   selftests: add clone3 to TARGETS

I wasn't able to build this patchset for arm64, I applied it on tag
next-20190904:

$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -skj$(getconf
_NPROCESSORS_ONLN) headers_install
$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -skj$(getconf
_NPROCESSORS_ONLN) -C tools/testing/selftests/clone3

clone3_set_tid.c: In function ‘raw_clone’:
clone3_set_tid.c:22:17: error: ‘__NR_clone3’ undeclared (first use in
this function); did you mean ‘raw_clone’?
  return syscall(__NR_clone3, args, sizeof(struct clone_args));
                 ^~~~~~~~~~~
                 raw_clone
clone3_set_tid.c:22:17: note: each undeclared identifier is reported
only once for each function it appears in
make: *** [../lib.mk:138:
/srv/src/kernel/kselftest-testing/tools/testing/selftests/clone3/clone3_set_tid]
Error 1
clone3.c: In function ‘raw_clone’:
clone3.c:41:17: error: ‘__NR_clone3’ undeclared (first use in this
function); did you mean ‘raw_clone’?
  return syscall(__NR_clone3, args, size);
                 ^~~~~~~~~~~
                 raw_clone
clone3.c:41:17: note: each undeclared identifier is reported only once
for each function it appears in
make: *** [../lib.mk:138:
/srv/src/kernel/kselftest-testing/tools/testing/selftests/clone3/clone3]
Error 1
make: Target 'all' not remade because of errors.


Any idea what I'm doing wrong?

Cheers,
Anders

>
>  tools/testing/selftests/Makefile        |  1 +
>  tools/testing/selftests/clone3/Makefile |  4 +--
>  tools/testing/selftests/clone3/clone3.c | 64 ++++++++++++++++++++++++++++-----
>  3 files changed, 57 insertions(+), 12 deletions(-)
>
> --
> 2.1.4
>

Re: [PATCH v3 0/6] Update clone3 self-tests

[Christian Brauner]

On Wed, Sep 11, 2019 at 07:01:54PM +0100, Eugene Syromiatnikov wrote:
> Hello.
> 
> This patch set updates clone3 selftest in several aspects:
>  - adding checks for exit_signal invalid values handling;
>  - adding clone3 to selftests targets;
>  - enabling clone3 tests on all architectures;
>  - minor cleanups of the clone3 test.
> 
> This respin alignes additional clone3 self-tests with v3 of the
> exit_signal checking patch[1].
> 
> Applied on top of brauer/linux.git/for-next.

Just base it on v5.3. That should be easier for everyone. :)

Christian