Quoting Oren Laadan (orenl@xxxxxxxxxxx):
> From: Oren Laadan <orenl@xxxxxxxxxxx>
>
> If prepare_descendants() is walking a tree and one of the tasks is
> forking, one of two bads can happen. If the child doesn't inherit the
> ->ctx, it breaks the assumption that the entire subtree is prepared.
> If the child inherits the ->ctx, it will have one without having taken
> a reference.
>
> This patch closed this race by explicitly getting and referencing the
> ->ctx for a child process should the parent have one, atomically under
> the tasklist_lock.
>
> Signed-off-by: Oren Laadan <orenl@xxxxxxxxxxxxxxx>
> ---
> kernel/fork.c | 11 ++++++++---
> 1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 9f13d7b..57118e4 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -62,6 +62,7 @@
> #include <linux/fs_struct.h>
> #include <linux/magic.h>
> #include <linux/perf_counter.h>
> +#include <linux/checkpoint.h>
>
> #include <asm/pgtable.h>
> #include <asm/pgalloc.h>
> @@ -1148,9 +1149,6 @@ static struct task_struct *copy_process(unsigned long clone_flags,
> INIT_LIST_HEAD(&p->pi_state_list);
> p->pi_state_cache = NULL;
> #endif
> -#ifdef CONFIG_CHECKPOINT
> - p->checkpoint_ctx = NULL;
> -#endif
> /*
> * sigaltstack should be cleared when sharing the same VM
> */
> @@ -1188,6 +1186,13 @@ static struct task_struct *copy_process(unsigned long clone_flags,
> /* Need tasklist lock for parent etc handling! */
> write_lock_irq(&tasklist_lock);
>
> +#ifdef CONFIG_CHECKPOINT
> + /* If parent is restarting, child should be too */
> + if (unlikely(current->checkpoint_ctx)) {
> + p->checkpoint_ctx = current->checkpoint_ctx;
Won't break anything, but technically p->checkpoint_ctx will
already be copied from current->checkpoint_ctx, so only the
ckpt_ctx_get() is necessary, so this could really read
if (p->checkpoint_ctx)
ckpt_ctx_get(p->checkpoint_ctx);
Right?
> + ckpt_ctx_get(p->checkpoint_ctx);
> + }
> +#endif
> /*
> * The task hasn't been attached yet, so its cpus_allowed mask will
> * not be changed, nor will its assigned CPU.
> --
> 1.6.0.4
>
> _______________________________________________
> Containers mailing list
> Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
