During restart, each process that completes its restore will wake up
the next process in line, using the pid provided in the image file.
A malicious user could provide bogus pids and cause arbitrary tasks to
be woken up.
This patch tightens the checks and requires that to wake-up a task by
pid during restart, the target task must belong to (have) the same
restart context (task->checkpoint_ctx) as the current process.
Also, update in-code comments about restart and zombie logic.
Signed-off-by: Oren Laadan <or...@cs.columbia.edu>
---
checkpoint/restart.c | 35 ++++++++++++++++++++---------------
1 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/checkpoint/restart.c b/checkpoint/restart.c
index 2282ffc..840b5cb 100644
--- a/checkpoint/restart.c
+++ b/checkpoint/restart.c
@@ -462,12 +462,12 @@ static int restore_read_tree(struct ckpt_ctx *ctx)
return PTR_ERR(h);
ret = -EINVAL;
- if (h->nr_tasks < 0)
+ if (h->nr_tasks <= 0)
goto out;
ctx->nr_pids = h->nr_tasks;
size = sizeof(*ctx->pids_arr) * ctx->nr_pids;
- if (size < 0) /* overflow ? */
+ if (size <= 0) /* overflow ? */
goto out;
ctx->pids_arr = kmalloc(size, GFP_KERNEL);
@@ -520,8 +520,11 @@ int ckpt_activate_next(struct ckpt_ctx *ctx)
rcu_read_lock();
task = find_task_by_pid_ns(pid, ctx->root_nsproxy->pid_ns);
- if (task)
+ /* target task must have same restart context */
+ if (task && task->checkpoint_ctx == ctx)
wake_up_process(task);
+ else
+ task = NULL;
rcu_read_unlock();
if (!task) {
@@ -568,9 +571,8 @@ static int do_restore_task(void)
int ret;
/*
- * Wait for coordinator to make become visible, then grab a
- * reference to its restart context. If we're the last task to
- * do it, notify the coordinator.
+ * Wait for coordinator to become visible, then grab a
+ * reference to its restart context.
*/
ret = wait_event_interruptible(waitq, current->checkpoint_ctx);
if (ret < 0)
@@ -610,8 +612,9 @@ static int do_restore_task(void)
goto out;
/*
- * zombie: we're done here; Save @ctx on task_struct, to be
- * used to ckpt_activate_next(), and released, from do_exit().
+ * zombie: we're done here; do_exit() will notice the @ctx on
+ * our current->checkpoint_ctx (and our PF_RESTARTING) - it
+ * will call ckpt_activate_next() and release the @ctx.
*/
if (ret)
do_exit(current->exit_code);
@@ -638,11 +641,11 @@ static int do_restore_task(void)
}
/**
- * prepare_descendants - set ->restart_tsk of all descendants
+ * prepare_descendants - set ->checkpoint_ctx of all descendants
* @ctx: checkpoint context
* @root: root process for restart
*
- * Called by the coodinator to set the ->restart_tsk pointer of the
+ * Called by the coodinator to set the ->checkpoint_ctx pointer of the
* root task and all its descendants.
*/
static int prepare_descendants(struct ckpt_ctx *ctx, struct task_struct *root)
@@ -662,7 +665,7 @@ static int prepare_descendants(struct ckpt_ctx *ctx, struct
task_struct *root)
break;
}
/*
- * Set task->restart_tsk of all non-zombie descendants.
+ * Set task->checkpoint_ctx of all non-zombie descendants.
* If a descendant already has a ->checkpoint_ctx, it
* must be a coordinator (for a different restart ?) so
* we fail.
@@ -727,6 +730,7 @@ static int wait_all_tasks_finish(struct ckpt_ctx *ctx)
init_completion(&ctx->complete);
+ BUG_ON(ctx->active_pid != -1);
ret = ckpt_activate_next(ctx);
if (ret < 0)
return ret;
@@ -839,7 +843,7 @@ static int do_restore_coord(struct ckpt_ctx *ctx, pid_t pid)
if (ret < 0)
goto out;
} else {
- /* prepare descendants' t->restart_tsk point to coord */
+ /* prepare descendants' t->checkpoint_ctx point to coord */
ret = prepare_descendants(ctx, ctx->root_task);
if (ret < 0)
goto out;
@@ -970,11 +974,12 @@ void exit_checkpoint(struct task_struct *tsk)
{
struct ckpt_ctx *ctx;
- ctx = tsk->checkpoint_ctx;
- tsk->checkpoint_ctx = NULL;
+ /* no one else will touch this, because @tsk is dead already */
+ ctx = xchg(&tsk->checkpoint_ctx, NULL);
- /* restarting zombies will acrivate next task in restart */
+ /* restarting zombies will activate next task in restart */
if (tsk->flags & PF_RESTARTING) {
+ BUG_ON(ctx->active_pid == -1);
if (ckpt_activate_next(ctx) < 0)
pr_warning("c/r: [%d] failed zombie exit\n", tsk->pid);
}
--
1.6.0.4
_______________________________________________
Containers mailing list
contain...@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
Devel@openvz.org
https://openvz.org/mailman/listinfo/devel
