[CRIU] [PATCH] filemap: Get vma mnt_id early

[Pavel Emelyanov]

We have a, well, issue with how we calculate the vma's mnt_id.

Right now get one via criu side file descriptor that it got by
opening the /proc/pid/map_files/ link. The problem is that these
descriptors are 'merged' or 'borrowed' by adjacent vmas from
previous ones. Thus, getting the mnt_id value for each of them
makes no sense -- these files are the same.

So move this mnt_id getting earlier into vma parsing code. This
brings a potential problem -- if we have two adjacent vmas
mapping the same inode (dev:ino pair) but living in different
mount namespaces -- this check would produce wrong result. 
"Wrong" from the perspective that on restore correct file would
be opened from wrong namespace.

I propose to live with it, since this is not worse than the
--evasive-devices option, it's _very_ unlikely, but saves a lot
of openeings.

Note, that in case app switched mount namespace and then mapped
some new library (with dlopen) things would work correctly -- new
vmas will likely be not adjacent and for different dev:ino.

Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
---
 cr-dump.c            | 15 +--------------
 include/proc_parse.h |  1 +
 include/vma.h        |  1 +
 proc_parse.c         | 23 +++++++++++++++++++++++
 4 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/cr-dump.c b/cr-dump.c
index 72eb94d..e873a38 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -247,17 +247,6 @@ static int collect_fds(pid_t pid, struct parasite_drain_fd *dfds)
 	return 0;
 }
 
-static int get_fd_mntid(int fd, int *mnt_id)
-{
-	struct fdinfo_common fdinfo = { .mnt_id = -1};
-
-	if (parse_fdinfo(fd, FD_TYPES__UND, NULL, &fdinfo))
-		return -1;
-
-	*mnt_id = fdinfo.mnt_id;
-	return 0;
-}
-
 static int fill_fd_params_special(int fd, struct fd_parms *p)
 {
 	*p = FD_PARMS_INIT;
@@ -371,6 +360,7 @@ static int dump_filemap(pid_t pid, struct vma_area *vma_area,
 
 	BUG_ON(!vma_area->vmst);
 	p.stat = *vma_area->vmst;
+	p.mnt_id = vma_area->mnt_id;
 
 	/*
 	 * AUFS support to compensate for the kernel bug
@@ -388,9 +378,6 @@ static int dump_filemap(pid_t pid, struct vma_area *vma_area,
 		p.link = &aufs_link;
 	}
 
-	if (get_fd_mntid(vma_area->vm_file_fd, &p.mnt_id))
-		return -1;
-
 	/* Flags will be set during restore in get_filemap_fd() */
 
 	if (fd_id_generate_special(&p, &id))
diff --git a/include/proc_parse.h b/include/proc_parse.h
index 7fade06..ab02f33 100644
--- a/include/proc_parse.h
+++ b/include/proc_parse.h
@@ -212,6 +212,7 @@ extern int parse_fdinfo_pid(int pid, int fd, int type,
 		int (*cb)(union fdinfo_entries *e, void *arg), void *arg);
 extern int parse_cpuinfo_features(int (*handler)(char *tok));
 extern int parse_file_locks(void);
+extern int get_fd_mntid(int fd, int *mnt_id);
 
 struct pid;
 extern int parse_threads(int pid, struct pid **_t, int *_n);
diff --git a/include/vma.h b/include/vma.h
index 0d7dcaf..d2ce80c 100644
--- a/include/vma.h
+++ b/include/vma.h
@@ -50,6 +50,7 @@ struct vma_area {
 			 */
 			bool		file_borrowed;
 			struct stat	*vmst;
+			int		mnt_id;
 		};
 
 		struct /* for restore */ {
diff --git a/proc_parse.c b/proc_parse.c
index 82af774..1ae2c52 100644
--- a/proc_parse.c
+++ b/proc_parse.c
@@ -214,6 +214,14 @@ static int vma_get_mapfile(struct vma_area *vma, DIR *mfd,
 		vma->vm_file_fd = prev->vm_file_fd;
 		if (prev->e->status & VMA_AREA_SOCKET)
 			vma->e->status |= VMA_AREA_SOCKET | VMA_AREA_REGULAR;
+
+		/*
+		 * FIXME -- in theory there can be vmas that have
+		 * dev:ino match, but live in different mount
+		 * namespaces. However, we only borrow files for
+		 * subsequent vmas. These are _very_ likely to
+		 * have files from the same namespaces.
+		 */
 		vma->file_borrowed = true;
 
 		return 0;
@@ -453,6 +461,7 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list, bool use_map_file
 			vma_area->e->status = prev->e->status;
 			vma_area->e->shmid = prev->e->shmid;
 			vma_area->vmst = prev->vmst;
+			vma_area->mnt_id = prev->mnt_id;
 		} else if (vma_area->vm_file_fd >= 0) {
 			struct stat *st_buf;
 
@@ -504,6 +513,9 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list, bool use_map_file
 				else
 					vma_area->e->status |= VMA_FILE_SHARED;
 			}
+
+			if (get_fd_mntid(vma_area->vm_file_fd, &vma_area->mnt_id))
+				return -1;
 		} else {
 			/*
 			 * No file but mapping -- anonymous one.
@@ -1410,6 +1422,17 @@ int parse_fdinfo(int fd, int type,
 	return parse_fdinfo_pid_s(PROC_SELF, fd, type, cb, arg);
 }
 
+int get_fd_mntid(int fd, int *mnt_id)
+{
+	struct fdinfo_common fdinfo = { .mnt_id = -1};
+
+	if (parse_fdinfo(fd, FD_TYPES__UND, NULL, &fdinfo))
+		return -1;
+
+	*mnt_id = fdinfo.mnt_id;
+	return 0;
+}
+
 static int parse_file_lock_buf(char *buf, struct file_lock *fl,
 				bool is_blocked)
 {
-- 
1.8.4.2