[CRIU] [PATCH 4/6] creds: fail to dump when creds in thread group don't match
Tycho Andersen
tycho.andersen at canonical.com
Thu Jun 18 10:59:19 PDT 2015
Since we don't support dumping per-thread creds, let's at least fail to
dump if the creds don't match.
Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
---
cr-dump.c | 19 ++++++-------------
cr-exec.c | 10 +++++++++-
cr-restore.c | 4 ++--
include/proc_parse.h | 2 ++
include/pstree.h | 14 +++++++++-----
include/ptrace.h | 3 ++-
proc_parse.c | 5 +++++
ptrace.c | 22 ++++++++++++++++++----
8 files changed, 53 insertions(+), 26 deletions(-)
diff --git a/cr-dump.c b/cr-dump.c
index ad78998..acbbe3f 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -674,10 +674,10 @@ static int dump_task_core_all(struct pstree_item *item,
if (ret < 0)
goto err;
- if (item->seccomp_mode != SECCOMP_MODE_DISABLED) {
- pr_info("got seccomp mode %d for %d\n", item->seccomp_mode, item->pid.virt);
+ if (item->creds->seccomp_mode != SECCOMP_MODE_DISABLED) {
+ pr_info("got seccomp mode %d for %d\n", item->creds->seccomp_mode, item->pid.virt);
core->tc->has_seccomp_mode = true;
- core->tc->seccomp_mode = item->seccomp_mode;
+ core->tc->seccomp_mode = item->creds->seccomp_mode;
}
strncpy((char *)core->tc->comm, stat->comm, TASK_COMM_LEN);
@@ -809,7 +809,7 @@ static int collect_children(struct pstree_item *item)
goto free;
}
- ret = seize_task(pid, item->pid.real, &c->seccomp_mode);
+ ret = seize_task(pid, item->pid.real, &c->creds);
if (ret < 0) {
/*
* Here is a race window between parse_children() and seize(),
@@ -921,14 +921,7 @@ static int collect_threads(struct pstree_item *item)
pr_info("\tSeizing %d's %d thread\n",
item->pid.real, pid);
- /*
- * FIXME: The NULL here is wrong; we really want to get the
- * seccomp state of this thread, but we have nowhere to put it,
- * so for now we ignore it. We should at least check to see
- * that the seccomp state is the same for all threads; we'll do
- * this in a future series.
- */
- ret = seize_task(pid, item_ppid(item), NULL);
+ ret = seize_task(pid, item_ppid(item), &item->creds);
if (ret < 0) {
/*
* Here is a race window between parse_threads() and seize(),
@@ -1078,7 +1071,7 @@ static int collect_pstree(pid_t pid)
return -1;
root_item->pid.real = pid;
- ret = seize_task(pid, -1, &root_item->seccomp_mode);
+ ret = seize_task(pid, -1, &root_item->creds);
if (ret < 0)
goto err;
pr_info("Seized task %d, state %d\n", pid, ret);
diff --git a/cr-exec.c b/cr-exec.c
index 9d7162a..f3d55f6 100644
--- a/cr-exec.c
+++ b/cr-exec.c
@@ -117,6 +117,7 @@ int cr_exec(int pid, char **opt)
struct parasite_ctl *ctl;
struct vm_area_list vmas;
int ret = -1, prev_state;
+ struct proc_status_creds *creds;
if (!sys_name) {
pr_err("Syscall name required\n");
@@ -129,12 +130,19 @@ int cr_exec(int pid, char **opt)
goto out;
}
- prev_state = ret = seize_task(pid, -1, NULL);
+ prev_state = ret = seize_task(pid, -1, &creds);
if (ret < 0) {
pr_err("Can't seize task %d\n", pid);
goto out;
}
+ /*
+ * We don't seize a task's threads here, and there is no reason to
+ * compare threads' creds in this use case anyway, so let's just free
+ * the creds.
+ */
+ free(creds);
+
ret = collect_mappings(pid, &vmas);
if (ret) {
pr_err("Can't collect vmas for %d\n", pid);
diff --git a/cr-restore.c b/cr-restore.c
index f93c814..7372743 100644
--- a/cr-restore.c
+++ b/cr-restore.c
@@ -1062,7 +1062,7 @@ static inline int fork_with_pid(struct pstree_item *item)
item->state = ca.core->tc->task_state;
rsti(item)->cg_set = ca.core->tc->cg_set;
- item->seccomp_mode = ca.core->tc->seccomp_mode;
+ item->has_seccomp = ca.core->tc->seccomp_mode != SECCOMP_MODE_DISABLED;
if (item->state == TASK_DEAD)
rsti(item->parent)->nr_zombies++;
@@ -1693,7 +1693,7 @@ static void finalize_restore(int status)
* doing an munmap in the process, which may be blocked by
* seccomp and cause the task to be killed.
*/
- if (item->seccomp_mode != SECCOMP_MODE_DISABLED && suspend_seccomp(pid) < 0)
+ if (item->has_seccomp && suspend_seccomp(pid) < 0)
pr_err("failed to suspend seccomp, restore will probably fail...\n");
ctl = parasite_prep_ctl(pid, NULL);
diff --git a/include/proc_parse.h b/include/proc_parse.h
index 3daf9a2..e9b60ee 100644
--- a/include/proc_parse.h
+++ b/include/proc_parse.h
@@ -88,6 +88,8 @@ struct proc_status_creds {
int seccomp_mode;
};
+bool proc_status_creds_eq(struct proc_status_creds *o1, struct proc_status_creds *o2);
+
struct mount_info;
struct fstype {
char *name;
diff --git a/include/pstree.h b/include/pstree.h
index f0cd53c..10c5021 100644
--- a/include/pstree.h
+++ b/include/pstree.h
@@ -25,12 +25,16 @@ struct pstree_item {
int state; /* TASK_XXX constants */
/*
- * We keep the seccomp mode here temporarily between seizing and
- * dumping the task to avoid parsing /proc/pid/status twice. We also
- * use it on restore to hold the seccomp mode so that we don't have to
- * keep track of each task's core entry in the main criu process.
+ * On restore, we set this flag when process has seccomp filters so
+ * that we know to suspend them before we unmap the restorer blob.
*/
- int seccomp_mode;
+ bool has_seccomp;
+
+ /*
+ * We keep the creds here so that we can compare creds while seizing
+ * threads. Dumping tasks with different creds is not supported.
+ */
+ struct proc_status_creds *creds;
int nr_threads; /* number of threads */
struct pid *threads; /* array of threads */
diff --git a/include/ptrace.h b/include/ptrace.h
index bb4411d..44b66c9 100644
--- a/include/ptrace.h
+++ b/include/ptrace.h
@@ -5,6 +5,7 @@
#include <sys/ptrace.h>
#include "config.h"
+#include "proc_parse.h"
/* some constants for ptrace */
#ifndef PTRACE_SEIZE
@@ -66,7 +67,7 @@ struct ptrace_peeksiginfo_args {
#define SI_EVENT(_si_code) (((_si_code) & 0xFFFF) >> 8)
-extern int seize_task(pid_t pid, pid_t ppid, int *seccomp_mode);
+extern int seize_task(pid_t pid, pid_t ppid, struct proc_status_creds **creds);
extern int suspend_seccomp(pid_t pid);
extern int unseize_task(pid_t pid, int orig_state, int state);
extern int ptrace_peek_area(pid_t pid, void *dst, void *addr, long bytes);
diff --git a/proc_parse.c b/proc_parse.c
index 9c14a27..168afcb 100644
--- a/proc_parse.c
+++ b/proc_parse.c
@@ -2056,3 +2056,8 @@ int aufs_parse(struct mount_info *new)
return ret;
}
+
+bool proc_status_creds_eq(struct proc_status_creds *o1, struct proc_status_creds *o2)
+{
+ return memcmp(o1, o2, sizeof(struct proc_status_creds)) == 0;
+}
diff --git a/ptrace.c b/ptrace.c
index 035b737..4f9e66e 100644
--- a/ptrace.c
+++ b/ptrace.c
@@ -67,13 +67,18 @@ int suspend_seccomp(pid_t pid)
* up with someone else.
*/
-int seize_task(pid_t pid, pid_t ppid, int *seccomp_mode)
+int seize_task(pid_t pid, pid_t ppid, struct proc_status_creds **creds)
{
siginfo_t si;
int status;
int ret, ret2, ptrace_errno, wait_errno = 0;
struct proc_status_creds cr;
+ /*
+ * For the comparison below, let's zero out any padding.
+ */
+ memzero(&cr, sizeof(struct proc_status_creds));
+
ret = ptrace(PTRACE_SEIZE, pid, NULL, 0);
ptrace_errno = errno;
if (ret == 0) {
@@ -111,9 +116,6 @@ try_again:
if (ret2)
goto err;
- if (seccomp_mode)
- *seccomp_mode = cr.seccomp_mode;
-
if (!may_dump(&cr)) {
pr_err("Check uid (pid: %d) failed\n", pid);
goto err;
@@ -166,6 +168,18 @@ try_again:
goto try_again;
}
+ if (*creds == NULL) {
+ *creds = xzalloc(sizeof(struct proc_status_creds));
+ if (!*creds)
+ goto err_stop;
+
+ **creds = cr;
+
+ } else if (!proc_status_creds_eq(*creds, &cr)) {
+ pr_err("creds don't match %d %d\n", pid, ppid);
+ goto err_stop;
+ }
+
if (cr.seccomp_mode != SECCOMP_MODE_DISABLED && suspend_seccomp(pid) < 0)
goto err_stop;
--
2.1.4
More information about the CRIU
mailing list