[CRIU] [PATCH 7/8] restore: Add restoration of alternative signal stack

[Pavel Emelyanov]

On 06/18/2013 05:17 PM, Cyrill Gorcunov wrote:
> On Tue, Jun 18, 2013 at 04:55:39PM +0400, Andrew Vagin wrote:
>>> --- a/pie/restorer.c
>>> +++ b/pie/restorer.c
>>> @@ -733,6 +733,15 @@ long __export_restore_task(struct task_restore_core_args *args)
>>>  		goto core_restore_end;
>>>  
>>>  	/*
>>> +	 * FIXME
>>> +	 * Alternative signal stack for the group leader
>>> +	 * is setting up separately from the threads.
>>> +	 * This better should be unified one day.
>>> +	 */
>>> +	if (args->has_sas)
>>> +		setup_sas(rt_sigframe, &args->sas);
>>> +
>>
>> I think would be better if setup_sas will be called after restore_signals,
>> because before this moment SIGCHLD isn't blocked.
>>
>> Can you explain how that works for nested signal handlers?
> 
> setup_sas does only one thing -- it prepares sigreturn frame, thus once
> sigreturn called the kernel sets up alternative signal frame for the
> task, iow simultaneously with setting up new cs:ip. So I don't get how
> it's related to nested signal handlers. If I understand you correctly
> the SIGCHLD which might be called there is handled by our code with our
> own signal handler, no? In turn former blocked signals mask is restored
> sigreturn too, thus (if only I've not missed something obvious) -- we're
> allowed to restore sas with sigreturn help insted of explicit calling
> for sigaltstack() system routine.
> .
> 

It would be nice if this text and the one below:

> We call sigaltstack in only one case -- on dump, to fetch the value(s)
> dumpee has. On restore we have 2 ways
> 
>  - either call for sigaltstack explicitly in restorer.c right before
>    we call for sigreturn
> 
>  - either pass stack value to sigreturn call and allow the kernel to
>    restore stack for us
> 
> I chose second way and simply tune up sigframe. Sounds reasonable?

are in the patch comment.