From e651a6eba4d4608a4b8d35ba81bcbee399997760 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Tue, 23 Sep 2014 20:32:00 +0400 Subject: [PATCH] filemap: Get vma mnt_id early 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 --- 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 72eb94dd3..e873a3832 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 7fade0669..ab02f33b5 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 0d7dcaf0b..d2ce80c47 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 4f6c8d2dc..40c30228f 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; @@ -454,6 +462,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; @@ -505,6 +514,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. @@ -1423,6 +1435,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) {