[CRIU] [PATCH 4/6] dump: dump signals before infecting with parasite

[Ruslan Kuprieiev]

05.08.2014 18:08, Andrew Vagin пишет:
> On Tue, Aug 05, 2014 at 05:13:46PM +0300, Ruslan Kuprieiev wrote:
>> We need this In order to fit signals into core.img. core.img is being
>> written
>> in dump_core_task_all,which is called right in the middle of dump_one_task,
>> when parasite code is already injected. We can't dump signals while
>> under parasite, because parasite has its own signals.
> No, it has not.

What do you mean?

> Here is another problem.
> Signals can be dumped only for stopped tasks.
>
> Look at the folling code:
> if (SI_EVENT(si.si_code) != PTRACE_EVENT_STOP) {
> 	/*
> 	 * Kernel notifies us about the task being seized
> 	 * received some
> 	 * event other than the STOP, i.e. -- a signal. Let the
> 	 * task
> 	 * handle one and repeat.
> 	 */
>
> 	if (ptrace(PTRACE_CONT, pid, NULL,
> 				(void *)(unsigned long)si.si_signo)) {
> 		pr_perror("Can't continue signal handling, aborting");
> 		goto err;
> 	}
>
> 	goto try_again;
> }
>
> The comments says that we exepect siganls here.

Yes, but seize_task is called in collect_pstree, which is called far 
before dumping.
cr-dump.c : cr_dump_tasks()
     /*
      * The collect_pstree will also stop (PTRACE_SEIZE) the tasks
      * thus ensuring that they don't modify anything we collect
      * afterwards.
      */

     if (collect_pstree(pid))
         goto err;

  ...........................................................

     for_each_pstree_item(item) {
         if (dump_one_task(item))
             goto err;
     }

The comment says that the whole tree is stopped after collect_pstree, so 
it's safe to dump signals.

>> This patch doesn't change anything about dumping singnals, because
>> in both cases task is seized and doesn't have parasite injected.
>>
>> I've tested it by running sigpending from zdtm and it seems to work well.
> It doesn't test a race condition. Let's imagine that we have two tasks.
>
> 1. criu freezes the task 1
> 2. task 2 sends signal to the task 1
> 3. criu infects the task 1
>
> Could you create this test?

Well, race conditions are hard to test =).

>> 05.08.2014 16:56, Andrew Vagin пишет:
>>> Why do we need this?
>>>
>>> If a tack has a few pending signals, will it be safe?
>>>
>>> On Tue, Aug 05, 2014 at 10:35:59AM +0200, Ruslan Kuprieiev wrote:
>>>> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
>>>> ---
>>>>   cr-dump.c | 12 ++++++------
>>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/cr-dump.c b/cr-dump.c
>>>> index d8ad0fc..c8be0bd 100644
>>>> --- a/cr-dump.c
>>>> +++ b/cr-dump.c
>>>> @@ -1498,6 +1498,12 @@ static int dump_one_task(struct pstree_item *item)
>>>>   		goto err;
>>>>   	}
>>>> +	ret = dump_task_signals(pid, item, cr_fdset);
>>>> +	if (ret) {
>>>> +		pr_err("Dump %d signals failed %d\n", pid, ret);
>>>> +		goto err;
>>>> +	}
>>>> +
>>>>   	ret = -1;
>>>>   	parasite_ctl = parasite_infect_seized(pid, item, &vmas, dfds, proc_args.timer_n);
>>>>   	if (!parasite_ctl) {
>>>> @@ -1632,12 +1638,6 @@ static int dump_one_task(struct pstree_item *item)
>>>>   		goto err;
>>>>   	}
>>>> -	ret = dump_task_signals(pid, item, cr_fdset);
>>>> -	if (ret) {
>>>> -		pr_err("Dump %d signals failed %d\n", pid, ret);
>>>> -		goto err;
>>>> -	}
>>>> -
>>>>   	close_cr_fdset(&cr_fdset);
>>>>   err:
>>>>   	close_pid_proc();
>>>> -- 
>>>> 1.9.1
>>>>
>>>> _______________________________________________
>>>> CRIU mailing list
>>>> CRIU at openvz.org
>>>> https://lists.openvz.org/mailman/listinfo/criu

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20140805/d0d648be/attachment.html>