From 67da5e102b6280e4373c209766e96d48e67619da Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 14 Oct 2019 12:45:37 +0200 Subject: [PATCH 01/13] UPSTREAM: clone3: add CLONE_CLEAR_SIGHAND Reset all signal handlers of the child not set to SIG_IGN to SIG_DFL. Mutually exclusive with CLONE_SIGHAND to not disturb other thread's signal handler. In the spirit of closer cooperation between glibc developers and kernel developers (cf. [2]) this patchset came out of a discussion on the glibc mailing list for improving posix_spawn() (cf. [1], [3], [4]). Kernel support for this feature has been explicitly requested by glibc and I see no reason not to help them with this. The child helper process on Linux posix_spawn must ensure that no signal handlers are enabled, so the signal disposition must be either SIG_DFL or SIG_IGN. However, it requires a sigprocmask to obtain the current signal mask and at least _NSIG sigaction calls to reset the signal handlers for each posix_spawn call or complex state tracking that might lead to data corruption in glibc. Adding this flags lets glibc avoid these problems. [1]: https://www.sourceware.org/ml/libc-alpha/2019-10/msg00149.html [3]: https://www.sourceware.org/ml/libc-alpha/2019-10/msg00158.html [4]: https://www.sourceware.org/ml/libc-alpha/2019-10/msg00160.html [2]: https://lwn.net/Articles/799331/ '[...] by asking for better cooperation with the C-library projects in general. They should be copied on patches containing ABI changes, for example. I noted that there are often times where C-library developers wish the kernel community had done things differently; how could those be avoided in the future? Members of the audience suggested that more glibc developers should perhaps join the linux-api list. The other suggestion was to "copy Florian on everything".' Cc: Florian Weimer Cc: libc-alpha@sourceware.org Cc: linux-api@vger.kernel.org Change-Id: Iced02ef93ed4dcda5ca5d3bd475cbfcae928fb19 Signed-off-by: Christian Brauner Reviewed-by: Oleg Nesterov Link: https://lore.kernel.org/r/20191014104538.3096-1-christian.brauner@ubuntu.com Signed-off-by: ralph950412 --- include/uapi/linux/sched.h | 3 +++ kernel/fork.c | 16 +++++++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h index 25b4fa00bad1..8a1c888fc2e7 100644 --- a/include/uapi/linux/sched.h +++ b/include/uapi/linux/sched.h @@ -33,6 +33,9 @@ #define CLONE_NEWNET 0x40000000 /* New network namespace */ #define CLONE_IO 0x80000000 /* Clone io context */ +/* Flags for the clone3() syscall. */ +#define CLONE_CLEAR_SIGHAND 0x100000000ULL /* Clear any signal handler and reset to SIG_DFL. */ + #ifndef __ASSEMBLY__ /** * struct clone_args - arguments for the clone3 syscall diff --git a/kernel/fork.c b/kernel/fork.c index bb45f6817d52..f12fe3e16a62 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1540,6 +1540,11 @@ static int copy_sighand(unsigned long clone_flags, struct task_struct *tsk) spin_lock_irq(¤t->sighand->siglock); memcpy(sig->action, current->sighand->action, sizeof(sig->action)); spin_unlock_irq(¤t->sighand->siglock); + + /* Reset all signal handler not set to SIG_IGN to SIG_DFL. */ + if (clone_flags & CLONE_CLEAR_SIGHAND) + flush_signal_handlers(tsk, 0); + return 0; } @@ -2721,11 +2726,8 @@ static inline bool clone3_stack_valid(struct kernel_clone_args *kargs) static bool clone3_args_valid(struct kernel_clone_args *kargs) { - /* - * All lower bits of the flag word are taken. - * Verify that no other unknown flags are passed along. - */ - if (kargs->flags & ~CLONE_LEGACY_FLAGS) + /* Verify that no unknown flags are passed along. */ + if (kargs->flags & ~(CLONE_LEGACY_FLAGS | CLONE_CLEAR_SIGHAND)) return false; /* @@ -2735,6 +2737,10 @@ static bool clone3_args_valid(struct kernel_clone_args *kargs) if (kargs->flags & (CLONE_DETACHED | CSIGNAL)) return false; + if ((kargs->flags & (CLONE_SIGHAND | CLONE_CLEAR_SIGHAND)) == + (CLONE_SIGHAND | CLONE_CLEAR_SIGHAND)) + return false; + if ((kargs->flags & (CLONE_THREAD | CLONE_PARENT)) && kargs->exit_signal) return false; From 399202290e9194c30f67cbab1fd61f27276d2071 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Mon, 14 Oct 2019 12:45:38 +0200 Subject: [PATCH 02/13] UPSTREAM: tests: test CLONE_CLEAR_SIGHAND Test that CLONE_CLEAR_SIGHAND resets signal handlers to SIG_DFL for the child process and that CLONE_CLEAR_SIGHAND and CLONE_SIGHAND are mutually exclusive. Cc: Florian Weimer Cc: libc-alpha@sourceware.org Cc: linux-api@vger.kernel.org Change-Id: I07ebb8cb2e741cf1a6a43ff4dbe6ee489636f506 Signed-off-by: Christian Brauner Link: https://lore.kernel.org/r/20191014104538.3096-2-christian.brauner@ubuntu.com Signed-off-by: ralph950412 --- MAINTAINERS | 1 + tools/testing/selftests/Makefile | 1 + tools/testing/selftests/clone3/.gitignore | 1 + tools/testing/selftests/clone3/Makefile | 6 + .../selftests/clone3/clone3_clear_sighand.c | 172 ++++++++++++++++++ 5 files changed, 181 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_clear_sighand.c diff --git a/MAINTAINERS b/MAINTAINERS index 7b73fc0469c8..3cf5cc8042c4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12851,6 +12851,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 9514f6dbe46b..7b4106268541 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..6c9f98097774 --- /dev/null +++ b/tools/testing/selftests/clone3/.gitignore @@ -0,0 +1 @@ +clone3_clear_sighand diff --git a/tools/testing/selftests/clone3/Makefile b/tools/testing/selftests/clone3/Makefile new file mode 100644 index 000000000000..e6f259321e16 --- /dev/null +++ b/tools/testing/selftests/clone3/Makefile @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0-only +CFLAGS += -g -I../../../../usr/include/ + +TEST_GEN_PROGS := clone3_clear_sighand + +include ../lib.mk diff --git a/tools/testing/selftests/clone3/clone3_clear_sighand.c b/tools/testing/selftests/clone3/clone3_clear_sighand.c new file mode 100644 index 000000000000..0d957be1bdc5 --- /dev/null +++ b/tools/testing/selftests/clone3/clone3_clear_sighand.c @@ -0,0 +1,172 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../kselftest.h" + +#ifndef CLONE_CLEAR_SIGHAND +#define CLONE_CLEAR_SIGHAND 0x100000000ULL +#endif + +#ifndef __NR_clone3 +#define __NR_clone3 -1 +struct clone_args { + __aligned_u64 flags; + __aligned_u64 pidfd; + __aligned_u64 child_tid; + __aligned_u64 parent_tid; + __aligned_u64 exit_signal; + __aligned_u64 stack; + __aligned_u64 stack_size; + __aligned_u64 tls; +}; +#endif + +static pid_t sys_clone3(struct clone_args *args, size_t size) +{ + return syscall(__NR_clone3, args, size); +} + +static void test_clone3_supported(void) +{ + pid_t pid; + struct clone_args args = {}; + + if (__NR_clone3 < 0) + ksft_exit_skip("clone3() syscall is not supported\n"); + + /* Set to something that will always cause EINVAL. */ + args.exit_signal = -1; + pid = sys_clone3(&args, sizeof(args)); + if (!pid) + exit(EXIT_SUCCESS); + + if (pid > 0) { + wait(NULL); + ksft_exit_fail_msg( + "Managed to create child process with invalid exit_signal\n"); + } + + if (errno == ENOSYS) + ksft_exit_skip("clone3() syscall is not supported\n"); + + ksft_print_msg("clone3() syscall supported\n"); +} + +static void nop_handler(int signo) +{ +} + +static int wait_for_pid(pid_t pid) +{ + int status, ret; + +again: + ret = waitpid(pid, &status, 0); + if (ret == -1) { + if (errno == EINTR) + goto again; + + return -1; + } + + if (!WIFEXITED(status)) + return -1; + + return WEXITSTATUS(status); +} + +static void test_clone3_clear_sighand(void) +{ + int ret; + pid_t pid; + struct clone_args args = {}; + struct sigaction act; + + /* + * Check that CLONE_CLEAR_SIGHAND and CLONE_SIGHAND are mutually + * exclusive. + */ + args.flags |= CLONE_CLEAR_SIGHAND | CLONE_SIGHAND; + args.exit_signal = SIGCHLD; + pid = sys_clone3(&args, sizeof(args)); + if (pid > 0) + ksft_exit_fail_msg( + "clone3(CLONE_CLEAR_SIGHAND | CLONE_SIGHAND) succeeded\n"); + + act.sa_handler = nop_handler; + ret = sigemptyset(&act.sa_mask); + if (ret < 0) + ksft_exit_fail_msg("%s - sigemptyset() failed\n", + strerror(errno)); + + act.sa_flags = 0; + + /* Register signal handler for SIGUSR1 */ + ret = sigaction(SIGUSR1, &act, NULL); + if (ret < 0) + ksft_exit_fail_msg( + "%s - sigaction(SIGUSR1, &act, NULL) failed\n", + strerror(errno)); + + /* Register signal handler for SIGUSR2 */ + ret = sigaction(SIGUSR2, &act, NULL); + if (ret < 0) + ksft_exit_fail_msg( + "%s - sigaction(SIGUSR2, &act, NULL) failed\n", + strerror(errno)); + + /* Check that CLONE_CLEAR_SIGHAND works. */ + args.flags = CLONE_CLEAR_SIGHAND; + pid = sys_clone3(&args, sizeof(args)); + if (pid < 0) + ksft_exit_fail_msg("%s - clone3(CLONE_CLEAR_SIGHAND) failed\n", + strerror(errno)); + + if (pid == 0) { + ret = sigaction(SIGUSR1, NULL, &act); + if (ret < 0) + exit(EXIT_FAILURE); + + if (act.sa_handler != SIG_DFL) + exit(EXIT_FAILURE); + + ret = sigaction(SIGUSR2, NULL, &act); + if (ret < 0) + exit(EXIT_FAILURE); + + if (act.sa_handler != SIG_DFL) + exit(EXIT_FAILURE); + + exit(EXIT_SUCCESS); + } + + ret = wait_for_pid(pid); + if (ret) + ksft_exit_fail_msg( + "Failed to clear signal handler for child process\n"); + + ksft_test_result_pass("Cleared signal handlers for child process\n"); +} + +int main(int argc, char **argv) +{ + ksft_print_header(); + ksft_set_plan(1); + + test_clone3_supported(); + test_clone3_clear_sighand(); + + return ksft_exit_pass(); +} From 00925755945f39ddaf3c456f06cc2b52c9944131 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 12 Nov 2019 10:58:51 +0100 Subject: [PATCH 03/13] UPSTREAM: selftests: add tests for clone3() 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. Change-Id: I32fecd920f7a820c0ca7869a70a8c5b34af583dd Signed-off-by: Eugene Syromiatnikov Signed-off-by: Adrian Reber Reviewed-by: Christian Brauner Link: https://lore.kernel.org/r/20191112095851.811884-1-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- tools/testing/selftests/clone3/.gitignore | 1 + tools/testing/selftests/clone3/Makefile | 4 +- tools/testing/selftests/clone3/clone3.c | 205 ++++++++++++++++++++++ 3 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/clone3/clone3.c diff --git a/tools/testing/selftests/clone3/.gitignore b/tools/testing/selftests/clone3/.gitignore index 6c9f98097774..2a30ae18b06e 100644 --- a/tools/testing/selftests/clone3/.gitignore +++ b/tools/testing/selftests/clone3/.gitignore @@ -1 +1,2 @@ +clone3 clone3_clear_sighand diff --git a/tools/testing/selftests/clone3/Makefile b/tools/testing/selftests/clone3/Makefile index e6f259321e16..eb26eb793c80 100644 --- a/tools/testing/selftests/clone3/Makefile +++ b/tools/testing/selftests/clone3/Makefile @@ -1,6 +1,6 @@ -# SPDX-License-Identifier: GPL-2.0-only +# SPDX-License-Identifier: GPL-2.0 CFLAGS += -g -I../../../../usr/include/ -TEST_GEN_PROGS := clone3_clear_sighand +TEST_GEN_PROGS := clone3 clone3_clear_sighand 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..0f8a9ef40117 --- /dev/null +++ b/tools/testing/selftests/clone3/clone3.c @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* Based on Christian Brauner's clone3() example */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#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_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_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 (waitpid(-1, &status, __WALL) < 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 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); + + test_clone3(0, sizeof(struct clone_args) + 8, 0, CLONE3_ARGS_ALL_0); + + test_clone3(0, sizeof(struct clone_args) + 16, -E2BIG, + CLONE3_ARGS_ALL_0); + + test_clone3(0, sizeof(struct clone_args) * 2, -E2BIG, + CLONE3_ARGS_ALL_0); + + /* 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(); +} From 62ec29d7ade78ee74e9fd5f996c410e70014e3e4 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Fri, 15 Nov 2019 13:36:20 +0100 Subject: [PATCH 04/13] UPSTREAM: fork: extend clone3() to support setting a PID The main motivation to add set_tid to clone3() is CRIU. To restore a process with the same PID/TID CRIU currently uses /proc/sys/kernel/ns_last_pid. It writes the desired (PID - 1) to ns_last_pid and then (quickly) does a clone(). This works most of the time, but it is racy. It is also slow as it requires multiple syscalls. Extending clone3() to support *set_tid makes it possible restore a process using CRIU without accessing /proc/sys/kernel/ns_last_pid and race free (as long as the desired PID/TID is available). This clone3() extension places the same restrictions (CAP_SYS_ADMIN) on clone3() with *set_tid as they are currently in place for ns_last_pid. The original version of this change was using a single value for set_tid. At the 2019 LPC, after presenting set_tid, it was, however, decided to change set_tid to an array to enable setting the PID of a process in multiple PID namespaces at the same time. If a process is created in a PID namespace it is possible to influence the PID inside and outside of the PID namespace. Details also in the corresponding selftest. To create a process with the following PIDs: PID NS level Requested PID 0 (host) 31496 1 42 2 1 For that example the two newly introduced parameters to struct clone_args (set_tid and set_tid_size) would need to be: set_tid[0] = 1; set_tid[1] = 42; set_tid[2] = 31496; set_tid_size = 3; If only the PIDs of the two innermost nested PID namespaces should be defined it would look like this: set_tid[0] = 1; set_tid[1] = 42; set_tid_size = 2; The PID of the newly created process would then be the next available free PID in the PID namespace level 0 (host) and 42 in the PID namespace at level 1 and the PID of the process in the innermost PID namespace would be 1. The set_tid array is used to specify the PID of a process starting from the innermost nested PID namespaces up to set_tid_size PID namespaces. set_tid_size cannot be larger then the current PID namespace level. Change-Id: I21a8a13c5a794b2a788f3e647fd639b5ae313ebf Signed-off-by: Adrian Reber Reviewed-by: Christian Brauner Reviewed-by: Oleg Nesterov Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com> Acked-by: Andrei Vagin Link: https://lore.kernel.org/r/20191115123621.142252-1-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- include/linux/pid.h | 3 +- include/linux/pid_namespace.h | 2 + include/linux/sched/task.h | 3 ++ include/uapi/linux/sched.h | 61 +++++++++++++++++++----------- kernel/fork.c | 24 +++++++++++- kernel/pid.c | 70 ++++++++++++++++++++++++++++------- kernel/pid_namespace.c | 2 - 7 files changed, 125 insertions(+), 40 deletions(-) diff --git a/include/linux/pid.h b/include/linux/pid.h index 8c836713d6c4..064847e90cf0 100644 --- a/include/linux/pid.h +++ b/include/linux/pid.h @@ -126,7 +126,8 @@ extern struct pid *find_vpid(int nr); extern struct pid *find_get_pid(int nr); extern struct pid *find_ge_pid(int nr, struct pid_namespace *); -extern struct pid *alloc_pid(struct pid_namespace *ns); +extern struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid, + size_t set_tid_size); extern void free_pid(struct pid *pid); extern void disable_pid_allocation(struct pid_namespace *ns); diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h index 49538b172483..2ed6af88794b 100644 --- a/include/linux/pid_namespace.h +++ b/include/linux/pid_namespace.h @@ -12,6 +12,8 @@ #include #include +/* MAX_PID_NS_LEVEL is needed for limiting size of 'struct pid' */ +#define MAX_PID_NS_LEVEL 32 struct fs_pin; diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h index e3c20a4f81f5..b53ee6d7111d 100644 --- a/include/linux/sched/task.h +++ b/include/linux/sched/task.h @@ -26,6 +26,9 @@ struct kernel_clone_args { unsigned long stack; unsigned long stack_size; unsigned long tls; + pid_t *set_tid; + /* Number of elements in *set_tid */ + size_t set_tid_size; }; /* diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h index 8a1c888fc2e7..4a0217832464 100644 --- a/include/uapi/linux/sched.h +++ b/include/uapi/linux/sched.h @@ -39,28 +39,42 @@ #ifndef __ASSEMBLY__ /** * struct clone_args - arguments for the clone3 syscall - * @flags: Flags for the new process as listed above. - * All flags are valid except for CSIGNAL and - * CLONE_DETACHED. - * @pidfd: If CLONE_PIDFD is set, a pidfd will be - * returned in this argument. - * @child_tid: If CLONE_CHILD_SETTID is set, the TID of the - * child process will be returned in the child's - * memory. - * @parent_tid: If CLONE_PARENT_SETTID is set, the TID of - * the child process will be returned in the - * parent's memory. - * @exit_signal: The exit_signal the parent process will be - * sent when the child exits. - * @stack: Specify the location of the stack for the - * child process. - * Note, @stack is expected to point to the - * lowest address. The stack direction will be - * determined by the kernel and set up - * appropriately based on @stack_size. - * @stack_size: The size of the stack for the child process. - * @tls: If CLONE_SETTLS is set, the tls descriptor - * is set to tls. + * @flags: Flags for the new process as listed above. + * All flags are valid except for CSIGNAL and + * CLONE_DETACHED. + * @pidfd: If CLONE_PIDFD is set, a pidfd will be + * returned in this argument. + * @child_tid: If CLONE_CHILD_SETTID is set, the TID of the + * child process will be returned in the child's + * memory. + * @parent_tid: If CLONE_PARENT_SETTID is set, the TID of + * the child process will be returned in the + * parent's memory. + * @exit_signal: The exit_signal the parent process will be + * sent when the child exits. + * @stack: Specify the location of the stack for the + * child process. + * Note, @stack is expected to point to the + * lowest address. The stack direction will be + * determined by the kernel and set up + * appropriately based on @stack_size. + * @stack_size: The size of the stack for the child process. + * @tls: If CLONE_SETTLS is set, the tls descriptor + * is set to tls. + * @set_tid: Pointer to an array of type *pid_t. The size + * of the array is defined using @set_tid_size. + * This array is used to select PIDs/TIDs for + * newly created processes. The first element in + * this defines the PID in the most nested PID + * namespace. Each additional element in the array + * defines the PID in the parent PID namespace of + * the original PID namespace. If the array has + * less entries than the number of currently + * nested PID namespaces only the PIDs in the + * corresponding namespaces are set. + * @set_tid_size: This defines the size of the array referenced + * in @set_tid. This cannot be larger than the + * kernel's limit of nested PID namespaces. * * The structure is versioned by size and thus extensible. * New struct members must go at the end of the struct and @@ -75,10 +89,13 @@ struct clone_args { __aligned_u64 stack; __aligned_u64 stack_size; __aligned_u64 tls; + __aligned_u64 set_tid; + __aligned_u64 set_tid_size; }; #endif #define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */ +#define CLONE_ARGS_SIZE_VER1 80 /* sizeof second published struct */ /* * Scheduling policies diff --git a/kernel/fork.c b/kernel/fork.c index f12fe3e16a62..3b98f44b8799 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2166,7 +2166,8 @@ static __latent_entropy struct task_struct *copy_process( stackleak_task_init(p); if (pid != &init_struct_pid) { - pid = alloc_pid(p->nsproxy->pid_ns_for_children); + pid = alloc_pid(p->nsproxy->pid_ns_for_children, args->set_tid, + args->set_tid_size); if (IS_ERR(pid)) { retval = PTR_ERR(pid); goto bad_fork_cleanup_thread; @@ -2664,6 +2665,7 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, { int err; struct clone_args args; + pid_t *kset_tid = kargs->set_tid; if (unlikely(usize > PAGE_SIZE)) return -E2BIG; @@ -2674,6 +2676,15 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, if (err) return err; + if (unlikely(args.set_tid_size > MAX_PID_NS_LEVEL)) + return -EINVAL; + + if (unlikely(!args.set_tid && args.set_tid_size > 0)) + return -EINVAL; + + if (unlikely(args.set_tid && args.set_tid_size == 0)) + return -EINVAL; + /* * Verify that higher 32bits of exit_signal are unset and that * it is a valid signal @@ -2691,8 +2702,16 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, .stack = args.stack, .stack_size = args.stack_size, .tls = args.tls, + .set_tid_size = args.set_tid_size, }; + if (args.set_tid && + copy_from_user(kset_tid, u64_to_user_ptr(args.set_tid), + (kargs->set_tid_size * sizeof(pid_t)))) + return -EFAULT; + + kargs->set_tid = kset_tid; + return 0; } @@ -2767,6 +2786,9 @@ SYSCALL_DEFINE2(clone3, struct clone_args __user *, uargs, size_t, size) int err; struct kernel_clone_args kargs; + pid_t set_tid[MAX_PID_NS_LEVEL]; + + kargs.set_tid = set_tid; err = copy_clone_args_from_user(&kargs, uargs, size); if (err) diff --git a/kernel/pid.c b/kernel/pid.c index 289d0b12ab86..15c80e211fb2 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -157,7 +157,8 @@ void free_pid(struct pid *pid) call_rcu(&pid->rcu, delayed_put_pid); } -struct pid *alloc_pid(struct pid_namespace *ns) +struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid, + size_t set_tid_size) { struct pid *pid; enum pid_type type; @@ -166,6 +167,17 @@ struct pid *alloc_pid(struct pid_namespace *ns) struct upid *upid; int retval = -ENOMEM; + /* + * set_tid_size contains the size of the set_tid array. Starting at + * the most nested currently active PID namespace it tells alloc_pid() + * which PID to set for a process in that most nested PID namespace + * up to set_tid_size PID namespaces. It does not have to set the PID + * for a process in all nested PID namespaces but set_tid_size must + * never be greater than the current ns->level + 1. + */ + if (set_tid_size > ns->level + 1) + return ERR_PTR(-EINVAL); + pid = kmem_cache_alloc(ns->pid_cachep, GFP_KERNEL); if (!pid) return ERR_PTR(retval); @@ -174,24 +186,54 @@ struct pid *alloc_pid(struct pid_namespace *ns) pid->level = ns->level; for (i = ns->level; i >= 0; i--) { - int pid_min = 1; + int tid = 0; + + if (set_tid_size) { + tid = set_tid[ns->level - i]; + + retval = -EINVAL; + if (tid < 1 || tid >= pid_max) + goto out_free; + /* + * Also fail if a PID != 1 is requested and + * no PID 1 exists. + */ + if (tid != 1 && !tmp->child_reaper) + goto out_free; + retval = -EPERM; + if (!ns_capable(tmp->user_ns, CAP_SYS_ADMIN)) + goto out_free; + set_tid_size--; + } idr_preload(GFP_KERNEL); spin_lock_irq(&pidmap_lock); - /* - * init really needs pid 1, but after reaching the maximum - * wrap back to RESERVED_PIDS - */ - if (idr_get_cursor(&tmp->idr) > RESERVED_PIDS) - pid_min = RESERVED_PIDS; + if (tid) { + nr = idr_alloc(&tmp->idr, NULL, tid, + tid + 1, GFP_ATOMIC); + /* + * If ENOSPC is returned it means that the PID is + * alreay in use. Return EEXIST in that case. + */ + if (nr == -ENOSPC) + nr = -EEXIST; + } else { + int pid_min = 1; + /* + * init really needs pid 1, but after reaching the + * maximum wrap back to RESERVED_PIDS + */ + if (idr_get_cursor(&tmp->idr) > RESERVED_PIDS) + pid_min = RESERVED_PIDS; - /* - * Store a null pointer so find_pid_ns does not find - * a partially initialized PID (see below). - */ - nr = idr_alloc_cyclic(&tmp->idr, NULL, pid_min, - pid_max, GFP_ATOMIC); + /* + * Store a null pointer so find_pid_ns does not find + * a partially initialized PID (see below). + */ + nr = idr_alloc_cyclic(&tmp->idr, NULL, pid_min, + pid_max, GFP_ATOMIC); + } spin_unlock_irq(&pidmap_lock); idr_preload_end(); diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c index ef914bb9b469..57db6df5b9d3 100644 --- a/kernel/pid_namespace.c +++ b/kernel/pid_namespace.c @@ -26,8 +26,6 @@ static DEFINE_MUTEX(pid_caches_mutex); static struct kmem_cache *pid_ns_cachep; -/* MAX_PID_NS_LEVEL is needed for limiting size of 'struct pid' */ -#define MAX_PID_NS_LEVEL 32 /* Write once array, filled from the beginning. */ static struct kmem_cache *pid_cache[MAX_PID_NS_LEVEL]; From 2c395af267bea437bbe124fb00e55aebdcad0813 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Fri, 15 Nov 2019 13:36:21 +0100 Subject: [PATCH 05/13] UPSTREAM: selftests: add tests for clone3() with *set_tid This tests clone3() with *set_tid to see if all desired PIDs are working as expected. The tests are trying multiple invalid input parameters as well as creating processes while specifying a certain PID in multiple PID namespaces at the same time. Additionally this moves common clone3() test code into clone3_selftests.h. Change-Id: Ic30e083c2b8c3e5c1fe9adff28387e84431f4c71 Signed-off-by: Adrian Reber Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20191115123621.142252-2-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- tools/testing/selftests/clone3/.gitignore | 1 + tools/testing/selftests/clone3/Makefile | 2 +- tools/testing/selftests/clone3/clone3.c | 8 +- .../selftests/clone3/clone3_clear_sighand.c | 20 +- .../selftests/clone3/clone3_selftests.h | 35 ++ .../testing/selftests/clone3/clone3_set_tid.c | 381 ++++++++++++++++++ 6 files changed, 421 insertions(+), 26 deletions(-) create mode 100644 tools/testing/selftests/clone3/clone3_selftests.h 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 index 2a30ae18b06e..0dc4f32c6cb8 100644 --- a/tools/testing/selftests/clone3/.gitignore +++ b/tools/testing/selftests/clone3/.gitignore @@ -1,2 +1,3 @@ clone3 clone3_clear_sighand +clone3_set_tid diff --git a/tools/testing/selftests/clone3/Makefile b/tools/testing/selftests/clone3/Makefile index eb26eb793c80..cf976c732906 100644 --- a/tools/testing/selftests/clone3/Makefile +++ b/tools/testing/selftests/clone3/Makefile @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 CFLAGS += -g -I../../../../usr/include/ -TEST_GEN_PROGS := clone3 clone3_clear_sighand +TEST_GEN_PROGS := clone3 clone3_clear_sighand clone3_set_tid include ../lib.mk diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c index 0f8a9ef40117..4669b3d418e7 100644 --- a/tools/testing/selftests/clone3/clone3.c +++ b/tools/testing/selftests/clone3/clone3.c @@ -18,6 +18,7 @@ #include #include "../kselftest.h" +#include "clone3_selftests.h" /* * Different sizes of struct clone_args @@ -35,11 +36,6 @@ enum test_mode { 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 = { @@ -83,7 +79,7 @@ static int call_clone3(uint64_t flags, size_t size, enum test_mode test_mode) memcpy(&args_ext.args, &args, sizeof(struct clone_args)); - pid = raw_clone((struct clone_args *)&args_ext, size); + pid = sys_clone3((struct clone_args *)&args_ext, size); if (pid < 0) { ksft_print_msg("%s - Failed to create new process\n", strerror(errno)); diff --git a/tools/testing/selftests/clone3/clone3_clear_sighand.c b/tools/testing/selftests/clone3/clone3_clear_sighand.c index 0d957be1bdc5..456783ad19d6 100644 --- a/tools/testing/selftests/clone3/clone3_clear_sighand.c +++ b/tools/testing/selftests/clone3/clone3_clear_sighand.c @@ -14,30 +14,12 @@ #include #include "../kselftest.h" +#include "clone3_selftests.h" #ifndef CLONE_CLEAR_SIGHAND #define CLONE_CLEAR_SIGHAND 0x100000000ULL #endif -#ifndef __NR_clone3 -#define __NR_clone3 -1 -struct clone_args { - __aligned_u64 flags; - __aligned_u64 pidfd; - __aligned_u64 child_tid; - __aligned_u64 parent_tid; - __aligned_u64 exit_signal; - __aligned_u64 stack; - __aligned_u64 stack_size; - __aligned_u64 tls; -}; -#endif - -static pid_t sys_clone3(struct clone_args *args, size_t size) -{ - return syscall(__NR_clone3, args, size); -} - static void test_clone3_supported(void) { pid_t pid; diff --git a/tools/testing/selftests/clone3/clone3_selftests.h b/tools/testing/selftests/clone3/clone3_selftests.h new file mode 100644 index 000000000000..1a270390766a --- /dev/null +++ b/tools/testing/selftests/clone3/clone3_selftests.h @@ -0,0 +1,35 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef _CLONE3_SELFTESTS_H +#define _CLONE3_SELFTESTS_H + +#define _GNU_SOURCE +#include +#include +#include +#include + +#define ptr_to_u64(ptr) ((__u64)((uintptr_t)(ptr))) + +#ifndef __NR_clone3 +#define __NR_clone3 -1 +struct clone_args { + __aligned_u64 flags; + __aligned_u64 pidfd; + __aligned_u64 child_tid; + __aligned_u64 parent_tid; + __aligned_u64 exit_signal; + __aligned_u64 stack; + __aligned_u64 stack_size; + __aligned_u64 tls; + __aligned_u64 set_tid; + __aligned_u64 set_tid_size; +}; +#endif + +static pid_t sys_clone3(struct clone_args *args, size_t size) +{ + return syscall(__NR_clone3, args, size); +} + +#endif /* _CLONE3_SELFTESTS_H */ 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..3480e1c46983 --- /dev/null +++ b/tools/testing/selftests/clone3/clone3_set_tid.c @@ -0,0 +1,381 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Based on Christian Brauner's clone3() example. + * These tests are assuming to be running in the host's + * PID namespace. + */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../kselftest.h" +#include "clone3_selftests.h" + +#ifndef MAX_PID_NS_LEVEL +#define MAX_PID_NS_LEVEL 32 +#endif + +static int pipe_1[2]; +static int pipe_2[2]; + +static int call_clone3_set_tid(pid_t *set_tid, + size_t set_tid_size, + int flags, + int expected_pid, + bool wait_for_it) +{ + int status; + pid_t pid = -1; + + struct clone_args args = { + .flags = flags, + .exit_signal = SIGCHLD, + .set_tid = ptr_to_u64(set_tid), + .set_tid_size = set_tid_size, + }; + + pid = sys_clone3(&args, sizeof(struct clone_args)); + if (pid < 0) { + ksft_print_msg("%s - Failed to create new process\n", + strerror(errno)); + return -errno; + } + + if (pid == 0) { + int ret; + char tmp = 0; + int exit_code = EXIT_SUCCESS; + + ksft_print_msg("I am the child, my PID is %d (expected %d)\n", + getpid(), set_tid[0]); + if (wait_for_it) { + ksft_print_msg("[%d] Child is ready and waiting\n", + getpid()); + + /* Signal the parent that the child is ready */ + close(pipe_1[0]); + ret = write(pipe_1[1], &tmp, 1); + if (ret != 1) { + ksft_print_msg( + "Writing to pipe returned %d", ret); + exit_code = EXIT_FAILURE; + } + close(pipe_1[1]); + close(pipe_2[1]); + ret = read(pipe_2[0], &tmp, 1); + if (ret != 1) { + ksft_print_msg( + "Reading from pipe returned %d", ret); + exit_code = EXIT_FAILURE; + } + close(pipe_2[0]); + } + + if (set_tid[0] != getpid()) + _exit(EXIT_FAILURE); + _exit(exit_code); + } + + if (expected_pid == 0 || expected_pid == pid) { + ksft_print_msg("I am the parent (%d). My child's pid is %d\n", + getpid(), pid); + } else { + ksft_print_msg( + "Expected child pid %d does not match actual pid %d\n", + expected_pid, pid); + return -1; + } + + if (waitpid(pid, &status, 0) < 0) { + ksft_print_msg("Child returned %s\n", strerror(errno)); + return -errno; + } + + if (!WIFEXITED(status)) + return -1; + + return WEXITSTATUS(status); +} + +static void test_clone3_set_tid(pid_t *set_tid, + size_t set_tid_size, + int flags, + int expected, + int expected_pid, + bool wait_for_it) +{ + int ret; + + ksft_print_msg( + "[%d] Trying clone3() with CLONE_SET_TID to %d and 0x%x\n", + getpid(), set_tid[0], flags); + ret = call_clone3_set_tid(set_tid, set_tid_size, flags, expected_pid, + wait_for_it); + ksft_print_msg( + "[%d] clone3() with CLONE_SET_TID %d says :%d - expected %d\n", + getpid(), set_tid[0], 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[]) +{ + FILE *f; + char buf; + char *line; + int status; + int ret = -1; + size_t len = 0; + int pid_max = 0; + uid_t uid = getuid(); + char proc_path[100] = {0}; + pid_t pid, ns1, ns2, ns3, ns_pid; + pid_t set_tid[MAX_PID_NS_LEVEL * 2]; + + if (pipe(pipe_1) < 0 || pipe(pipe_2) < 0) + ksft_exit_fail_msg("pipe() failed\n"); + + ksft_print_header(); + ksft_set_plan(27); + + 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); + + /* Try invalid settings */ + memset(&set_tid, 0, sizeof(set_tid)); + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL + 1, 0, -EINVAL, 0, 0); + + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL * 2, 0, -EINVAL, 0, 0); + + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL * 2 + 1, 0, + -EINVAL, 0, 0); + + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL * 42, 0, -EINVAL, 0, 0); + + /* + * This can actually work if this test running in a MAX_PID_NS_LEVEL - 1 + * nested PID namespace. + */ + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL - 1, 0, -EINVAL, 0, 0); + + memset(&set_tid, 0xff, sizeof(set_tid)); + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL + 1, 0, -EINVAL, 0, 0); + + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL * 2, 0, -EINVAL, 0, 0); + + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL * 2 + 1, 0, + -EINVAL, 0, 0); + + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL * 42, 0, -EINVAL, 0, 0); + + /* + * This can actually work if this test running in a MAX_PID_NS_LEVEL - 1 + * nested PID namespace. + */ + test_clone3_set_tid(set_tid, MAX_PID_NS_LEVEL - 1, 0, -EINVAL, 0, 0); + + memset(&set_tid, 0, sizeof(set_tid)); + /* Try with an invalid PID */ + set_tid[0] = 0; + test_clone3_set_tid(set_tid, 1, 0, -EINVAL, 0, 0); + + set_tid[0] = -1; + test_clone3_set_tid(set_tid, 1, 0, -EINVAL, 0, 0); + + /* Claim that the set_tid array actually contains 2 elements. */ + test_clone3_set_tid(set_tid, 2, 0, -EINVAL, 0, 0); + + /* Try it in a new PID namespace */ + if (uid == 0) + test_clone3_set_tid(set_tid, 1, CLONE_NEWPID, -EINVAL, 0, 0); + else + ksft_test_result_skip("Clone3() with set_tid requires root\n"); + + /* Try with a valid PID (1) this should return -EEXIST. */ + set_tid[0] = 1; + if (uid == 0) + test_clone3_set_tid(set_tid, 1, 0, -EEXIST, 0, 0); + else + ksft_test_result_skip("Clone3() with set_tid requires root\n"); + + /* Try it in a new PID namespace */ + if (uid == 0) + test_clone3_set_tid(set_tid, 1, CLONE_NEWPID, 0, 0, 0); + else + ksft_test_result_skip("Clone3() with set_tid requires root\n"); + + /* pid_max should fail everywhere */ + set_tid[0] = pid_max; + test_clone3_set_tid(set_tid, 1, 0, -EINVAL, 0, 0); + + if (uid == 0) + test_clone3_set_tid(set_tid, 1, CLONE_NEWPID, -EINVAL, 0, 0); + else + ksft_test_result_skip("Clone3() with set_tid requires root\n"); + + if (uid != 0) { + /* + * All remaining tests require root. Tell the framework + * that all those tests are skipped as non-root. + */ + ksft_cnt.ksft_xskip += ksft_plan - ksft_test_num(); + goto out; + } + + /* Find the current active PID */ + pid = fork(); + if (pid == 0) { + ksft_print_msg("Child has PID %d\n", getpid()); + _exit(EXIT_SUCCESS); + } + if (waitpid(pid, &status, 0) < 0) + ksft_exit_fail_msg("Waiting for child %d failed", pid); + + /* After the child has finished, its PID should be free. */ + set_tid[0] = pid; + test_clone3_set_tid(set_tid, 1, 0, 0, 0, 0); + + /* This should fail as there is no PID 1 in that namespace */ + test_clone3_set_tid(set_tid, 1, CLONE_NEWPID, -EINVAL, 0, 0); + + /* + * Creating a process with PID 1 in the newly created most nested + * PID namespace and PID 'pid' in the parent PID namespace. This + * needs to work. + */ + set_tid[0] = 1; + set_tid[1] = pid; + test_clone3_set_tid(set_tid, 2, CLONE_NEWPID, 0, pid, 0); + + ksft_print_msg("unshare PID namespace\n"); + if (unshare(CLONE_NEWPID) == -1) + ksft_exit_fail_msg("unshare(CLONE_NEWPID) failed: %s\n", + strerror(errno)); + + set_tid[0] = pid; + + /* This should fail as there is no PID 1 in that namespace */ + test_clone3_set_tid(set_tid, 1, 0, -EINVAL, 0, 0); + + /* 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()); + set_tid[0] = 2; + test_clone3_set_tid(set_tid, 1, 0, 0, 2, 0); + + set_tid[0] = 1; + set_tid[1] = -1; + set_tid[2] = pid; + /* This should fail as there is invalid PID at level '1'. */ + test_clone3_set_tid(set_tid, 3, CLONE_NEWPID, -EINVAL, 0, 0); + + set_tid[0] = 1; + set_tid[1] = 42; + set_tid[2] = pid; + /* + * This should fail as there are not enough active PID + * namespaces. Again assuming this is running in the host's + * PID namespace. Not yet nested. + */ + test_clone3_set_tid(set_tid, 4, CLONE_NEWPID, -EINVAL, 0, 0); + + /* + * This should work and from the parent we should see + * something like 'NSpid: pid 42 1'. + */ + test_clone3_set_tid(set_tid, 3, CLONE_NEWPID, 0, 42, true); + + _exit(ksft_cnt.ksft_pass); + } + + close(pipe_1[1]); + close(pipe_2[0]); + while (read(pipe_1[0], &buf, 1) > 0) { + ksft_print_msg("[%d] Child is ready and waiting\n", getpid()); + break; + } + + snprintf(proc_path, sizeof(proc_path), "/proc/%d/status", pid); + f = fopen(proc_path, "r"); + if (f == NULL) + ksft_exit_fail_msg( + "%s - Could not open %s\n", + strerror(errno), proc_path); + + while (getline(&line, &len, f) != -1) { + if (strstr(line, "NSpid")) { + int i; + + /* Verify that all generated PIDs are as expected. */ + i = sscanf(line, "NSpid:\t%d\t%d\t%d", + &ns3, &ns2, &ns1); + if (i != 3) { + ksft_print_msg( + "Unexpected 'NSPid:' entry: %s", + line); + ns1 = ns2 = ns3 = 0; + } + break; + } + } + fclose(f); + free(line); + close(pipe_2[0]); + + /* Tell the clone3()'d child to finish. */ + write(pipe_2[1], &buf, 1); + close(pipe_2[1]); + + if (waitpid(ns_pid, &status, 0) < 0) { + ksft_print_msg("Child returned %s\n", strerror(errno)); + ret = -errno; + goto out; + } + + if (!WIFEXITED(status)) + ksft_test_result_fail("Child error\n"); + + if (WEXITSTATUS(status)) + /* + * Update the number of total tests with the tests from the + * child processes. + */ + ksft_cnt.ksft_pass = WEXITSTATUS(status); + + if (ns3 == pid && ns2 == 42 && ns1 == 1) + ksft_test_result_pass( + "PIDs in all namespaces as expected (%d,%d,%d)\n", + ns3, ns2, ns1); + else + ksft_test_result_fail( + "PIDs in all namespaces not as expected (%d,%d,%d)\n", + ns3, ns2, ns1); +out: + ret = 0; + + return !ret ? ksft_exit_pass() : ksft_exit_fail(); +} From ef8a66d0ee9a542a85263edb01b659eb44071b21 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Sun, 19 Jul 2020 12:04:11 +0200 Subject: [PATCH 06/13] UPSTREAM: capabilities: Introduce CAP_CHECKPOINT_RESTORE This patch introduces CAP_CHECKPOINT_RESTORE, a new capability facilitating checkpoint/restore for non-root users. Over the last years, The CRIU (Checkpoint/Restore In Userspace) team has been asked numerous times if it is possible to checkpoint/restore a process as non-root. The answer usually was: 'almost'. The main blocker to restore a process as non-root was to control the PID of the restored process. This feature available via the clone3 system call, or via /proc/sys/kernel/ns_last_pid is unfortunately guarded by CAP_SYS_ADMIN. In the past two years, requests for non-root checkpoint/restore have increased due to the following use cases: * Checkpoint/Restore in an HPC environment in combination with a resource manager distributing jobs where users are always running as non-root. There is a desire to provide a way to checkpoint and restore long running jobs. * Container migration as non-root * We have been in contact with JVM developers who are integrating CRIU into a Java VM to decrease the startup time. These checkpoint/restore applications are not meant to be running with CAP_SYS_ADMIN. We have seen the following workarounds: * Use a setuid wrapper around CRIU: See https://github.com/FredHutch/slurm-examples/blob/master/checkpointer/lib/checkpointer/checkpointer-suid.c * Use a setuid helper that writes to ns_last_pid. Unfortunately, this helper delegation technique is impossible to use with clone3, and is thus prone to races. See https://github.com/twosigma/set_ns_last_pid * Cycle through PIDs with fork() until the desired PID is reached: This has been demonstrated to work with cycling rates of 100,000 PIDs/s See https://github.com/twosigma/set_ns_last_pid * Patch out the CAP_SYS_ADMIN check from the kernel * Run the desired application in a new user and PID namespace to provide a local CAP_SYS_ADMIN for controlling PIDs. This technique has limited use in typical container environments (e.g., Kubernetes) as /proc is typically protected with read-only layers (e.g., /proc/sys) for hardening purposes. Read-only layers prevent additional /proc mounts (due to proc's SB_I_USERNS_VISIBLE property), making the use of new PID namespaces limited as certain applications need access to /proc matching their PID namespace. The introduced capability allows to: * Control PIDs when the current user is CAP_CHECKPOINT_RESTORE capable for the corresponding PID namespace via ns_last_pid/clone3. * Open files in /proc/pid/map_files when the current user is CAP_CHECKPOINT_RESTORE capable in the root namespace, useful for recovering files that are unreachable via the file system such as deleted files, or memfd files. See corresponding selftest for an example with clone3(). Change-Id: Ia9ae3fc9a698e37774be945bac5a7d2a0d640cbc Signed-off-by: Adrian Reber Signed-off-by: Nicolas Viennot Reviewed-by: Serge Hallyn Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20200719100418.2112740-2-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- include/linux/capability.h | 6 ++++++ include/uapi/linux/capability.h | 9 ++++++++- security/selinux/include/classmap.h | 5 +++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/include/linux/capability.h b/include/linux/capability.h index b4345b38a6be..1e7fe311cabe 100644 --- a/include/linux/capability.h +++ b/include/linux/capability.h @@ -261,6 +261,12 @@ static inline bool bpf_capable(void) return capable(CAP_BPF) || capable(CAP_SYS_ADMIN); } +static inline bool checkpoint_restore_ns_capable(struct user_namespace *ns) +{ + return ns_capable(ns, CAP_CHECKPOINT_RESTORE) || + ns_capable(ns, CAP_SYS_ADMIN); +} + /* audit system wants to get cap info from files as well */ extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data *cpu_caps); diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h index 5fa5aac3f2ef..ecd623eea9fe 100644 --- a/include/uapi/linux/capability.h +++ b/include/uapi/linux/capability.h @@ -405,7 +405,14 @@ struct vfs_ns_cap_data { */ #define CAP_BPF 39 -#define CAP_LAST_CAP CAP_BPF + +/* Allow checkpoint/restore related operations */ +/* Allow PID selection during clone3() */ +/* Allow writing to ns_last_pid */ + +#define CAP_CHECKPOINT_RESTORE 40 + +#define CAP_LAST_CAP CAP_CHECKPOINT_RESTORE #define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP) diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h index 78ece5ab9886..d77f52b70671 100644 --- a/security/selinux/include/classmap.h +++ b/security/selinux/include/classmap.h @@ -27,9 +27,10 @@ "audit_control", "setfcap" #define COMMON_CAP2_PERMS "mac_override", "mac_admin", "syslog", \ - "wake_alarm", "block_suspend", "audit_read", "perfmon", "bpf" + "wake_alarm", "block_suspend", "audit_read", "perfmon", "bpf", \ + "checkpoint_restore" -#if CAP_LAST_CAP > CAP_BPF +#if CAP_LAST_CAP > CAP_CHECKPOINT_RESTORE #error New capability defined, please update COMMON_CAP2_PERMS. #endif From 5c348f563d5457e1e1b20c01ee42bffd1fe265ae Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Sun, 19 Jul 2020 12:04:12 +0200 Subject: [PATCH 07/13] UPSTREAM: pid: use checkpoint_restore_ns_capable() for set_tid Use the newly introduced capability CAP_CHECKPOINT_RESTORE to allow using clone3() with set_tid set. Change-Id: Ifc86c455e500e59a62f9960ba094c4f9973891ff Signed-off-by: Adrian Reber Signed-off-by: Nicolas Viennot Reviewed-by: Serge Hallyn Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20200719100418.2112740-3-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- kernel/pid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/pid.c b/kernel/pid.c index 15c80e211fb2..b7adaef4889b 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -201,7 +201,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid, if (tid != 1 && !tmp->child_reaper) goto out_free; retval = -EPERM; - if (!ns_capable(tmp->user_ns, CAP_SYS_ADMIN)) + if (!checkpoint_restore_ns_capable(tmp->user_ns)) goto out_free; set_tid_size--; } From 63b4832326141e0d0b0f83e1a4508c067d3af566 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Sun, 19 Jul 2020 12:04:13 +0200 Subject: [PATCH 08/13] UPSTREAM: pid_namespace: use checkpoint_restore_ns_capable() for ns_last_pid Use the newly introduced capability CAP_CHECKPOINT_RESTORE to allow writing to ns_last_pid. Change-Id: I95727c385f136818553260db0e9edfa654f7907e Signed-off-by: Adrian Reber Signed-off-by: Nicolas Viennot Reviewed-by: Serge Hallyn Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20200719100418.2112740-4-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- kernel/pid_namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c index 57db6df5b9d3..78b409b88fbd 100644 --- a/kernel/pid_namespace.c +++ b/kernel/pid_namespace.c @@ -270,7 +270,7 @@ static int pid_ns_ctl_handler(struct ctl_table *table, int write, struct ctl_table tmp = *table; int ret, next; - if (write && !ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN)) + if (write && !checkpoint_restore_ns_capable(pid_ns->user_ns)) return -EPERM; /* From ef13cc913cc47965ed9c43ec4079cae454b123f9 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Sun, 19 Jul 2020 12:04:14 +0200 Subject: [PATCH 09/13] UPSTREAM: proc: allow access in init userns for map_files with CAP_CHECKPOINT_RESTORE Opening files in /proc/pid/map_files when the current user is CAP_CHECKPOINT_RESTORE capable in the root namespace is useful for checkpointing and restoring to recover files that are unreachable via the file system such as deleted files, or memfd files. Change-Id: I6ec3846f81a5208b66ef41e00229b7535e1e6668 Signed-off-by: Adrian Reber Signed-off-by: Nicolas Viennot Reviewed-by: Cyrill Gorcunov Reviewed-by: Serge Hallyn Link: https://lore.kernel.org/r/20200719100418.2112740-5-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- fs/proc/base.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index 9143e5f41890..791ade1d1019 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2211,16 +2211,16 @@ struct map_files_info { }; /* - * Only allow CAP_SYS_ADMIN to follow the links, due to concerns about how the - * symlinks may be used to bypass permissions on ancestor directories in the - * path to the file in question. + * Only allow CAP_SYS_ADMIN and CAP_CHECKPOINT_RESTORE to follow the links, due + * to concerns about how the symlinks may be used to bypass permissions on + * ancestor directories in the path to the file in question. */ static const char * proc_map_files_get_link(struct dentry *dentry, struct inode *inode, struct delayed_call *done) { - if (!capable(CAP_SYS_ADMIN)) + if (!checkpoint_restore_ns_capable(&init_user_ns)) return ERR_PTR(-EPERM); return proc_pid_get_link(dentry, inode, done); From c90ed82a1d05e3271d99ce9b8c81aaf381066f6e Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Sun, 19 Jul 2020 12:04:15 +0200 Subject: [PATCH 10/13] UPSTREAM: prctl: Allow local CAP_CHECKPOINT_RESTORE to change /proc/self/exe Originally, only a local CAP_SYS_ADMIN could change the exe link, making it difficult for doing checkpoint/restore without CAP_SYS_ADMIN. This commit adds CAP_CHECKPOINT_RESTORE in addition to CAP_SYS_ADMIN for permitting changing the exe link. The following describes the history of the /proc/self/exe permission checks as it may be difficult to understand what decisions lead to this point. * [1] May 2012: This commit introduces the ability of changing /proc/self/exe if the user is CAP_SYS_RESOURCE capable. In the related discussion [2], no clear thread model is presented for what could happen if the /proc/self/exe changes multiple times, or why would the admin be at the mercy of userspace. * [3] Oct 2014: This commit introduces a new API to change /proc/self/exe. The permission no longer checks for CAP_SYS_RESOURCE, but instead checks if the current user is root (uid=0) in its local namespace. In the related discussion [4] it is said that "Controlling exe_fd without privileges may turn out to be dangerous. At least things like tomoyo examine it for making policy decisions (see tomoyo_manager())." * [5] Dec 2016: This commit removes the restriction to change /proc/self/exe at most once. The related discussion [6] informs that the audit subsystem relies on the exe symlink, presumably audit_log_d_path_exe() in kernel/audit.c. * [7] May 2017: This commit changed the check from uid==0 to local CAP_SYS_ADMIN. No discussion. * [8] July 2020: A PoC to spoof any program's /proc/self/exe via ptrace is demonstrated Overall, the concrete points that were made to retain capability checks around changing the exe symlink is that tomoyo_manager() and audit_log_d_path_exe() uses the exe_file path. Christian Brauner said that relying on /proc//exe being immutable (or guarded by caps) in a sake of security is a bit misleading. It can only be used as a hint without any guarantees of what code is being executed once execve() returns to userspace. Christian suggested that in the future, we could call audit_log() or similar to inform the admin of all exe link changes, instead of attempting to provide security guarantees via permission checks. However, this proposed change requires the understanding of the security implications in the tomoyo/audit subsystems. [1] b32dfe377102 ("c/r: prctl: add ability to set new mm_struct::exe_file") [2] https://lore.kernel.org/patchwork/patch/292515/ [3] f606b77f1a9e ("prctl: PR_SET_MM -- introduce PR_SET_MM_MAP operation") [4] https://lore.kernel.org/patchwork/patch/479359/ [5] 3fb4afd9a504 ("prctl: remove one-shot limitation for changing exe link") [6] https://lore.kernel.org/patchwork/patch/697304/ [7] 4d28df6152aa ("prctl: Allow local CAP_SYS_ADMIN changing exe_file") [8] https://github.com/nviennot/run_as_exe Change-Id: I44b78ea8000443bc862178aae3f847e86785a1a1 Signed-off-by: Nicolas Viennot Signed-off-by: Adrian Reber Link: https://lore.kernel.org/r/20200719100418.2112740-6-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- kernel/sys.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/kernel/sys.c b/kernel/sys.c index fa3f48a51aa9..96f06c4984a7 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -2005,11 +2005,14 @@ static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data if (prctl_map.exe_fd != (u32)-1) { /* - * Make sure the caller has the rights to - * change /proc/pid/exe link: only local sys admin should - * be allowed to. + * Check if the current user is checkpoint/restore capable. + * At the time of this writing, it checks for CAP_SYS_ADMIN + * or CAP_CHECKPOINT_RESTORE. + * Note that a user with access to ptrace can masquerade an + * arbitrary program as any executable, even setuid ones. + * This may have implications in the tomoyo subsystem. */ - if (!ns_capable(current_user_ns(), CAP_SYS_ADMIN)) + if (!checkpoint_restore_ns_capable(current_user_ns())) return -EINVAL; error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd); From 6b4aab010e0946adfd3c1fc332e68be0e5fe002d Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Sun, 19 Jul 2020 12:04:16 +0200 Subject: [PATCH 11/13] UPSTREAM: prctl: exe link permission error changed from -EINVAL to -EPERM This brings consistency with the rest of the prctl() syscall where -EPERM is returned when failing a capability check. Change-Id: I1b27a485d4ca9dd99376c6628572509835aabaf3 Signed-off-by: Nicolas Viennot Signed-off-by: Adrian Reber Reviewed-by: Serge Hallyn Link: https://lore.kernel.org/r/20200719100418.2112740-7-areber@redhat.com Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- kernel/sys.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sys.c b/kernel/sys.c index 96f06c4984a7..3ed05f046c92 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -2013,7 +2013,7 @@ static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data * This may have implications in the tomoyo subsystem. */ if (!checkpoint_restore_ns_capable(current_user_ns())) - return -EINVAL; + return -EPERM; error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd); if (error) From e5abb5d796df9f9895aa62c799879d0417e2784c Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Sun, 19 Jul 2020 12:04:17 +0200 Subject: [PATCH 12/13] UPSTREAM: selftests: add clone3() CAP_CHECKPOINT_RESTORE test This adds a test that changes its UID, uses capabilities to get CAP_CHECKPOINT_RESTORE and uses clone3() with set_tid to create a process with a given PID as non-root. Change-Id: Iac62e21213ad1bd04f5a2762255fc924c3830ea6 Signed-off-by: Adrian Reber Link: https://lore.kernel.org/r/20200719100418.2112740-8-areber@redhat.com [christian.brauner@ubuntu.com: use TH_LOG() instead of ksft_print_msg()] Signed-off-by: Christian Brauner Signed-off-by: ralph950412 --- tools/testing/selftests/clone3/.gitignore | 1 + tools/testing/selftests/clone3/Makefile | 4 +- .../clone3/clone3_cap_checkpoint_restore.c | 182 ++++++++++++++++++ 3 files changed, 186 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c diff --git a/tools/testing/selftests/clone3/.gitignore b/tools/testing/selftests/clone3/.gitignore index 0dc4f32c6cb8..fb6d704eb61e 100644 --- a/tools/testing/selftests/clone3/.gitignore +++ b/tools/testing/selftests/clone3/.gitignore @@ -1,3 +1,4 @@ clone3 clone3_clear_sighand clone3_set_tid +clone3_cap_checkpoint_restore diff --git a/tools/testing/selftests/clone3/Makefile b/tools/testing/selftests/clone3/Makefile index cf976c732906..ef7564cb7abe 100644 --- a/tools/testing/selftests/clone3/Makefile +++ b/tools/testing/selftests/clone3/Makefile @@ -1,6 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 CFLAGS += -g -I../../../../usr/include/ +LDLIBS += -lcap -TEST_GEN_PROGS := clone3 clone3_clear_sighand clone3_set_tid +TEST_GEN_PROGS := clone3 clone3_clear_sighand clone3_set_tid \ + clone3_cap_checkpoint_restore include ../lib.mk diff --git a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c new file mode 100644 index 000000000000..9562425aa0a9 --- /dev/null +++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* + * Based on Christian Brauner's clone3() example. + * These tests are assuming to be running in the host's + * PID namespace. + */ + +/* capabilities related code based on selftests/bpf/test_verifier.c */ + +#define _GNU_SOURCE +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../kselftest_harness.h" +#include "clone3_selftests.h" + +#ifndef MAX_PID_NS_LEVEL +#define MAX_PID_NS_LEVEL 32 +#endif + +static void child_exit(int ret) +{ + fflush(stdout); + fflush(stderr); + _exit(ret); +} + +static int call_clone3_set_tid(struct __test_metadata *_metadata, + pid_t *set_tid, size_t set_tid_size) +{ + int status; + pid_t pid = -1; + + struct clone_args args = { + .exit_signal = SIGCHLD, + .set_tid = ptr_to_u64(set_tid), + .set_tid_size = set_tid_size, + }; + + pid = sys_clone3(&args, sizeof(struct clone_args)); + if (pid < 0) { + TH_LOG("%s - Failed to create new process", strerror(errno)); + return -errno; + } + + if (pid == 0) { + int ret; + char tmp = 0; + + TH_LOG("I am the child, my PID is %d (expected %d)", getpid(), set_tid[0]); + + if (set_tid[0] != getpid()) + child_exit(EXIT_FAILURE); + child_exit(EXIT_SUCCESS); + } + + TH_LOG("I am the parent (%d). My child's pid is %d", getpid(), pid); + + if (waitpid(pid, &status, 0) < 0) { + TH_LOG("Child returned %s", strerror(errno)); + return -errno; + } + + if (!WIFEXITED(status)) + return -1; + + return WEXITSTATUS(status); +} + +static int test_clone3_set_tid(struct __test_metadata *_metadata, + pid_t *set_tid, size_t set_tid_size) +{ + int ret; + + TH_LOG("[%d] Trying clone3() with CLONE_SET_TID to %d", getpid(), set_tid[0]); + ret = call_clone3_set_tid(_metadata, set_tid, set_tid_size); + TH_LOG("[%d] clone3() with CLONE_SET_TID %d says:%d", getpid(), set_tid[0], ret); + return ret; +} + +struct libcap { + struct __user_cap_header_struct hdr; + struct __user_cap_data_struct data[2]; +}; + +static int set_capability(void) +{ + cap_value_t cap_values[] = { CAP_SETUID, CAP_SETGID }; + struct libcap *cap; + int ret = -1; + cap_t caps; + + caps = cap_get_proc(); + if (!caps) { + perror("cap_get_proc"); + return -1; + } + + /* Drop all capabilities */ + if (cap_clear(caps)) { + perror("cap_clear"); + goto out; + } + + cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET); + cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET); + + cap = (struct libcap *) caps; + + /* 40 -> CAP_CHECKPOINT_RESTORE */ + cap->data[1].effective |= 1 << (40 - 32); + cap->data[1].permitted |= 1 << (40 - 32); + + if (cap_set_proc(caps)) { + perror("cap_set_proc"); + goto out; + } + ret = 0; +out: + if (cap_free(caps)) + perror("cap_free"); + return ret; +} + +TEST(clone3_cap_checkpoint_restore) +{ + pid_t pid; + int status; + int ret = 0; + pid_t set_tid[1]; + + test_clone3_supported(); + + EXPECT_EQ(getuid(), 0) + XFAIL(return, "Skipping all tests as non-root\n"); + + memset(&set_tid, 0, sizeof(set_tid)); + + /* Find the current active PID */ + pid = fork(); + if (pid == 0) { + TH_LOG("Child has PID %d", getpid()); + child_exit(EXIT_SUCCESS); + } + ASSERT_GT(waitpid(pid, &status, 0), 0) + TH_LOG("Waiting for child %d failed", pid); + + /* After the child has finished, its PID should be free. */ + set_tid[0] = pid; + + ASSERT_EQ(set_capability(), 0) + TH_LOG("Could not set CAP_CHECKPOINT_RESTORE"); + + ASSERT_EQ(prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0), 0); + + EXPECT_EQ(setgid(65534), 0) + TH_LOG("Failed to setgid(65534)"); + ASSERT_EQ(setuid(65534), 0); + + set_tid[0] = pid; + /* This would fail without CAP_CHECKPOINT_RESTORE */ + ASSERT_EQ(test_clone3_set_tid(_metadata, set_tid, 1), -EPERM); + ASSERT_EQ(set_capability(), 0) + TH_LOG("Could not set CAP_CHECKPOINT_RESTORE"); + /* This should work as we have CAP_CHECKPOINT_RESTORE as non-root */ + ASSERT_EQ(test_clone3_set_tid(_metadata, set_tid, 1), 0); +} + +TEST_HARNESS_MAIN From ad10207c9d778d3ee766bf987fed03cde8a65408 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Fri, 8 Dec 2023 15:36:56 +0100 Subject: [PATCH 13/13] UPSTREAM: tty: allow TIOCSLCKTRMIOS with CAP_CHECKPOINT_RESTORE [ Upstream commit e0f25b8992345aa5f113da2815f5add98738c611 ] The capability CAP_CHECKPOINT_RESTORE was introduced to allow non-root users to checkpoint and restore processes as non-root with CRIU. This change extends CAP_CHECKPOINT_RESTORE to enable the CRIU option '--shell-job' as non-root. CRIU's man-page describes the '--shell-job' option like this: Allow one to dump shell jobs. This implies the restored task will inherit session and process group ID from the criu itself. This option also allows to migrate a single external tty connection, to migrate applications like top. TIOCSLCKTRMIOS can only be done if the process has CAP_SYS_ADMIN and this change extends it to CAP_SYS_ADMIN or CAP_CHECKPOINT_RESTORE. With this change it is possible to checkpoint and restore processes which have a tty connection as non-root if CAP_CHECKPOINT_RESTORE is set. Acked-by: Christian Brauner Change-Id: I079e010f7ab7c1dc2e829bfa4023405efd5aada8 Signed-off-by: Adrian Reber Acked-by: Andrei Vagin Link: https://lore.kernel.org/r/20231208143656.1019-1-areber@redhat.com Signed-off-by: Greg Kroah-Hartman Signed-off-by: Sasha Levin Signed-off-by: ralph950412 --- drivers/tty/tty_ioctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/tty_ioctl.c b/drivers/tty/tty_ioctl.c index 5d833f0b412d..2fbd991f5e34 100644 --- a/drivers/tty/tty_ioctl.c +++ b/drivers/tty/tty_ioctl.c @@ -807,7 +807,7 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file, ret = -EFAULT; return ret; case TIOCSLCKTRMIOS: - if (!capable(CAP_SYS_ADMIN)) + if (!checkpoint_restore_ns_capable(&init_user_ns)) return -EPERM; copy_termios_locked(real_tty, &kterm); if (user_termios_to_kernel_termios(&kterm, @@ -824,7 +824,7 @@ int tty_mode_ioctl(struct tty_struct *tty, struct file *file, ret = -EFAULT; return ret; case TIOCSLCKTRMIOS: - if (!capable(CAP_SYS_ADMIN)) + if (!checkpoint_restore_ns_capable(&init_user_ns)) return -EPERM; copy_termios_locked(real_tty, &kterm); if (user_termios_to_kernel_termios_1(&kterm,