diff --git a/criu/criu/cr-restore.c b/criu/criu/cr-restore.c index aef0ed53b..4f9b96026 100644 --- a/criu/criu/cr-restore.c +++ b/criu/criu/cr-restore.c @@ -581,7 +581,29 @@ static int populate_pid_proc(void) return 0; } -static int __collect_child_pids(struct pstree_item *p, int state, unsigned int *n) +/* + * The pid of `pi` as seen by `p`. ns[0] is innermost, so stepping in + * (child depth - p's depth) entries gives p's view. localpid() only + * matches when both live in the same namespace: a nested pidns init is 1 + * to itself and something else entirely to its parent. + */ +static pid_t pid_in_parent_ns(struct pstree_item *p, struct pstree_item *pi) +{ + int idx = pi->pid->ns_level - p->pid->ns_level; + + if (idx <= 0 || idx >= pi->pid->ns_level) + return localpid(pi); + + return pi->pid->ns[idx].ns_pid; +} + +/* + * `observer` is whoever waits on these children, which is not always p: + * children of helpers and zombies are reparented to init. Pids are + * recorded in the observer's namespace. + */ +static int __collect_child_pids(struct pstree_item *p, struct pstree_item *observer, int state, + unsigned int *n) { struct pstree_item *pi; @@ -596,7 +618,15 @@ static int __collect_child_pids(struct pstree_item *p, int state, unsigned int * return -1; (*n)++; - *child = localpid(pi); + /* + * Zombies only. A helper's pid chain is synthesised by + * get_or_create_helper_item(), so translating it through ns[] + * would yield a pid that was never real. + */ + if (state == TASK_DEAD) + *child = pid_in_parent_ns(observer, pi); + else + *child = localpid(pi); } return 0; @@ -617,12 +647,12 @@ static int collect_child_pids(int state, unsigned int *n) for_each_pstree_item(pi) { if (pi->pid->state != TASK_HELPER && pi->pid->state != TASK_DEAD) continue; - if (__collect_child_pids(pi, state, n)) + if (__collect_child_pids(pi, current, state, n)) return -1; } } - return __collect_child_pids(current, state, n); + return __collect_child_pids(current, current, state, n); } static int collect_helper_pids(struct task_restore_args *ta) @@ -1546,11 +1576,18 @@ static inline int fork_with_pid_mode(struct pstree_item *item, bool parallel_sib ca.item = item; ca.clone_flags = rsti(item)->clone_flags; - if (opts.tfork.active && item != root_item && + /* + * Innermost pid 1 below the restore root means a nested pidns init, + * which needs CLONE_NEWPID. A zombie has no ids image, so + * get_clone_mask() cannot derive the flag and clone3() would apply + * the set_tid chain against the parent's namespace and hit EEXIST. + * Not tfork-specific: plain dump/restore breaks the same way. + */ + if (item != root_item && !(ca.clone_flags & CLONE_NEWPID) && item->pid->ns_level > 1 && item->pid->ns[0].ns_pid == INIT_PID) { - pr_info("tfork: repairing missing CLONE_NEWPID for pidns init uid=%d local=%d parent_local=%d level=%d\n", + pr_info("repairing missing CLONE_NEWPID for pidns init uid=%d local=%d parent_local=%d level=%d\n", uid(item), pid, item->parent ? localpid(item->parent) : -1, item->pid->ns_level); @@ -1606,45 +1643,13 @@ static inline int fork_with_pid_mode(struct pstree_item *item, bool parallel_sib syscall_clone_flags |= CLONE_PARENT; if (kdat.has_clone3_set_tid) { - if (opts.tfork.active && (syscall_clone_flags & CLONE_NEWPID)) { - pr_info("tfork: restore pidns init uid=%d local pid %d with fresh parent pid, dumped chain level=%d\n", - uid(item), pid, item->pid->ns_level); + if (item->pid->ns_level == 1) ret = clone3_with_pid_noasan(restore_task_with_children, &ca, syscall_clone_flags & ~strip, SIGCHLD, pid); - } else if (item->pid->ns_level == 1) - ret = clone3_with_pid_noasan(restore_task_with_children, &ca, - syscall_clone_flags & ~strip, SIGCHLD, pid); - else { - struct pid tfork_pid = {}; - struct pid *restore_pid = item->pid; - - if (opts.tfork.active && (root_ns_mask & CLONE_NEWPID) && - root_item && root_item->pid->ns_level > 1 && - item->pid->ns_level > 1) { - /* - * Copy only scalar pid identity. struct pid also - * embeds rb_node links owned by the dumped pid trees; - * copying those nodes into a temporary stack object - * corrupts the tree metadata if it ever gets reused. - */ - tfork_pid.item = item->pid->item; - tfork_pid.real = item->pid->real; - tfork_pid.local = item->pid->local; - tfork_pid.uid = item->pid->uid; - tfork_pid.state = item->pid->state; - tfork_pid.stop_signo = item->pid->stop_signo; - tfork_pid.ns_level = item->pid->ns_level - 1; - tfork_pid.leaf_ns_id = item->pid->leaf_ns_id; - memcpy(tfork_pid.ns, item->pid->ns, sizeof(tfork_pid.ns)); - restore_pid = &tfork_pid; - pr_info("tfork: restore pid uid=%d local=%d with rebased pid chain level %d -> %d\n", - uid(item), pid, item->pid->ns_level, - restore_pid->ns_level); - } + else ret = clone3_with_nested_pid_noasan(restore_task_with_children, &ca, syscall_clone_flags & ~strip, - SIGCHLD, restore_pid); - } + SIGCHLD, item->pid); } else { BUG_ON(item->pid->ns_level >= 1); close_pid_proc(); @@ -1671,7 +1676,8 @@ static inline int fork_with_pid_mode(struct pstree_item *item, bool parallel_sib goto err_unlock; } - if (opts.tfork.active || item == root_item) { + if (item == root_item) { + /* Non-root tasks publish their real PID from __restore_task_with_children(). */ item->pid->real = ret; pr_debug("PID: real %d virt %d\n", item->pid->real, localpid(item)); } diff --git a/criu/criu/cr-tfork.c b/criu/criu/cr-tfork.c index 843344eac..f07ce9440 100644 --- a/criu/criu/cr-tfork.c +++ b/criu/criu/cr-tfork.c @@ -1140,7 +1140,7 @@ int cr_tfork_tasks(pid_t pid) list_for_each_entry(cgo_iter, &opts.new_cgroup_roots, node) rpc_n_cg_root++; - rpc_max = 33 + 2 * (rpc_n_ifd + rpc_n_ext + rpc_n_cg_root + rpc_max = 32 + 2 * (rpc_n_ifd + rpc_n_ext + rpc_n_cg_root + opts.tfork.snap_mount_n) + rpc_n_copy_args + 2; rpc_argv = calloc(rpc_max, sizeof(*rpc_argv)); @@ -1160,7 +1160,6 @@ int cr_tfork_tasks(pid_t pid) rpc_argv[rpc_n++] = "-o"; rpc_argv[rpc_n++] = restore_log_arg; rpc_argv[rpc_n++] = "-v2"; - rpc_argv[rpc_n++] = "--keep-pid-hierarchy"; if (opts.root) { rpc_argv[rpc_n++] = "--root"; @@ -1330,7 +1329,7 @@ int cr_tfork_tasks(pid_t pid) buf[off] = '\0'; end = buf + off; - argv_max = 8 + 3; + argv_max = 8 + 2; for (p = buf; p < end; p++) if (*p == '\0') argv_max++; @@ -1375,11 +1374,10 @@ int cr_tfork_tasks(pid_t pid) argv_new[argc_new++] = "--pidfile"; argv_new[argc_new++] = pidfile_arg; } - if ((size_t)argc_new + 1 >= argv_max) { + if ((size_t)argc_new >= argv_max) { pr_err("tfork restore argv overflow: used=%d max=%zu\n", argc_new, argv_max); exit(1); } - argv_new[argc_new++] = "--keep-pid-hierarchy"; argv_new[argc_new] = NULL; execv("/proc/self/exe", argv_new); diff --git a/criu/criu/pie/restorer.c b/criu/criu/pie/restorer.c index a63dbdd62..8e49872c7 100644 --- a/criu/criu/pie/restorer.c +++ b/criu/criu/pie/restorer.c @@ -2485,18 +2485,6 @@ __visible long __export_restore_task(struct task_restore_args *args) c_args.set_tid = ptr_to_u64(thread_args[i].tid_in_ns); c_args.flags = clone_flags; c_args.set_tid_size = thread_args[i].ns_level; - if (args->tfork_active && thread_args[i].ns_level > 0) { - /* - * Preserve the TID visible in the clone's innermost PID namespace. - * Outer namespace TIDs are allocated by the kernel so concurrent - * copy helpers cannot collide with each other on the host. - */ - pr_debug("tfork: restore thread pid=%d with innermost tid=%d, set_tid_size %d -> 1\n", - thread_args[i].pid, - thread_args[i].tid_in_ns[0], - thread_args[i].ns_level); - c_args.set_tid_size = 1; - } /* The kernel does stack + stack_size. */ c_args.stack = new_sp - RESTORE_STACK_SIZE; c_args.stack_size = RESTORE_STACK_SIZE; diff --git a/criu/test/others/tfork-ncopy-nested-pidns-threads.c b/criu/test/others/tfork-ncopy-nested-pidns-threads.c new file mode 100644 index 000000000..23cfd5f26 --- /dev/null +++ b/criu/test/others/tfork-ncopy-nested-pidns-threads.c @@ -0,0 +1,154 @@ +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define READY_PATH "/tmp/tfork-ncopy-nested-pidns-ready" + +static void set_name(const char *name) +{ + if (prctl(PR_SET_NAME, name, 0, 0, 0)) { + perror("prctl(PR_SET_NAME)"); + exit(1); + } +} + +static void unblock_signals(void) +{ + sigset_t mask; + int ret; + + sigemptyset(&mask); + ret = pthread_sigmask(SIG_SETMASK, &mask, NULL); + if (ret) { + errno = ret; + perror("pthread_sigmask"); + exit(1); + } +} + +static void park(void) +{ + for (;;) + pause(); +} + +static void *thread_main(void *arg) +{ + (void)arg; + set_name("tfork-thread"); + park(); + return NULL; +} + +static void spawn_siblings(int count) +{ + int i; + + for (i = 0; i < count; i++) { + pid_t pid = fork(); + + if (pid < 0) { + perror("fork sibling"); + exit(1); + } + if (pid == 0) { + set_name("tfork-sibling"); + park(); + _exit(0); + } + } +} + +static void nested_pidns_helper(int thread_count) +{ + pid_t nested_init; + int status; + + set_name("tfork-helper"); + if (unshare(CLONE_NEWPID)) { + perror("unshare(CLONE_NEWPID)"); + exit(1); + } + + nested_init = fork(); + if (nested_init < 0) { + perror("fork nested init"); + exit(1); + } + if (nested_init == 0) { + pthread_t *threads; + int fd, i; + + set_name("tfork-ns-init"); + threads = calloc(thread_count, sizeof(*threads)); + if (!threads) { + perror("calloc threads"); + _exit(1); + } + for (i = 0; i < thread_count; i++) { + int ret = pthread_create(&threads[i], NULL, thread_main, NULL); + + if (ret) { + errno = ret; + perror("pthread_create"); + _exit(1); + } + } + + fd = open(READY_PATH, O_WRONLY | O_CREAT | O_TRUNC, 0644); + if (fd < 0) { + perror("open ready file"); + _exit(1); + } + dprintf(fd, "pid=%d threads=%d\n", getpid(), thread_count + 1); + close(fd); + park(); + _exit(0); + } + + if (waitpid(nested_init, &status, 0) != nested_init) + perror("waitpid nested init"); + exit(1); +} + +int main(int argc, char **argv) +{ + int siblings, threads; + pid_t helper; + + if (argc != 3) { + fprintf(stderr, "usage: %s SIBLINGS THREADS\n", argv[0]); + return 2; + } + siblings = atoi(argv[1]); + threads = atoi(argv[2]); + if (siblings < 1 || threads < 1) + return 2; + + /* CRIU's parasite bootstrap must be able to deliver its SIGTRAP. */ + unblock_signals(); + set_name("tfork-test-root"); + spawn_siblings(siblings); + helper = fork(); + if (helper < 0) { + perror("fork helper"); + return 1; + } + if (helper == 0) + nested_pidns_helper(threads); + + while (access(READY_PATH, F_OK)) + usleep(10000); + park(); + return 0; +} diff --git a/criu/test/others/tfork-ncopy-nested-pidns-threads.sh b/criu/test/others/tfork-ncopy-nested-pidns-threads.sh new file mode 100755 index 000000000..24c2623e6 --- /dev/null +++ b/criu/test/others/tfork-ncopy-nested-pidns-threads.sh @@ -0,0 +1,120 @@ +#!/usr/bin/env bash +set -euo pipefail + +PODMAN=${PODMAN:-podman} +PODMAN_GLOBAL_ARGS=${PODMAN_GLOBAL_ARGS:-} +OS4AGENT_CRUN=${OS4AGENT_CRUN:-crun} +CC=${CC:-cc} +IMAGE=${IMAGE:-docker.io/library/alpine:3.19} +COPIES=${COPIES:-4} +ITERATIONS=${ITERATIONS:-3} +SIBLINGS=${SIBLINGS:-40} +THREADS=${THREADS:-8} +PARALLEL_WORKERS=${PARALLEL_WORKERS:-4} +PREFIX=${PREFIX:-tfork-ncopy-pidns-$RANDOM} +TFORK_CLONE_ARGS=${TFORK_CLONE_ARGS:---tfork-overlay-btrfs} + +read -r -a podman_global_args <<<"$PODMAN_GLOBAL_ARGS" +read -r -a tfork_clone_args <<<"$TFORK_CLONE_ARGS" +script_dir=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +workload_src=$script_dir/tfork-ncopy-nested-pidns-threads.c +work_dir=$(mktemp -d /tmp/tfork-ncopy-pidns.XXXXXX) +workload=$work_dir/tfork-ncopy-workload +source_name=${PREFIX}-source + +podman_cmd() { + "$PODMAN" "${podman_global_args[@]}" "$@" +} + +cleanup_containers() { + podman_cmd ps -a --format '{{.Names}}' | + awk -v prefix="$PREFIX" 'index($0, prefix) == 1' | + while read -r name; do + podman_cmd rm -f -t 0 "$name" >/dev/null 2>&1 || true + done +} + +cleanup() { + cleanup_containers + rm -rf "$work_dir" +} +trap cleanup EXIT + +check_nested_init() { + local name=$1 + podman_cmd exec \ + -e TFORK_EXPECT_THREADS="$((THREADS + 1))" \ + "$name" sh -eu -c ' +found=0 +for status in /proc/[0-9]*/status; do + name_value= + nspid= + threads= + while IFS=: read -r key value; do + case "$key" in + Name) set -- $value; name_value=$1 ;; + NSpid) nspid=$value ;; + Threads) set -- $value; threads=$1 ;; + esac + done <"$status" + [ "$name_value" = tfork-ns-init ] || continue + set -- $nspid + [ "$#" -ge 2 ] + for last do :; done + [ "$last" -eq 1 ] + [ "$threads" -eq "$TFORK_EXPECT_THREADS" ] + found=$((found + 1)) + echo "nested-init nspid=$nspid threads=$threads" +done +[ "$found" -eq 1 ] +' +} + +cleanup_containers +test -f "$workload_src" +"$CC" -O2 -Wall -Wextra -pthread -static "$workload_src" -o "$workload" +podman_cmd run -d --name "$source_name" \ + --log-driver k8s-file \ + --cap-add SYS_ADMIN \ + --security-opt seccomp=unconfined \ + --security-opt apparmor=unconfined \ + -v "$workload:/tfork-ncopy-workload:ro" \ + "$IMAGE" /tfork-ncopy-workload "$SIBLINGS" "$THREADS" >/dev/null + +for ((attempt = 0; attempt < 200; attempt++)); do + if podman_cmd exec "$source_name" \ + test -f /tmp/tfork-ncopy-nested-pidns-ready; then + break + fi + sleep 0.05 +done +podman_cmd exec "$source_name" test -f /tmp/tfork-ncopy-nested-pidns-ready +check_nested_init "$source_name" + +for ((iteration = 1; iteration <= ITERATIONS; iteration++)); do + clone_base=${PREFIX}-clone-${iteration} + env \ + CRIU_TFORK_PARALLEL_SIBLINGS="$PARALLEL_WORKERS" \ + OS4AGENT_CRUN="$OS4AGENT_CRUN" \ + "$PODMAN" "${podman_global_args[@]}" container clone \ + --live --copies "$COPIES" "${tfork_clone_args[@]}" \ + "$source_name" "$clone_base" >/dev/null + + for ((copy = 0; copy < COPIES; copy++)); do + name=${clone_base}-${copy} + check_nested_init "$name" + done + + rootfs=$(podman_cmd inspect --format '{{.Rootfs}}' "${clone_base}-0") + bundle=$(dirname "$rootfs") + grep -Eq 'tfork: creating [0-9]+ .*siblings with [0-9]+ temporary helpers' \ + "$bundle/img/tfork-restore.log.copy0" + + for ((copy = 0; copy < COPIES; copy++)); do + podman_cmd rm -f -t 0 "${clone_base}-${copy}" >/dev/null + done + printf 'iteration %d: %d concurrent copies passed\n' "$iteration" "$COPIES" +done + +podman_cmd rm -f -t 0 "$source_name" >/dev/null +trap - EXIT diff --git a/criu/test/zdtm/static/Makefile b/criu/test/zdtm/static/Makefile index e1df2e5fa..3ec972f8f 100644 --- a/criu/test/zdtm/static/Makefile +++ b/criu/test/zdtm/static/Makefile @@ -11,6 +11,7 @@ TST_NOFILE := \ caps01 \ wait00 \ zombie00 \ + zombie_pidns_init \ zombie01 \ fpu00 \ fpu01 \ diff --git a/criu/test/zdtm/static/zombie_pidns_init.c b/criu/test/zdtm/static/zombie_pidns_init.c new file mode 100644 index 000000000..ca6acd5f1 --- /dev/null +++ b/criu/test/zdtm/static/zombie_pidns_init.c @@ -0,0 +1,66 @@ +#include +#include +#include +#include +#include + +#include "zdtmtst.h" + +const char *test_doc = "Check that an outer pid namespace can reap a restored zombie init"; +const char *test_author = "GenseeAI"; + +#define ZOMBIE_EXIT_CODE 42 + +int main(int argc, char **argv) +{ + siginfo_t info; + pid_t zombie; + int status; + + test_init(argc, argv); + + /* The next child becomes init (PID 1) of a nested PID namespace. */ + if (unshare(CLONE_NEWPID) < 0) { + pr_perror("Unable to create nested pid namespace"); + return 1; + } + + zombie = fork(); + if (zombie < 0) { + pr_perror("fork failed"); + return 1; + } + + if (zombie == 0) { + if (getpid() != 1) + _exit(1); + _exit(ZOMBIE_EXIT_CODE); + } + + /* Observe the exit without reaping it, so CRIU must restore the zombie. */ + if (waitid(P_PID, zombie, &info, WNOWAIT | WEXITED) < 0) { + pr_perror("Unable to observe zombie pid %d", zombie); + return 1; + } + if (info.si_pid != zombie || info.si_code != CLD_EXITED || + info.si_status != ZOMBIE_EXIT_CODE) { + fail("Unexpected pre-dump zombie status: pid=%d code=%d status=%d", + info.si_pid, info.si_code, info.si_status); + return 1; + } + + test_daemon(); + test_waitsig(); + + if (waitpid(zombie, &status, 0) != zombie) { + pr_perror("Unable to reap restored zombie pid %d", zombie); + return 1; + } + if (!WIFEXITED(status) || WEXITSTATUS(status) != ZOMBIE_EXIT_CODE) { + fail("Restored zombie has wrong status: %#x", status); + return 1; + } + + pass(); + return 0; +} diff --git a/criu/test/zdtm/static/zombie_pidns_init.desc b/criu/test/zdtm/static/zombie_pidns_init.desc new file mode 100644 index 000000000..7657ba45c --- /dev/null +++ b/criu/test/zdtm/static/zombie_pidns_init.desc @@ -0,0 +1 @@ +{'flavor': 'ns uns', 'flags': 'suid'}