[CRIU] [PATCH 8/9] seccomp: Dont forget to suspend filtering on threads

Cyrill Gorcunov gorcunov at gmail.com
Thu Apr 26 23:14:42 MSK 2018


When considering if we to call PTRACE_O_SUSPEND_SECCOMP
on the tid we should take into account if there at least
one thread which has seccomp mode enabled, otherwise
we might miss filter suspension and restore procedure
might break due to own criu syscall get filtered out.

Same time we should move seccomp restore for threads
to take place after CR_STATE_RESTORE_SIGCHLD state
so that main criu code will attach to threads and
setup seccomp suspension flag before we start
restoring the filters.

Reported-by: Andrei Vagin <avagin at virtuozzo.com>
Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
---
 criu/cr-restore.c   | 29 +++++++++++++++++++++++++----
 criu/pie/restorer.c | 21 ++++++++++-----------
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index 1404a3ee87c5..4a6ac97918df 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -914,6 +914,23 @@ static int open_cores(int pid, CoreEntry *leader_core)
 
 	current->core = cores;
 
+	/*
+	 * Walk over all threads and if one them is having
+	 * active seccomp mode we will suspend filtering
+	 * on the whole group until restore complete.
+	 *
+	 * Otherwise any criu code which might use same syscall
+	 * if present inside a filter chain would take filter
+	 * action and might break restore procedure.
+	 */
+	for (i = 0; i < current->nr_threads; i++) {
+		ThreadCoreEntry *thread_core = cores[i]->thread_core;
+		if (thread_core->seccomp_mode != SECCOMP_MODE_DISABLED) {
+			rsti(current)->has_seccomp = true;
+			break;
+		}
+	}
+
 	return 0;
 err:
 	xfree(cores);
@@ -1525,10 +1542,14 @@ static inline int fork_with_pid(struct pstree_item *item)
 			return -1;
 		}
 
-		if (item->pid->state != TASK_DEAD)
-			rsti(item)->has_seccomp = ca.core->thread_core->seccomp_mode != SECCOMP_MODE_DISABLED;
-		else
-			rsti(item)->has_seccomp = false;
+		/*
+		 * By default we assume that seccomp is not
+		 * used at all (especially on dead task). Later
+		 * we will walk over all threads and check in
+		 * details if filter is present setting up
+		 * this flag as appropriate.
+		 */
+		rsti(item)->has_seccomp = false;
 
 		if (unlikely(item == root_item))
 			maybe_clone_parent(item, &ca);
diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c
index 79a06d880f59..f06b44c8050f 100644
--- a/criu/pie/restorer.c
+++ b/criu/pie/restorer.c
@@ -574,17 +574,6 @@ long __export_restore_thread(struct thread_restore_args *args)
 		sys_close(fd);
 	}
 
-	/*
-	 * Make sure it's before creds, since it's privileged
-	 * operation bound to uid 0 in current user ns.
-	 */
-	if (restore_seccomp(args))
-		goto core_restore_end;
-
-	ret = restore_creds(args->creds_args, args->ta->proc_fd);
-	if (ret)
-		goto core_restore_end;
-
 	ret = restore_dumpable_flag(&args->ta->mm);
 	if (ret)
 		goto core_restore_end;
@@ -599,6 +588,16 @@ long __export_restore_thread(struct thread_restore_args *args)
 	restore_finish_stage(task_entries_local, CR_STATE_RESTORE_SIGCHLD);
 	restore_pdeath_sig(args);
 
+	/*
+	 * Make sure it's before creds, since it's privileged
+	 * operation bound to uid 0 in current user ns.
+	 */
+	if (restore_seccomp(args))
+		goto core_restore_end;
+
+	ret = restore_creds(args->creds_args, args->ta->proc_fd);
+	if (ret)
+		goto core_restore_end;
 	restore_finish_stage(task_entries_local, CR_STATE_RESTORE_CREDS);
 
 	futex_dec_and_wake(&thread_inprogress);
-- 
2.14.3



More information about the CRIU mailing list