[CRIU] [PATCH v2] user_ns: Block SIGCHLD during namespaces generation

[Kirill Tkhai]

On 05.04.2017 19:56, Andrei Vagin wrote:
> On Wed, Apr 5, 2017 at 2:47 AM, Kirill Tkhai <ktkhai at virtuozzo.com> wrote:
>> On 05.04.2017 09:24, Andrei Vagin wrote:
>>> On Tue, Apr 04, 2017 at 12:19:32PM +0300, Kirill Tkhai wrote:
>>>> On 04.04.2017 02:57, Andrei Vagin wrote:
>>>>> On Sat, Apr 01, 2017 at 04:05:40PM +0300, Kirill Tkhai wrote:
>>>>>> We don't want asynchronous signal handler does wait()
>>>>>> as we do that synchronous. So block the signal.
>>>>>>
>>>>>> v2: Set initial ret = 0
>>>>>>
>>>>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>>>>> ---
>>>>>>  criu/namespaces.c |   21 ++++++++++++++++++++-
>>>>>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/criu/namespaces.c b/criu/namespaces.c
>>>>>> index 6637abcb..5e71a3a9 100644
>>>>>> --- a/criu/namespaces.c
>>>>>> +++ b/criu/namespaces.c
>>>>>> @@ -2257,7 +2257,26 @@ static int create_user_ns_hierarhy_fn(void *in_arg)
>>>>>>  static int create_user_ns_hierarhy(void)
>>>>>>  {
>>>>>>    struct ns_arg arg = { .me = root_user_ns };
>>>>>> -  return create_user_ns_hierarhy_fn(&arg);
>>>>>> +  sigset_t blockmask, oldmask;
>>>>>> +  int ret = 0;
>>>>>> +
>>>>>> +  sigemptyset(&blockmask);
>>>>>> +  sigaddset(&blockmask, SIGCHLD);
>>>>>> +
>>>>>> +  if (sigprocmask(SIG_BLOCK, &blockmask, &oldmask) == -1) {
>>>>>> +          pr_perror("Can not set mask of blocked signals");
>>>>>> +          return -1;
>>>>>> +  }
>>>>>
>>>>> Looks like we have to block sigchld in restore_task_with_children() to
>>>>> cover all other cases. What do you think about this?
>>>>
>>>> If we did that, we wouldn't have emergency abort in case of a child exits.
>>>
>>> We don't have children at this stage. I suggest to unblock sigchld
>>> before restoring children of the root task. We run a lot of
>>> sub-processes to restore namespaces, so it looks reasonable to block
>>> sigchld once.
>>
>> Hm, but we have usernsd helper and we need to handle the situation when he dies
>> (meets segfault for example). Otherwise, the restore may hang in half-died state.
>> How do you looks on this?
> 
> usernsd is a child of the criu task, here we are talking about the root task.

Ah, sure, so that will work.
 
>>
>>>
>>>> Though, generation of ns tree is a special case. We don't want make it be
>>>> handled via sigchld_handler(), do we? We may introduce one more temporary
>>>> handler or even overload cr_restore.c::sigchld_handler(), but I'm not sure
>>>> it's the best solution. What do you think?
>>>>
>>>>>> +
>>>>>> +  if (create_user_ns_hierarhy_fn(&arg))
>>>>>> +          ret = -1;
>>>>>> +
>>>>>> +  if (sigprocmask(SIG_SETMASK, &oldmask, NULL) == -1) {
>>>>>> +          pr_perror("Can not unset mask of blocked signals");
>>>>>> +          ret = -1;
>>>>>> +  }
>>>>>> +
>>>>>> +  return ret;
>>>>>>  }
>>>>>>
>>>>>>  int prepare_namespace(struct pstree_item *item, unsigned long clone_flags)
>>>>>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU at openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu