On Mon, Nov 04, 2019 at 02:18:46PM +0100, Adrian Reber wrote: > This adds tests for clone3() with different values and sizes > of struct clone_args. > > This selftest was initially part of of the clone3() with PID selftest. > After that patch was almost merged Eugene sent out a couple of patches > to fix problems with these test. > > This commit now only contains the clone3() selftest after the LPC > decision to rework clone3() with PID to allow setting the PID in > multiple PID namespaces including all of Eugene's patches. > > Signed-off-by: Eugene Syromiatnikov <esyr@xxxxxxxxxx> > Signed-off-by: Adrian Reber <areber@xxxxxxxxxx> > > A few more comments below. > > Also, would you be open to adding tests here for the newly added .stack > and .stack_size API (cf. [1])? > > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=fa729c4df558936b4a1a7b3e2234011f44ede28b > > --- > v2: > - Applied Christian's suggestions > - Skip root-only tests when running as non-root > --- > MAINTAINERS | 1 + > tools/testing/selftests/Makefile | 1 + > tools/testing/selftests/clone3/.gitignore | 1 + > tools/testing/selftests/clone3/Makefile | 7 + > tools/testing/selftests/clone3/clone3.c | 225 ++++++++++++++++++++++ > 5 files changed, 235 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 > > diff --git a/MAINTAINERS b/MAINTAINERS > index cba1095547fd..0040b7a6410b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12829,6 +12829,7 @@ S: Maintained > T: git git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux.git > F: samples/pidfd/ > F: tools/testing/selftests/pidfd/ > +F: tools/testing/selftests/clone3/ > K: (?i)pidfd > K: (?i)clone3 > K: \b(clone_args|kernel_clone_args)\b > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 4cdbae6f4e61..ad442364218a 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 > diff --git a/tools/testing/selftests/clone3/.gitignore b/tools/testing/selftests/clone3/.gitignore > new file mode 100644 > index 000000000000..85d9d3ba2524 > --- /dev/null > +++ b/tools/testing/selftests/clone3/.gitignore > @@ -0,0 +1 @@ > +clone3 > diff --git a/tools/testing/selftests/clone3/Makefile b/tools/testing/selftests/clone3/Makefile > new file mode 100644 > index 000000000000..ea922c014ae4 > --- /dev/null > +++ b/tools/testing/selftests/clone3/Makefile > @@ -0,0 +1,7 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +CFLAGS += -I../../../../usr/include/ > + > +TEST_GEN_PROGS := clone3 > + > +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..a982d95189bf > --- /dev/null > +++ b/tools/testing/selftests/clone3/clone3.c > @@ -0,0 +1,225 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* Based on Christian Brauner's clone3() example */ > + > +#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> > +#include <sys/types.h> > +#include <sys/un.h> > +#include <sys/wait.h> > +#include <unistd.h> > +#include <sched.h> > + > +#include "../kselftest.h" > + > +/* > + * Different sizes of struct clone_args > + */ > +#ifndef CLONE3_ARGS_SIZE_V0 > +#define CLONE3_ARGS_SIZE_V0 64 > +#endif > + > +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) > +{ > + return syscall(__NR_clone3, args, size); > +} > + > +static int call_clone3(uint64_t flags, size_t size, enum test_mode test_mode) > +{ > + struct clone_args args = { > + .flags = flags, > + .exit_signal = SIGCHLD, > + }; > + > + struct clone_args_extended { > + struct clone_args args; > + __aligned_u64 excess_space[2]; > + } args_ext; > + > + pid_t pid = -1; > + int status; > + > + memset(&args_ext, 0, sizeof(args_ext)); > + if (size > sizeof(struct clone_args)) > + args_ext.excess_space[1] = 1; > + > + if (size == 0) > + size = sizeof(struct clone_args); > + > + switch (test_mode) { > + case CLONE3_ARGS_ALL_0: > + args.flags = 0; > + args.exit_signal = 0; > + break; > + case CLONE3_ARGS_ALL_1: > > I don't fully understand this test case. What is this for exactly? > > + args.flags = 1; > + args.pidfd = 1; > + args.child_tid = 1; > + args.parent_tid = 1; > + args.exit_signal = 1; > + args.stack = 1; > + args.tls = 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; > + } > + > + memcpy(&args_ext.args, &args, sizeof(struct clone_args)); > + > + pid = raw_clone((struct clone_args *)&args_ext, size); > + 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); > + } > + > + ksft_print_msg("I am the parent (%d). My child's pid is %d\n", > + getpid(), pid); > + > + if (wait(&status) < 0) { > + ksft_print_msg("Child returned %s\n", strerror(errno)); > + return -errno; > + } > + if (WEXITSTATUS(status)) > + return WEXITSTATUS(status); > + > + return 0; > +} > + > +static void 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 %#" 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", > + getpid(), ret, expected); > + if (ret != expected) > + ksft_test_result_fail( > + "[%d] Result (%d) is different than expected (%d)\n", > + getpid(), ret, expected); > + else > + ksft_test_result_pass( > + "[%d] Result (%d) matches expectation (%d)\n", > + getpid(), ret, expected); > +} > +int main(int argc, char *argv[]) > +{ > + pid_t pid; > + > + uid_t uid = getuid(); > + > + ksft_print_header(); > + ksft_set_plan(17); > + > + /* Just a simple clone3() should return 0.*/ > + test_clone3(0, 0, 0, CLONE3_ARGS_NO_TEST); > + > + /* Do a clone3() in a new PID NS.*/ > + if (uid == 0) > + test_clone3(CLONE_NEWPID, 0, 0, CLONE3_ARGS_NO_TEST); > + else > + ksft_test_result_skip("Skipping clone3() with CLONE_NEWPID\n"); > + > + /* Do a clone3() with CLONE3_ARGS_SIZE_V0. */ > + test_clone3(0, CLONE3_ARGS_SIZE_V0, 0, CLONE3_ARGS_NO_TEST); > + > + /* Do a clone3() with CLONE3_ARGS_SIZE_V0 - 8 */ > + test_clone3(0, CLONE3_ARGS_SIZE_V0 - 8, -EINVAL, CLONE3_ARGS_NO_TEST); > + > + /* Do a clone3() with sizeof(struct clone_args) + 8 */ > + test_clone3(0, sizeof(struct clone_args) + 8, 0, CLONE3_ARGS_NO_TEST); > + > + /* Do a clone3() with all members set to 1 */ > + test_clone3(0, 0, -EINVAL, CLONE3_ARGS_ALL_1); > + > + /* Do a clone3() with exit_signal having highest 32 bits non-zero */ > + test_clone3(0, 0, -EINVAL, CLONE3_ARGS_INVAL_EXIT_SIGNAL_BIG); > + > + /* Do a clone3() with negative 32-bit exit_signal */ > + test_clone3(0, 0, -EINVAL, CLONE3_ARGS_INVAL_EXIT_SIGNAL_NEG); > + > + /* Do a clone3() with exit_signal not fitting into CSIGNAL mask */ > + test_clone3(0, 0, -EINVAL, CLONE3_ARGS_INVAL_EXIT_SIGNAL_CSIG); > + > + /* Do a clone3() with NSIG < exit_signal < CSIG */ > + test_clone3(0, 0, -EINVAL, CLONE3_ARGS_INVAL_EXIT_SIGNAL_NSIG); > + > + /* > + * Do a clone3() with sizeof(struct clone_args) + 8 > + * and all members set to 0. This resets exit_signal and wait() > + * will not get a result. > + */ > + test_clone3(0, sizeof(struct clone_args) + 8, -ECHILD, > + CLONE3_ARGS_ALL_0); > > Ah, I haven't caught this during the first review. No, that's not > correct. You don't get a signal since exit_signal is 0, that's correct > _but_ you still need to wait on the child. You don't see the child > because the kernel will by default not wait on clone()-children aka > processes created with exit_signal != SIGCHLD. To wait on > clone()-children as well, you need to set __WCLONE or __WALL in the > options argument for wait{p}id(2). > > + > + /* > + * Do a clone3() with sizeof(struct clone_args) + 8 > + * and all members set to 0. > + */ > + test_clone3(0, sizeof(struct clone_args) + 8, -EINVAL, > + CLONE3_ARGS_ALL_1); > > The comment and the test don't line up. :) > > + > + /* Do a clone3() with > page size */ > + test_clone3(0, getpagesize() + 8, -E2BIG, CLONE3_ARGS_NO_TEST); > + > + /* Do a clone3() with CLONE3_ARGS_SIZE_V0 in a new PID NS. */ > + if (uid == 0) > + test_clone3(CLONE_NEWPID, CLONE3_ARGS_SIZE_V0, 0, > + CLONE3_ARGS_NO_TEST); > + else > + ksft_test_result_skip("Skipping clone3() with CLONE_NEWPID\n"); > + > + /* Do a clone3() with CLONE3_ARGS_SIZE_V0 - 8 in a new PID NS */ > + test_clone3(CLONE_NEWPID, CLONE3_ARGS_SIZE_V0 - 8, -EINVAL, > + CLONE3_ARGS_NO_TEST); > + > + /* Do a clone3() with sizeof(struct clone_args) + 8 in a new PID NS */ > + if (uid == 0) > + test_clone3(CLONE_NEWPID, sizeof(struct clone_args) + 8, 0, > + CLONE3_ARGS_NO_TEST); > + else > + ksft_test_result_skip("Skipping clone3() with CLONE_NEWPID\n"); > + > + /* Do a clone3() with > page size in a new PID NS */ > + test_clone3(CLONE_NEWPID, getpagesize() + 8, -E2BIG, > + CLONE3_ARGS_NO_TEST); > + > + return !ksft_get_fail_cnt() ? ksft_exit_pass() : ksft_exit_fail(); > +} > -- > 2.23.0 >