[CRIU] [PATCH 08/10] seccomp: Dont forget to suspend filtering on threads

Cyrill Gorcunov gorcunov at gmail.com
Mon May 7 11:42:48 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>
Reviewed-by: Dmitry Safonov <0x7f454c46 at gmail.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 a7a232b2e028..89d73e191e97 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);
@@ -1531,10 +1548,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 2ba0bcf2f72f..44f8648319e1 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