[CRIU] [PATCH v2 1/5] user_ns: Close pid proc in create_user_ns_hierarhy_fn()

[Kirill Tkhai]

On 03.04.2017 23:15, Andrei Vagin wrote:
> On Sat, Apr 01, 2017 at 02:29:32PM +0300, Kirill Tkhai wrote:
>> On 31.03.2017 21:13, Andrei Vagin wrote:
>>> On Wed, Mar 29, 2017 at 12:19:55PM +0300, Kirill Tkhai wrote:
>>>> Child is cloned with CLONE_FILES, so it may rewrite our
>>>> PROC_SELF file. Close it manually.
>>>>
>>>> We can't use call_in_child_process(), because we need
>>>> additional actions between clone() and wait().
>>>>
>>>> Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
>>>> ---
>>>>  criu/namespaces.c |    1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/criu/namespaces.c b/criu/namespaces.c
>>>> index c1bee49f..3a24099c 100644
>>>> --- a/criu/namespaces.c
>>>> +++ b/criu/namespaces.c
>>>> @@ -2251,6 +2251,7 @@ static int create_user_ns_hierarhy_fn(void *in_arg)
>>>>  		futex_set_and_wake(p_futex, ret ? NS__ERROR : NS__RESTORED);
>>>>  	if (arg)
>>>>  		munmap(arg, sizeof(*arg));
>>>> +	close_pid_proc();
>>>
>>>                 pid = clone(create_user_ns_hierarhy_fn, stack + 128,
>>> CLONE_NEWUSER | CLONE_FILES | SIGCHLD, arg);
>>>
>>> This function is called from a process which shares files but doesn't
>>> share memory. close_pid_proc() closes a file descriptor and changes data
>>> in memory, but the parent process doesn't see changes in memory and may
>>> try to use the old file descriptor, which is closed already.
>>>
>>> I want to say that close_pid_proc(), open_proc() can't be used in case,
>>> because it is unclear how it will affect a parent process.
>>>
>>> https://github.com/xemul/criu/commit/9a38c503
>>
>> I agree with you, and even started reworked, and after that a better solution
>> for me seems to be to add CLONE_VM there. We call common primitives
>> like fdstore_add() and get_self_real_pid() right now, and nobody knows
>> how they change in the future. If we use CLONE_VM, we'll have generic
>> solution for any changes, and we won't have problems with this function
>> anymore. Speaking about memory usage: even when we do not use CLONE_VM, we
>> allocate pages on COW, so it seems to be almost the same as we allocate
>> a separate stack. How are you about that?
> 
> I don't have any objections about this.

It's in v3.