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

[Andrew Vagin]

On Thu, Aug 07, 2014 at 08:46:19AM -0500, Tycho Andersen wrote:
> On Thu, Aug 07, 2014 at 10:08:06AM +0400, Pavel Emelyanov wrote:
> > On 08/07/2014 07:33 AM, Tycho Andersen wrote:
> > > 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.
> 
> Yep, agreed.
> 
> > > 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/..."?
> 
> Ah, I didn't realize those would poke at the right /proc, that is
> handy :). Yes, parse_mountinfo looks in /proc directly. However, when I do
> this:
> 
> diff --git a/proc_parse.c b/proc_parse.c
> index 3da5445..68d932b 100644
> --- a/proc_parse.c
> +++ b/proc_parse.c
> @@ -944,8 +944,7 @@ struct mount_info *parse_mountinfo(pid_t pid, struct ns_id *nsid)
>         FILE *f;
>         char str[1024];
>  
> -       snprintf(str, sizeof(str), "/proc/%d/mountinfo", pid);
> -       f = fopen(str, "r");
> +       f = fopen_proc(pid, "mountinfo");
>         if (!f) {
>                 pr_perror("Can't open %d mountinfo", pid);
>                 return NULL;
> 
> I still get Error (mount.c:1952): New root and old root are the same.

Tycho, look at the attached patch. This patch uses /proc/self/mountinfo
to collect mounts for the root task on restore.

Thanks.


> 
> Tycho
-------------- next part --------------
diff --git a/mount.c b/mount.c
index e0e88ea..3287282 100644
--- a/mount.c
+++ b/mount.c
@@ -1911,7 +1911,7 @@ int prepare_mnt_ns(void)
 {
 	int ret = -1;
 	struct mount_info *mis, *old;
-	struct ns_id ns = { .pid = getpid(), .nd = &mnt_ns_desc };
+	struct ns_id ns = { .pid = PROC_SELF, .nd = &mnt_ns_desc };
 
 	if (!(root_ns_mask & CLONE_NEWNS))
 		return rst_collect_local_mntns();
diff --git a/proc_parse.c b/proc_parse.c
index 66a1e12..e50063f 100644
--- a/proc_parse.c
+++ b/proc_parse.c
@@ -944,7 +944,10 @@ struct mount_info *parse_mountinfo(pid_t pid, struct ns_id *nsid)
 	FILE *f;
 	char str[1024];
 
-	snprintf(str, sizeof(str), "/proc/%d/mountinfo", pid);
+	if (pid == PROC_SELF)
+		snprintf(str, sizeof(str), "/proc/self/mountinfo");
+	else
+		snprintf(str, sizeof(str), "/proc/%d/mountinfo", pid);
 	f = fopen(str, "r");
 	if (!f) {
 		pr_perror("Can't open %d mountinfo", pid);