[CRIU] [PATCH 1/2] seize: Simplify creds checkers
Pavel Emelyanov
xemul at virtuozzo.com
Tue Sep 27 10:45:04 PDT 2016
Don't do comparisons of creds insize core seizing routine. Instead,
pull the creds on the caller's stack and compare them there if
required.
At the same time more the proc_status_creds_dumpable() to the place
where it's only needed and make it static.
Signed-off-by: Pavel Emelyanov <xemul at virtuozzo.com>
---
criu/cr-exec.c | 14 ++++-----
criu/include/proc_parse.h | 3 --
criu/include/ptrace.h | 2 +-
criu/proc_parse.c | 49 -------------------------------
criu/ptrace.c | 36 ++++++-----------------
criu/seize.c | 75 +++++++++++++++++++++++++++++++++++++++++++++--
6 files changed, 88 insertions(+), 91 deletions(-)
diff --git a/criu/cr-exec.c b/criu/cr-exec.c
index 6e7a5de..d1424fc 100644
--- a/criu/cr-exec.c
+++ b/criu/cr-exec.c
@@ -119,7 +119,7 @@ int cr_exec(int pid, char **opt)
struct parasite_ctl *ctl;
struct vm_area_list vmas;
int ret, prev_state, exit_code = -1;
- struct proc_status_creds *creds = NULL;
+ struct proc_status_creds creds;
unsigned long p_start;
if (!sys_name) {
@@ -135,19 +135,17 @@ int cr_exec(int pid, char **opt)
if (seize_catch_task(pid))
goto out;
+ /*
+ * We don't seize a task's threads here, so there is no reason to
+ * mess with creds in this use case anyway.
+ */
+
prev_state = ret = seize_wait_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, NULL);
if (ret) {
pr_err("Can't collect vmas for %d\n", pid);
diff --git a/criu/include/proc_parse.h b/criu/include/proc_parse.h
index fb0d643..ac7a780 100644
--- a/criu/include/proc_parse.h
+++ b/criu/include/proc_parse.h
@@ -94,9 +94,6 @@ struct proc_status_creds {
u32 cap_bnd[PROC_CAP_SIZE];
};
-bool proc_status_creds_dumpable(struct proc_status_creds *parent,
- struct proc_status_creds *child);
-
#define INVALID_UID ((uid_t)-1)
extern int parse_pid_stat(pid_t pid, struct proc_pid_stat *s);
diff --git a/criu/include/ptrace.h b/criu/include/ptrace.h
index bf416b3..b30d4ba 100644
--- a/criu/include/ptrace.h
+++ b/criu/include/ptrace.h
@@ -76,7 +76,7 @@ struct ptrace_peeksiginfo_args {
extern int processes_to_wait;
extern int seize_catch_task(pid_t pid);
-extern int seize_wait_task(pid_t pid, pid_t ppid, struct proc_status_creds **creds);
+extern int seize_wait_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/criu/proc_parse.c b/criu/proc_parse.c
index 49d6c1a..cca63f4 100644
--- a/criu/proc_parse.c
+++ b/criu/proc_parse.c
@@ -2506,55 +2506,6 @@ int aufs_parse(struct mount_info *new)
return ret;
}
-bool proc_status_creds_dumpable(struct proc_status_creds *parent,
- struct proc_status_creds *child)
-{
- const size_t size = sizeof(struct proc_status_creds) -
- offsetof(struct proc_status_creds, cap_inh);
-
- /*
- * The comparison rules are the following
- *
- * - CAPs can be different
- * - seccomp filters should be passed via
- * semantic comparison (FIXME) but for
- * now we require them to be exactly
- * identical
- * - the rest of members must match
- */
-
- if (memcmp(parent, child, size)) {
- if (!pr_quelled(LOG_DEBUG)) {
- pr_debug("Creds undumpable (parent:child)\n"
- " uids: %d:%d %d:%d %d:%d %d:%d\n"
- " gids: %d:%d %d:%d %d:%d %d:%d\n"
- " state: %d:%d"
- " ppid: %d:%d\n"
- " sigpnd: %llu:%llu\n"
- " shdpnd: %llu:%llu\n"
- " seccomp_mode: %d:%d\n"
- " last_filter: %u:%u\n",
- parent->uids[0], child->uids[0],
- parent->uids[1], child->uids[1],
- parent->uids[2], child->uids[2],
- parent->uids[3], child->uids[3],
- parent->gids[0], child->gids[0],
- parent->gids[1], child->gids[1],
- parent->gids[2], child->gids[2],
- parent->gids[3], child->gids[3],
- parent->state, child->state,
- parent->ppid, child->ppid,
- parent->sigpnd, child->sigpnd,
- parent->shdpnd, child->shdpnd,
- parent->seccomp_mode, child->seccomp_mode,
- parent->last_filter, child->last_filter);
- }
- return false;
- }
-
- return true;
-}
-
int parse_children(pid_t pid, pid_t **_c, int *_n)
{
pid_t *ch = NULL;
diff --git a/criu/ptrace.c b/criu/ptrace.c
index 3d2f56e..a449487 100644
--- a/criu/ptrace.c
+++ b/criu/ptrace.c
@@ -148,17 +148,11 @@ static int skip_sigstop(int pid, int nr_signals)
* of it so the task would not know if it was saddled
* up with someone else.
*/
-int seize_wait_task(pid_t pid, pid_t ppid, struct proc_status_creds **creds)
+int seize_wait_task(pid_t pid, pid_t ppid, struct proc_status_creds *creds)
{
siginfo_t si;
int status, nr_sigstop;
int ret = 0, ret2, 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));
/*
* It's ugly, but the ptrace API doesn't allow to distinguish
@@ -184,26 +178,26 @@ try_again:
wait_errno = errno;
}
- ret2 = parse_pid_status(pid, &cr);
+ ret2 = parse_pid_status(pid, creds);
if (ret2)
goto err;
if (ret < 0 || WIFEXITED(status) || WIFSIGNALED(status)) {
- if (cr.state != 'Z') {
+ if (creds->state != 'Z') {
if (pid == getpid())
pr_err("The criu itself is within dumped tree.\n");
else
pr_err("Unseizable non-zombie %d found, state %c, err %d/%d\n",
- pid, cr.state, ret, wait_errno);
+ pid, creds->state, ret, wait_errno);
return -1;
}
return TASK_DEAD;
}
- if ((ppid != -1) && (cr.ppid != ppid)) {
+ if ((ppid != -1) && (creds->ppid != ppid)) {
pr_err("Task pid reused while suspending (%d: %d -> %d)\n",
- pid, ppid, cr.ppid);
+ pid, ppid, creds->ppid);
goto err;
}
@@ -235,25 +229,13 @@ try_again:
goto try_again;
}
- if (*creds == NULL) {
- *creds = xzalloc(sizeof(struct proc_status_creds));
- if (!*creds)
- goto err;
-
- **creds = cr;
-
- } else if (!proc_status_creds_dumpable(*creds, &cr)) {
- pr_err("creds don't match %d %d\n", pid, ppid);
- goto err;
- }
-
- if (cr.seccomp_mode != SECCOMP_MODE_DISABLED && suspend_seccomp(pid) < 0)
+ if (creds->seccomp_mode != SECCOMP_MODE_DISABLED && suspend_seccomp(pid) < 0)
goto err;
nr_sigstop = 0;
- if (cr.sigpnd & (1 << (SIGSTOP - 1)))
+ if (creds->sigpnd & (1 << (SIGSTOP - 1)))
nr_sigstop++;
- if (cr.shdpnd & (1 << (SIGSTOP - 1)))
+ if (creds->shdpnd & (1 << (SIGSTOP - 1)))
nr_sigstop++;
if (si.si_signo == SIGSTOP)
nr_sigstop++;
diff --git a/criu/seize.c b/criu/seize.c
index e1e805a..0a9a750 100644
--- a/criu/seize.c
+++ b/criu/seize.c
@@ -437,6 +437,7 @@ static int collect_children(struct pstree_item *item)
nr_inprogress = 0;
for (i = 0; i < nr_children; i++) {
struct pstree_item *c;
+ struct proc_status_creds *creds;
pid_t pid = ch[i];
/* Is it already frozen? */
@@ -462,7 +463,13 @@ static int collect_children(struct pstree_item *item)
/* fails when meets a zombie */
seize_catch_task(pid);
- ret = seize_wait_task(pid, item->pid.real, &dmpi(c)->pi_creds);
+ creds = xzalloc(sizeof(*creds));
+ if (!creds) {
+ ret = -1;
+ goto free;
+ }
+
+ ret = seize_wait_task(pid, item->pid.real, creds);
if (ret < 0) {
/*
* Here is a race window between parse_children() and seize(),
@@ -473,9 +480,11 @@ static int collect_children(struct pstree_item *item)
*/
ret = 0;
xfree(c);
+ xfree(creds);
continue;
}
+ dmpi(c)->pi_creds = creds;
c->pid.real = pid;
c->parent = item;
c->pid.state = ret;
@@ -591,6 +600,55 @@ static inline bool thread_collected(struct pstree_item *i, pid_t tid)
return false;
}
+static bool creds_dumpable(struct proc_status_creds *parent,
+ struct proc_status_creds *child)
+{
+ const size_t size = sizeof(struct proc_status_creds) -
+ offsetof(struct proc_status_creds, cap_inh);
+
+ /*
+ * The comparison rules are the following
+ *
+ * - CAPs can be different
+ * - seccomp filters should be passed via
+ * semantic comparison (FIXME) but for
+ * now we require them to be exactly
+ * identical
+ * - the rest of members must match
+ */
+
+ if (memcmp(parent, child, size)) {
+ if (!pr_quelled(LOG_DEBUG)) {
+ pr_debug("Creds undumpable (parent:child)\n"
+ " uids: %d:%d %d:%d %d:%d %d:%d\n"
+ " gids: %d:%d %d:%d %d:%d %d:%d\n"
+ " state: %d:%d"
+ " ppid: %d:%d\n"
+ " sigpnd: %llu:%llu\n"
+ " shdpnd: %llu:%llu\n"
+ " seccomp_mode: %d:%d\n"
+ " last_filter: %u:%u\n",
+ parent->uids[0], child->uids[0],
+ parent->uids[1], child->uids[1],
+ parent->uids[2], child->uids[2],
+ parent->uids[3], child->uids[3],
+ parent->gids[0], child->gids[0],
+ parent->gids[1], child->gids[1],
+ parent->gids[2], child->gids[2],
+ parent->gids[3], child->gids[3],
+ parent->state, child->state,
+ parent->ppid, child->ppid,
+ parent->sigpnd, child->sigpnd,
+ parent->shdpnd, child->shdpnd,
+ parent->seccomp_mode, child->seccomp_mode,
+ parent->last_filter, child->last_filter);
+ }
+ return false;
+ }
+
+ return true;
+}
+
static int collect_threads(struct pstree_item *item)
{
struct pid *threads = NULL;
@@ -618,6 +676,7 @@ static int collect_threads(struct pstree_item *item)
nr_inprogress = 0;
for (i = 0; i < nr_threads; i++) {
pid_t pid = threads[i].real;
+ struct proc_status_creds t_creds = {};
if (thread_collected(item, pid))
continue;
@@ -630,7 +689,7 @@ static int collect_threads(struct pstree_item *item)
if (!opts.freeze_cgroup && seize_catch_task(pid))
continue;
- ret = seize_wait_task(pid, item_ppid(item), &dmpi(item)->pi_creds);
+ ret = seize_wait_task(pid, item_ppid(item), &t_creds);
if (ret < 0) {
/*
* Here is a race window between parse_threads() and seize(),
@@ -651,6 +710,9 @@ static int collect_threads(struct pstree_item *item)
goto err;
}
+ if (!creds_dumpable(dmpi(item)->pi_creds, &t_creds))
+ goto err;
+
if (ret == TASK_STOPPED) {
nr_stopped++;
}
@@ -737,6 +799,7 @@ int collect_pstree(void)
{
pid_t pid = root_item->pid.real;
int ret = -1;
+ struct proc_status_creds *creds;
timing_start(TIME_FREEZING);
@@ -755,11 +818,17 @@ int collect_pstree(void)
goto err;
}
- ret = seize_wait_task(pid, -1, &dmpi(root_item)->pi_creds);
+ creds = xzalloc(sizeof(*creds));
+ if (!creds)
+ goto err;
+
+ ret = seize_wait_task(pid, -1, creds);
if (ret < 0)
goto err;
+
pr_info("Seized task %d, state %d\n", pid, ret);
root_item->pid.state = ret;
+ dmpi(root_item)->pi_creds = creds;
ret = collect_task(root_item);
if (ret < 0)
--
2.5.0
More information about the CRIU
mailing list