[CRIU] [PATCH] restorer: block signals before restoring SIGCHLD handler

[Andrew Vagin]

On Fri, Sep 20, 2013 at 06:59:33PM +0400, Pavel Emelyanov wrote:
> On 09/20/2013 06:21 PM, Andrey Vagin wrote:
> > Otherwise the process can handler this signal before calling sigreturn.
> > 
> > And threads must block signals before switching on
> > CR_STATE_RESTORE_SIGCHLD, because signals are restoring on this stage.
> > 
> > So if a thread leader starts restoring shared signals before another
> > thread has blocked signals, this thread can start to hadle the signal.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The previous two lines describe the race

__export_restore_task				| __export_restore_thread

restore_finish_stage(CR_STATE_RESTORE);		| restore_finish_stage(CR_STATE_RESTORE);
sys_sigaction(SIGCHLD, &args->sigchld_act, ...) |
restore_signals()				|
	ksigfillset(&to_block);			|
	sys_rt_sigqueueinfo(sys_getpid(), ...); |
						| execute signal handler() <------ ALARM !!!
						| restore_signals()
						|	ksigfillset(&to_block);

> > I have seen this race in static/sigpending.
> 
> What race? Plz, elaborate.

> 
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  pie/restorer.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> > 
> > diff --git a/pie/restorer.c b/pie/restorer.c
> > index 95bb63f..cc58584 100644
> > --- a/pie/restorer.c
> > +++ b/pie/restorer.c
> > @@ -211,14 +211,6 @@ static void restore_rlims(struct task_restore_core_args *ta)
> >  static int restore_signals(siginfo_t *ptr, int nr, bool group)
> >  {
> >  	int ret, i;
> > -	k_rtsigset_t to_block;
> > -
> > -	ksigfillset(&to_block);
> > -	ret = sys_sigprocmask(SIG_SETMASK, &to_block, NULL, sizeof(k_rtsigset_t));
> > -	if (ret) {
> > -		pr_err("Unable to block signals %d", ret);
> > -		return -1;
> > -	}
> >  
> >  	for (i = 0; i < nr; i++) {
> >  		siginfo_t *info = ptr + i;
> > @@ -272,6 +264,7 @@ static int restore_thread_common(struct rt_sigframe *sigframe,
> >  long __export_restore_thread(struct thread_restore_args *args)
> >  {
> >  	struct rt_sigframe *rt_sigframe;
> > +	k_rtsigset_t to_block;
> >  	unsigned long new_sp;
> >  	int my_pid = sys_gettid();
> >  	int ret;
> > @@ -281,6 +274,14 @@ long __export_restore_thread(struct thread_restore_args *args)
> >  		goto core_restore_end;
> >  	}
> >  
> > +	/* All signals must be handled by thread leader */
> > +	ksigfillset(&to_block);
> > +	ret = sys_sigprocmask(SIG_SETMASK, &to_block, NULL, sizeof(k_rtsigset_t));
> > +	if (ret) {
> > +		pr_err("Unable to block signals %d", ret);
> > +		goto core_restore_end;
> > +	}
> > +
> >  	rt_sigframe = (void *)args->mem_zone.rt_sigframe;
> >  
> >  	if (restore_thread_common(rt_sigframe, args))
> > @@ -568,6 +569,7 @@ long __export_restore_task(struct task_restore_core_args *args)
> >  
> >  	struct rt_sigframe *rt_sigframe;
> >  	unsigned long new_sp;
> > +	k_rtsigset_t to_block;
> >  	pid_t my_pid = sys_getpid();
> >  	rt_sigaction_t act;
> >  
> > @@ -853,6 +855,13 @@ long __export_restore_task(struct task_restore_core_args *args)
> >  
> >  	futex_wait_while_gt(&zombies_inprogress, 0);
> >  
> > +	ksigfillset(&to_block);
> > +	ret = sys_sigprocmask(SIG_SETMASK, &to_block, NULL, sizeof(k_rtsigset_t));
> > +	if (ret) {
> > +		pr_err("Unable to block signals %ld", ret);
> > +		goto core_restore_end;
> > +	}
> > +
> >  	sys_sigaction(SIGCHLD, &args->sigchld_act, NULL, sizeof(k_rtsigset_t));
> >  
> >  	ret = restore_signals(args->siginfo, args->siginfo_nr, true);
> > 
> 
>