From e0b24e21d3500aec212b1083b90d17f70d997de7 Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Thu, 18 Jun 2015 11:59:19 -0600 Subject: [PATCH] creds: fail to dump when creds in thread group don't match 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 Signed-off-by: Pavel Emelyanov --- 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 ad7899865..acbbe3f25 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 9d7162a48..f3d55f6d9 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 756e29022..45c746e7a 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 3daf9a285..e9b60eeba 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 f0cd53c30..10c502193 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 bb4411dd1..44b66c9af 100644 --- a/include/ptrace.h +++ b/include/ptrace.h @@ -5,6 +5,7 @@ #include #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 9c14a27a3..168afcb67 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 035b7374b..4f9e66ef9 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;