Messages in this thread | | | Date | Sun, 11 Aug 2019 14:37:03 -0700 | Subject | Re: [PATCH v5 2/2] selftests: add tests for clone3() | From | Christian Brauner <> |
| |
On August 11, 2019 1:33:27 PM PDT, Adrian Reber <areber@redhat.com> wrote: >This tests clone3() with and without set_tid to see if all desired PIDs >are working as expected. The test tries to clone3() with a set_tid of >-1, 1, pid_max, a PID which is already in use and an unused PID. The >same tests are also running in PID namespace. > >Signed-off-by: Adrian Reber <areber@redhat.com> >--- > tools/testing/selftests/clone3/.gitignore | 2 + > tools/testing/selftests/clone3/Makefile | 11 ++ > tools/testing/selftests/clone3/clone3.c | 141 +++++++++++++++ > .../testing/selftests/clone3/clone3_set_tid.c | 161 ++++++++++++++++++ > 4 files changed, 315 insertions(+) > create mode 100644 tools/testing/selftests/clone3/.gitignore > create mode 100644 tools/testing/selftests/clone3/Makefile > create mode 100644 tools/testing/selftests/clone3/clone3.c > create mode 100644 tools/testing/selftests/clone3/clone3_set_tid.c > >diff --git a/tools/testing/selftests/clone3/.gitignore >b/tools/testing/selftests/clone3/.gitignore >new file mode 100644 >index 000000000000..c63c64a78ddf >--- /dev/null >+++ b/tools/testing/selftests/clone3/.gitignore >@@ -0,0 +1,2 @@ >+clone3_set_tid >+clone3 >diff --git a/tools/testing/selftests/clone3/Makefile >b/tools/testing/selftests/clone3/Makefile >new file mode 100644 >index 000000000000..4efcf45b995b >--- /dev/null >+++ b/tools/testing/selftests/clone3/Makefile >@@ -0,0 +1,11 @@ >+# SPDX-License-Identifier: GPL-2.0 >+uname_M := $(shell uname -m 2>/dev/null || echo not) >+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 >+ >+include ../lib.mk >diff --git a/tools/testing/selftests/clone3/clone3.c >b/tools/testing/selftests/clone3/clone3.c >new file mode 100644 >index 000000000000..55a6915566b8 >--- /dev/null >+++ b/tools/testing/selftests/clone3/clone3.c >@@ -0,0 +1,141 @@ >+// SPDX-License-Identifier: GPL-2.0 >+ >+/* Based on Christian Brauner's clone3() example */ >+ >+#define _GNU_SOURCE >+#include <errno.h> >+#include <linux/types.h> >+#include <linux/sched.h> >+#include <stdio.h> >+#include <stdlib.h> >+#include <sys/syscall.h> >+#include <sys/types.h> >+#include <sys/un.h> >+#include <sys/wait.h> >+#include <unistd.h> >+#include <sched.h> >+ >+#include "../kselftest.h" >+ >+static pid_t raw_clone(struct clone_args *args) >+{ >+ return syscall(__NR_clone3, args, sizeof(struct clone_args)); >+} >+ >+static int call_clone3(int flags) >+{ >+ struct clone_args args = {0}; >+ pid_t ppid = -1; >+ pid_t pid = -1; >+ int status; >+ >+ args.flags = flags; >+ args.exit_signal = SIGCHLD; >+ >+ pid = raw_clone(&args); >+ if (pid < 0) { >+ ksft_print_msg("%s - Failed to create new process\n", >+ strerror(errno)); >+ return -errno; >+ } >+ >+ if (pid == 0) { >+ ksft_print_msg("I am the child, my PID is %d\n", getpid()); >+ _exit(EXIT_SUCCESS); >+ } >+ >+ ppid = getpid(); >+ ksft_print_msg("I am the parent (%d). My child's pid is %d\n", >+ ppid, pid); >+ >+ (void)wait(&status); >+ if (WEXITSTATUS(status)) >+ return WEXITSTATUS(status); >+ >+ return 0; >+} >+ >+static int test_clone3(int flags, int expected) >+{ >+ int ret; >+ >+ ksft_print_msg("[%d] Trying clone3() with flags 0x%x\n", >+ getpid(), flags); >+ ret = call_clone3(flags); >+ ksft_print_msg("[%d] clone3() with flags says :%d expected %d\n", >+ getpid(), ret, expected); >+ if (ret != expected) >+ ksft_exit_fail_msg( >+ "[%d] Result (%d) is different than expected (%d)\n", >+ getpid(), ret, expected); >+ ksft_test_result_pass("[%d] Result (%d) matches expectation (%d)\n", >+ getpid(), ret, expected); >+ return 0; >+} >+int main(int argc, char *argv[]) >+{ >+ int ret = -1; >+ pid_t pid; >+ >+ ksft_print_header(); >+ ksft_set_plan(3); >+ >+ /* Just a simple clone3() should return 0.*/ >+ if (test_clone3(0, 0)) >+ goto on_error; >+ /* Do a clone3() in a new PID NS.*/ >+ if (test_clone3(CLONE_NEWPID, 0)) >+ goto on_error; >+ ksft_print_msg("First unshare\n"); >+ if (unshare(CLONE_NEWPID)) >+ goto on_error; >+ /* >+ * Before clone3()ing in a new PID NS with >+ * CLONE_NEWPID a fork() is necessary. >+ */ >+ if (test_clone3(CLONE_NEWPID, -EINVAL)) >+ goto on_error; >+ pid = fork(); >+ if (pid < 0) { >+ ksft_print_msg("First fork() failed\n"); >+ goto on_error; >+ } >+ if (pid > 0) { >+ (void)wait(NULL); >+ goto parent_out; >+ } >+ ksft_set_plan(6); >+ if (test_clone3(CLONE_NEWPID, 0)) >+ goto on_error; >+ if (test_clone3(0, 0)) >+ goto on_error; >+ ksft_print_msg("Second unshare\n"); >+ if (unshare(CLONE_NEWPID)) >+ goto on_error; >+ /* >+ * Before clone3()ing in a new PID NS with >+ * CLONE_NEWPID a fork() is necessary. >+ */ >+ if (test_clone3(CLONE_NEWPID, -EINVAL)) >+ goto on_error; >+ pid = fork(); >+ if (pid < 0) { >+ ksft_print_msg("Second fork() failed\n"); >+ goto on_error; >+ } >+ if (pid > 0) { >+ (void)wait(NULL); >+ goto parent_out; >+ } >+ ksft_set_plan(8); >+ if (test_clone3(CLONE_NEWPID, 0)) >+ goto on_error; >+ if (test_clone3(0, 0)) >+ goto on_error; >+ >+parent_out: >+ ret = 0; >+on_error: >+ >+ return !ret ? ksft_exit_pass() : ksft_exit_fail(); >+} >diff --git a/tools/testing/selftests/clone3/clone3_set_tid.c >b/tools/testing/selftests/clone3/clone3_set_tid.c >new file mode 100644 >index 000000000000..f5012e84dcb3 >--- /dev/null >+++ b/tools/testing/selftests/clone3/clone3_set_tid.c >@@ -0,0 +1,161 @@ >+// SPDX-License-Identifier: GPL-2.0 >+ >+/* Based on Christian Brauner's clone3() example */ >+ >+#define _GNU_SOURCE >+#include <errno.h> >+#include <linux/types.h> >+#include <linux/sched.h> >+#include <stdio.h> >+#include <stdlib.h> >+#include <sys/syscall.h> >+#include <sys/types.h> >+#include <sys/un.h> >+#include <sys/wait.h> >+#include <unistd.h> >+#include <sched.h> >+ >+#include "../kselftest.h" >+ >+static pid_t raw_clone(struct clone_args *args) >+{ >+ return syscall(__NR_clone3, args, sizeof(struct clone_args)); >+} >+ >+static int call_clone3_set_tid(int set_tid, int flags) >+{ >+ struct clone_args args = {0}; >+ pid_t ppid = -1; >+ pid_t pid = -1; >+ int status; >+ >+ args.flags = flags; >+ args.exit_signal = SIGCHLD; >+ args.set_tid = set_tid; >+ >+ pid = raw_clone(&args); >+ if (pid < 0) { >+ ksft_print_msg("%s - Failed to create new process\n", >+ strerror(errno)); >+ return -errno; >+ } >+ >+ if (pid == 0) { >+ ksft_print_msg("I am the child, my PID is %d (expected %d)\n", >+ getpid(), set_tid); >+ if (set_tid != getpid()) >+ _exit(EXIT_FAILURE); >+ _exit(EXIT_SUCCESS); >+ } >+ >+ ppid = getpid(); >+ ksft_print_msg("I am the parent (%d). My child's pid is %d\n", >+ ppid, pid); >+ >+ (void)wait(&status); >+ if (WEXITSTATUS(status)) >+ return WEXITSTATUS(status); >+ >+ return 0; >+} >+ >+static int test_clone3_set_tid(int set_tid, int flags, int expected) >+{ >+ int ret; >+ >+ ksft_print_msg( >+ "[%d] Trying clone3() with CLONE_SET_TID to %d and 0x%x\n", >+ getpid(), set_tid, flags); >+ ret = call_clone3_set_tid(set_tid, flags); >+ ksft_print_msg( >+ "[%d] clone3() with CLONE_SET_TID %d says :%d - expected %d\n", >+ getpid(), set_tid, ret, expected); >+ if (ret != expected) >+ ksft_exit_fail_msg( >+ "[%d] Result (%d) is different than expected (%d)\n", >+ getpid(), ret, expected); >+ ksft_test_result_pass("[%d] Result (%d) matches expectation (%d)\n", >+ getpid(), ret, expected); >+ return 0; >+} >+int main(int argc, char *argv[]) >+{ >+ FILE *f; >+ int pid_max = 0; >+ pid_t pid; >+ pid_t ns_pid; >+ int ret = -1; >+ >+ ksft_print_header(); >+ ksft_set_plan(13); >+ >+ f = fopen("/proc/sys/kernel/pid_max", "r"); >+ if (f == NULL) >+ ksft_exit_fail_msg( >+ "%s - Could not open /proc/sys/kernel/pid_max\n", >+ strerror(errno)); >+ fscanf(f, "%d", &pid_max); >+ fclose(f); >+ ksft_print_msg("/proc/sys/kernel/pid_max %d\n", pid_max); >+ >+ /* First try with an invalid PID */ >+ if (test_clone3_set_tid(-1, 0, -EINVAL)) >+ goto on_error; >+ if (test_clone3_set_tid(-1, CLONE_NEWPID, -EINVAL)) >+ goto on_error; >+ /* Then with PID 1 */ >+ if (test_clone3_set_tid(1, 0, -EEXIST)) >+ goto on_error; >+ /* PID 1 should not fail in a PID namespace */ >+ if (test_clone3_set_tid(1, CLONE_NEWPID, 0)) >+ goto on_error; >+ /* pid_max should fail everywhere */ >+ if (test_clone3_set_tid(pid_max, 0, -EINVAL)) >+ goto on_error; >+ if (test_clone3_set_tid(pid_max, CLONE_NEWPID, -EINVAL)) >+ goto on_error; >+ /* Find the current active PID */ >+ pid = fork(); >+ if (pid == 0) { >+ ksft_print_msg("Child has PID %d\n", getpid()); >+ sleep(1); >+ _exit(EXIT_SUCCESS); >+ } >+ /* Try to create a process with that PID should fail */ >+ if (test_clone3_set_tid(pid, 0, -EEXIST)) >+ goto on_error; >+ (void)wait(NULL); >+ /* After the child has finished, try again with the same PID */ >+ if (test_clone3_set_tid(pid, 0, 0)) >+ goto on_error; >+ /* This should fail as there is no PID 1 in that namespace */ >+ if (test_clone3_set_tid(pid, CLONE_NEWPID, -EINVAL)) >+ goto on_error; >+ unshare(CLONE_NEWPID); >+ if (test_clone3_set_tid(10, 0, -EINVAL)) >+ goto on_error; >+ /* Let's create a PID 1 */ >+ ns_pid = fork(); >+ if (ns_pid == 0) { >+ ksft_print_msg("Child in PID namespace has PID %d\n", getpid()); >+ sleep(1); >+ _exit(EXIT_SUCCESS); >+ } >+ /* >+ * Now, after the unshare() it should be possible to create a process >+ * with another ID than 1 in the PID namespace. >+ */ >+ if (test_clone3_set_tid(2, 0, 0)) >+ goto on_error; >+ /* Use a different PID in this namespace. */ >+ if (test_clone3_set_tid(2222, 0, 0)) >+ goto on_error; >+ if (test_clone3_set_tid(1, 0, -EEXIST)) >+ goto on_error; >+ (void)wait(NULL); >+ >+ ret = 0; >+on_error: >+ >+ return !ret ? ksft_exit_pass() : ksft_exit_fail(); >+}
Thanks for the tests! Could you also add tests where we pass invalid or unknown struct sizes?: - pass struct size that is too small aka < CLONE3_ARGS_SIZE_V0 - pass a struct size that is too large aka > PAGE_SIZE - pass struct size that is smaller than the one the kernel supports - pass struct size that is larger than the one the kernel supports; once with all unknown fields set to 0 and once with a field set to non-0
I think this might fit well with this patch since it changes the struct size. :)
Sorry, I didn't know you'd be sending out a new version today. :)
Christian
| |