[CRIU] [PATCH 2/2] restore: remount /proc after clone(CLONE_NEWPID)

[Pavel Emelyanov]

On 08/07/2014 07:33 AM, Tycho Andersen wrote:
> Hi Andrew,
> 
> On Thu, Aug 07, 2014 at 12:27:11AM +0400, Andrew Vagin wrote:
>> On Wed, Aug 06, 2014 at 03:12:25PM -0500, Tycho Andersen wrote:
>>> Hi Andrew,
>>>
>>> On Wed, Aug 06, 2014 at 11:54:06PM +0400, Andrew Vagin wrote:
>>>> On Wed, Aug 06, 2014 at 01:52:17PM -0500, Tycho Andersen wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> On Wed, Aug 06, 2014 at 10:44:12PM +0400, Andrew Vagin wrote:
>>>>>> On Wed, Aug 06, 2014 at 01:34:07PM -0500, Tycho Andersen wrote:
>>>>>>> Hi Andrew,
>>>>>>>
>>>>>>> On Wed, Aug 06, 2014 at 10:25:10PM +0400, Andrew Vagin wrote:
>>>>>>>> On Wed, Aug 06, 2014 at 01:06:29PM -0500, Tycho Andersen wrote:
>>>>>>>>> We need to remount /proc after the clone because things like getpid() return
>>>>>>>>> the pid in the new namespace, but /proc still has the old namespace's info in
>>>>>>>>> it. This causes problems when e.g. there are some things in criu's private
>>>>>>>>> mount namespace but not in (the original) init's namespace.
>>>>>>>>
>>>>>>>> Could you show an example of problems?
>>>>>>>
>>>>>>> Yes, if you:
>>>>>>>
>>>>>>> 1. unshare(CLONE_NEWNS)
>>>>>>> 2. mount() some directory to pass as --root
>>>>>>> 3. exec(criu)
>>>>>>
>>>>>> Could you show how criu fails in this case?
>>>>>
>>>>> Yes,
>>>>>
>>>>>>>
>>>>>>> criu fails with "(mount.c:1958): New root and old root are the same".
>>>>
>>>> And this patch fixes this error? I don't understand how it can be.
>>>
>>> Yes. When /proc is remounted, the process' "private" (i.e. whatever
>>> happened after unshare()) mount namespace is visible in
>>> /proc/1/mountinfo. Since it is visible, criu parses it and resolves it
>>> correctly and all is happy.
>>
>> Eh. I have understood the problem. It's my bug. I think we should use
>> /proc/self/mountinfo to collect mountpoints or we can collect it before
>> forking the root task. I will look on the code tomorrow. Thanks you for
>> pointing out the problem.
>>
> 
> Isn't it still a bug not to re-mount /proc though?

If we don't create new mount namespace, then re-mounting seem like bug --
if we change the way information is represented in it for root (init) task
we restore, then we change the way it looks for the tasks living in the
pid namespace criu was launched in.

> Or, I guess it
> isn't totally a bug, as long as you don't look in /proc, but criu
> seems to do that a lot. 

It does, but it should do all the proc stuff via proc service descriptor.
The open_proc, fopen_proc and others do exactly this.

Do you know any places where criu just goes and accesses "/proc/..."?

> I guess we could not re-mount /proc if we
> don't make a new mount ns, but it seems like we should re-mount if we
> do, since it would be less error prone.

If criu erroneously accesses wrong proc instance when restoring pid
namespaces, we should fix this even if mount namespaces are not there
and we don't remount proc.


Thanks,
Pavel