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

[Tycho Andersen]

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

> > > Tycho, could you look at https://github.com/avagin/criu/commit/6d1cc9acb7e3f442cafbb4b6ff2596ad32b7ac34
> > > This patch reworks code, which reports the error "(mount.c:1958): New
> > > root and old root are the same".
> > 
> > This would fix my issue as well (I think). Won't it leak the mount if
> > criu doesn't create a new mount namespace, though? (I do think this
> > nicer than forcing the user to mount their own --root.)
> 
> This code is executed only if a new mntns is created.

Ah, ok.

Tycho