Merge pull request #35 from castai/filipe/LIVE-600-aptos

fix: aptos thread with a ghost file

Author: fals

Makefile

@@ -421,6 +421,24 @@ docker-test:
 		./test/zdtm.py run -a --keep-going --ignore-taint
 .PHONY: docker-test
 
+#
+# CastAI custom targets
+#
+# castai-test-build: build the Docker image with CRIU compiled.
+#   Only needed when CRIU source (not test) code changes.
+# castai-test: run the full ZDTM test suite inside Docker.
+#   Volume-mounts test/ so test code changes take effect immediately
+#   without rebuilding the image (zdtm.py compiles tests on the fly).
+castai-test-build:
+	docker build -t criu-test -f test/Dockerfile .
+.PHONY: castai-test-build
+
+castai-test: castai-test-build
+	docker run --rm --privileged --cgroupns=host -v /lib/modules:/lib/modules \
+		-v $(CURDIR)/test:/criu/test \
+		criu-test run -a -p 4 --keep-going --ignore-taint
+.PHONY: castai-test
+
 help:
 	@echo '    Targets:'
 	@echo '      all             - Build all [*] targets'
@@ -439,6 +457,7 @@ help:
 	@echo '      test            - Run zdtm test-suite'
 	@echo '      gcov            - Make code coverage report'
 	@echo '      unittest        - Run unit tests'
+	@echo '      castai-test     - Build and run all ZDTM tests in Docker'
 	@echo '      lint            - Run code linters'
 	@echo '      indent          - Indent C code'
 	@echo '      amdgpu_plugin   - Make AMD GPU plugin'

criu/cr-restore.c

@@ -272,16 +272,19 @@ static int crtools_prepare_shared(void)
  */
 
 static struct collect_image_info *cinfos[] = {
-	&file_locks_cinfo,  &pipe_data_cinfo, &fifo_data_cinfo, &sk_queues_cinfo,
+	&file_locks_cinfo,
+	&pipe_data_cinfo,
+	&fifo_data_cinfo,
+	&sk_queues_cinfo,
 #ifdef CONFIG_HAS_LIBBPF
 	&bpfmap_data_cinfo,
 #endif
 };
 
 static struct collect_image_info *cinfos_files[] = {
-	&unix_sk_cinfo,	      &fifo_cinfo,     &pipe_cinfo,    &nsfile_cinfo,	    &packet_sk_cinfo,
-	&netlink_sk_cinfo,    &eventfd_cinfo,  &epoll_cinfo,   &epoll_tfd_cinfo,    &signalfd_cinfo,
-	&tunfile_cinfo,	      &timerfd_cinfo,  &inotify_cinfo, &inotify_mark_cinfo, &fanotify_cinfo,
+	&unix_sk_cinfo, &fifo_cinfo, &pipe_cinfo, &nsfile_cinfo, &packet_sk_cinfo,
+	&netlink_sk_cinfo, &eventfd_cinfo, &epoll_cinfo, &epoll_tfd_cinfo, &signalfd_cinfo,
+	&tunfile_cinfo, &timerfd_cinfo, &inotify_cinfo, &inotify_mark_cinfo, &fanotify_cinfo,
 	&fanotify_mark_cinfo, &ext_file_cinfo, &memfd_cinfo, &pidfd_cinfo
 };
 
@@ -500,6 +503,65 @@ static int collect_inotify_fds(struct task_restore_args *ta)
 	return 0;
 }
 
+static int collect_deferred_proc_fds(struct task_restore_args *ta)
+{
+	struct list_head *list = &rsti(current)->fds;
+	struct fdt *fdt = rsti(current)->fdt;
+	struct fdinfo_list_entry *fle;
+
+	/* Only the fdt owner restores fds */
+	if (fdt && fdt->pid != vpid(current))
+		return 0;
+
+	ta->deferred_fds = (struct deferred_proc_fd *)rst_mem_align_cpos(RM_PRIVATE);
+	ta->deferred_fds_n = 0;
+
+	list_for_each_entry(fle, list, ps_list) {
+		struct file_desc *d = fle->desc;
+		struct reg_file_info *rfi;
+		struct deferred_proc_fd *df;
+		char *orig_path;
+
+		if (d->ops->type != FD_TYPES__REG)
+			continue;
+
+		rfi = container_of(d, struct reg_file_info, d);
+		if (!rfi->deferred_thread_fd)
+			continue;
+
+		df = rst_mem_alloc(sizeof(*df), RM_PRIVATE);
+		if (!df)
+			return -1;
+
+		orig_path = rfi->orig_path;
+
+		/*
+		 * orig_path is "proc/<pid>/task/<tid>/...", strip "proc/"
+		 * prefix to get a path relative to /proc for
+		 * sys_openat(proc_fd, ...).
+		 */
+		if (strncmp(orig_path, "proc/", 5) == 0)
+			orig_path += 5;
+
+		if (strlen(orig_path) >= sizeof(df->path)) {
+			pr_err("Deferred proc path too long: %s\n", orig_path);
+			return -1;
+		}
+
+		df->target_fd = fle->fe->fd;
+		df->flags = rfi->rfe->flags;
+		df->pos = rfi->rfe->pos;
+		strncpy(df->path, orig_path, sizeof(df->path) - 1);
+		df->path[sizeof(df->path) - 1] = '\0';
+
+		ta->deferred_fds_n++;
+		pr_info("Collected deferred proc fd %d -> %s\n",
+			df->target_fd, df->path);
+	}
+
+	return 0;
+}
+
 static int open_core(int pid, CoreEntry **pcore)
 {
 	int ret;
@@ -676,6 +738,9 @@ static int restore_one_alive_task(int pid, CoreEntry *core)
 	if (collect_inotify_fds(ta) < 0)
 		return -1;
 
+	if (collect_deferred_proc_fds(ta) < 0)
+		return -1;
+
 	if (prepare_proc_misc(pid, core->tc, ta))
 		return -1;
 
@@ -1709,7 +1774,10 @@ static int restore_task_with_children(void *_arg)
 }
 
 int __attribute((weak)) arch_ptrace_restore(int pid, struct pstree_item *item);
-int arch_ptrace_restore(int pid, struct pstree_item *item) { return 0; }
+int arch_ptrace_restore(int pid, struct pstree_item *item)
+{
+	return 0;
+}
 
 static int attach_to_tasks(bool root_seized)
 {
@@ -3133,7 +3201,9 @@ static void *restorer_munmap_addr(CoreEntry *core, void *restorer_blob)
 }
 
 void arch_rsti_init(struct pstree_item *p) __attribute__((weak));
-void arch_rsti_init(struct pstree_item *p) {}
+void arch_rsti_init(struct pstree_item *p)
+{
+}
 
 static int sigreturn_restore(pid_t pid, struct task_restore_args *task_args, unsigned long alen, CoreEntry *core)
 {
@@ -3329,6 +3399,7 @@ static int sigreturn_restore(pid_t pid, struct task_restore_args *task_args, uns
 	RST_MEM_FIXUP_PPTR(task_args->zombies);
 	RST_MEM_FIXUP_PPTR(task_args->vma_ios);
 	RST_MEM_FIXUP_PPTR(task_args->inotify_fds);
+	RST_MEM_FIXUP_PPTR(task_args->deferred_fds);
 
 	task_args->compatible_mode = core_is_compat(core);
 	/*

criu/files-reg.c

@@ -2552,8 +2552,28 @@ int collect_filemap(struct vma_area *vma)
 
 static int open_fe_fd(struct file_desc *fd, int *new_fd)
 {
+	struct reg_file_info *rfi;
 	int tmp;
 
+	rfi = container_of(fd, struct reg_file_info, d);
+
+	if (rfi->deferred_thread_fd) {
+		/*
+		 * This is a live-thread /proc/<pid>/task/<tid>/... fd.
+		 * The thread doesn't exist yet (created later by the
+		 * restorer via clone()), so open /dev/null as a
+		 * placeholder. The restorer will reopen the correct
+		 * path after threads are created.
+		 */
+		tmp = open("/dev/null", rfi->rfe->flags & O_ACCMODE);
+		if (tmp < 0) {
+			pr_perror("Can't open /dev/null for deferred proc fd");
+			return -1;
+		}
+		*new_fd = tmp;
+		return 0;
+	}
+
 	tmp = open_path(fd, do_open_reg, NULL);
 	if (tmp < 0)
 		return -1;
@@ -2596,6 +2616,121 @@ struct file_desc *try_collect_special_file(u32 id, int optional)
 	return fdesc;
 }
 
+/*
+ * On restore, fix up paths like proc/<pid>/task/<tid>/... where
+ * tid != pid. These paths can't be opened at prepare_fds() time
+ * because non-leader threads don't exist yet -- they are created
+ * later by the restorer blob via clone(). Two cases:
+ *
+ * 1) Dead thread (tid not in pstree): create a TASK_HELPER process
+ *    with vPID=tid and rewrite the path to proc/<tid>/task/<tid>/...
+ *    so it resolves through the helper's /proc entry.
+ *
+ * 2) Live thread (tid in pstree as TASK_THREAD): rewrite the path
+ *    to proc/<pid>/task/<pid>/... so it points to the thread-group
+ *    leader, which does exist at prepare_fds() time.
+ */
+static int fixup_thread_proc_path(struct reg_file_info *rfi)
+{
+	char *path = rfi->path;
+	char *task_str, *tid_str, *tid_end;
+	pid_t pid, tid;
+	char *new_path;
+	struct pid *tid_node;
+
+	/*
+	 * rfi->path looks like "proc/<pid>/task/<tid>/..." (leading
+	 * slash already stripped). We only care about procfs paths.
+	 */
+	if (strncmp(path, "proc/", 5))
+		return 0;
+
+	/* Parse the pid: "proc/<pid>/task/..." */
+	pid = strtol(path + 5, &task_str, 10);
+	if (pid == 0 || *task_str != '/')
+		return 0;
+
+	/* Check for "/task/<tid>" */
+	if (strncmp(task_str, "/task/", 6))
+		return 0;
+
+	tid_str = task_str + 6;
+	tid = strtol(tid_str, &tid_end, 10);
+	if (tid == 0 || (*tid_end != '/' && *tid_end != '\0'))
+		return 0;
+
+	/* If pid == tid the path already refers to the leader */
+	if (pid == tid)
+		return 0;
+
+	tid_node = pstree_pid_by_virt(tid);
+
+	/*
+	 * If the TID belongs to a process (not a thread), the path
+	 * will resolve on restore without any fixup.
+	 */
+	if (tid_node && tid_node->state != TASK_THREAD)
+		return 0;
+
+	if (!tid_node) {
+		/*
+		 * Dead thread: tid is not in the process tree.
+		 * Create a TASK_HELPER with vPID=tid so that
+		 * /proc/<tid>/task/<tid>/... resolves on restore.
+		 */
+		struct pstree_item *helper;
+
+		helper = lookup_create_item(tid);
+		if (!helper)
+			return -1;
+
+		if (helper->pid->state == TASK_UNDEF) {
+			helper->sid = root_item->sid;
+			helper->pgid = root_item->pgid;
+			helper->pid->ns[0].virt = tid;
+			helper->parent = root_item;
+			helper->ids = root_item->ids;
+			if (init_pstree_helper(helper)) {
+				pr_err("Can't init helper for dead thread %d\n", tid);
+				return -1;
+			}
+			list_add_tail(&helper->sibling, &root_item->children);
+			pr_info("Added a helper for restoring dead thread /proc/%d/task/%d\n",
+				pid, tid);
+		}
+
+		/*
+		 * "proc/" + tid + "/task/" + tid + tid_end + '\0'
+		 * Use PATH_MAX as a safe upper bound.
+		 */
+		new_path = xmalloc(PATH_MAX);
+		if (!new_path)
+			return -1;
+
+		snprintf(new_path, PATH_MAX, "proc/%d/task/%d%s",
+			 tid, tid, tid_end);
+		pr_info("Rewrote dead thread path: %s -> %s\n",
+			rfi->path, new_path);
+	} else {
+		/*
+		 * Live thread: the thread exists in the pstree but
+		 * won't be created until the restorer blob runs
+		 * clone(), which is after prepare_fds(). Mark this
+		 * file as deferred -- open_fe_fd() will open /dev/null
+		 * as a placeholder, and the restorer will reopen the
+		 * correct path after threads exist.
+		 */
+		rfi->deferred_thread_fd = true;
+		rfi->orig_path = rfi->path;
+		pr_info("Deferred live thread proc path: %s\n", rfi->path);
+		return 0;
+	}
+
+	rfi->path = new_path;
+
+	return 0;
+}
+
 static int collect_one_regfile(void *o, ProtobufCMessage *base, struct cr_img *i)
 {
 	struct reg_file_info *rfi = o;
@@ -2609,6 +2744,11 @@ static int collect_one_regfile(void *o, ProtobufCMessage *base, struct cr_img *i
 		rfi->path = rfi->rfe->name + 1;
 	rfi->remap = NULL;
 	rfi->size_mode_checked = false;
+	rfi->deferred_thread_fd = false;
+	rfi->orig_path = NULL;
+
+	if (fixup_thread_proc_path(rfi))
+		return -1;
 
 	pr_info("Collected [%s] ID %#x\n", rfi->path, rfi->rfe->id);
 	return file_desc_add(&rfi->d, rfi->rfe->id, &reg_desc_ops);

criu/include/files-reg.h

@@ -24,7 +24,9 @@ struct reg_file_info {
 	struct file_remap *remap;
 	bool size_mode_checked;
 	bool is_dir;
+	bool deferred_thread_fd; /* placeholder for live thread proc fd */
 	char *path;
+	char *orig_path; /* original proc path for deferred reopen */
 };
 
 extern int open_reg_by_id(u32 id);

criu/include/restorer.h

@@ -141,6 +141,13 @@ struct restore_vma_io {
 
 #define RIO_SIZE(niovs) (sizeof(struct restore_vma_io) + (niovs) * sizeof(struct iovec))
 
+struct deferred_proc_fd {
+	int target_fd;	/* fd number to dup2 over */
+	int flags;	/* open flags (O_RDONLY etc) */
+	off_t pos;	/* file position to restore */
+	char path[128]; /* path relative to /proc, e.g. "1/task/7/stat" */
+};
+
 struct task_restore_args {
 	struct thread_restore_args *t; /* thread group leader */
 
@@ -196,6 +203,9 @@ struct task_restore_args {
 	int *inotify_fds; /* fds to cleanup inotify events at CR_STATE_RESTORE_SIGCHLD stage */
 	unsigned int inotify_fds_n;
 
+	struct deferred_proc_fd *deferred_fds; /* live-thread proc fds to reopen after clone() */
+	unsigned int deferred_fds_n;
+
 	/* * * * * * * * * * * * * * * * * * * * */
 
 	unsigned long task_size;