Re: [PATCH 1/1] don't call pre_restore_task twice

["Serge E. Hallyn" <serue@xxxxxxxxxx>]

Quoting Matt Helsley (matthltc@xxxxxxxxxx):
> On Wed, Oct 07, 2009 at 06:47:50PM -0500, Serge E. Hallyn wrote:
> > Pre_restore_task is being called both before and inside
> > restore_task, causing a memory leak at
> > current->checkpoint_data.
> > 
> > Only call it once, outside restore_task.
> > 
> > This fixes a memory leak spotted by Dan Smith, and the
> > actual bug was deduced by Matt Helsley.
> > 
> > Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx>
> > Reported-by: Dan Smith <danms@xxxxxxxxxx>
> > Cc: Dan Smith <danms@xxxxxxxxxx>
> > Cc: Matt Helsley <matthltc@xxxxxxxxxx>
> > 
> > Signed-off-by: Serge E. Hallyn <serue@xxxxxxxxxx>
> 
> Reviewed-by: Matt Helsley <matthltc@xxxxxxxxxx>
> 
> However, I think I spotted another problem:
> 
> int pre_restore_task()
> {
>         sigset_t sigset;
> 
>         /* task-specific restart data: freed from post_restore_task() */
>         current->checkpoint_data = kzalloc(sizeof(struct ckpt_data),
> GFP_KERNEL);
>         if (!current->checkpoint_data)
>                 return -ENOMEM;
> ...
> }
> 
> void post_restore_task()
> {
> 	sigprocmask(SIG_SETMASK, &current->checkpoint_data->blocked, NULL);
> ...
> }
> 
> then in do_restore_coord():
> 
> if (ctx->uflags & RESTART_TASKSELF) {
>                 ret = pre_restore_task();
>                 ckpt_debug("pre restore task: %d\n", ret);
>                 if (ret < 0)
>                         goto out;
> ...
>  out:
>         if (ctx->uflags & RESTART_TASKSELF)
>                 post_restore_task();
> 
> But if we got -ENOMEM from pre_restore_task() then I think there will be a
> NULL dereference.

But the very first thing post_restore_task() does is

	/* can happen if restart failed early */
	if (!current->checkpoint_data)
		return;

-serge
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers