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

[Andrei Vagin]

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.

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