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

[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. Here is another problem. 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.

> 
> 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?

> 
> 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
>