From 1a944326df713b1bf24e0747b46862d4a62f688c Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Fri, 22 Sep 2023 22:40:27 +0000 Subject: [PATCH] memfd: don't reopen file descriptors for memory mappings One memfd can be shared by a few restored files. Only of these files is restored with a file created with memfd_open. Others are restored by reopening memfd files via /proc/self/fd/. It seems unnecessary for restoring memfd memory mappings. We can always use the origin file. Signed-off-by: Andrei Vagin --- criu/files-reg.c | 2 +- criu/include/memfd.h | 4 +++- criu/memfd.c | 12 ++++++++---- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/criu/files-reg.c b/criu/files-reg.c index cf0c84b52e..4456ec0d6f 100644 --- a/criu/files-reg.c +++ b/criu/files-reg.c @@ -2509,7 +2509,7 @@ static int open_filemap(int pid, struct vma_area *vma) ret = dup(plugin_fd); } else if (vma->e->status & VMA_AREA_MEMFD) { if (!inherited_fd(vma->vmfd, &ret)) - ret = memfd_open(vma->vmfd, &flags); + ret = memfd_open(vma->vmfd, &flags, true); } else { ret = open_path(vma->vmfd, do_open_reg_noseek_flags, &flags); } diff --git a/criu/include/memfd.h b/criu/include/memfd.h index 1b1dc79bbc..78d8100198 100644 --- a/criu/include/memfd.h +++ b/criu/include/memfd.h @@ -1,7 +1,9 @@ #ifndef __CR_MEMFD_H__ #define __CR_MEMFD_H__ +#include #include + #include "int.h" #include "common/config.h" @@ -12,7 +14,7 @@ extern int is_memfd(dev_t dev); extern int dump_one_memfd_cond(int lfd, u32 *id, struct fd_parms *parms); extern const struct fdtype_ops memfd_dump_ops; -extern int memfd_open(struct file_desc *d, u32 *fdflags); +extern int memfd_open(struct file_desc *d, u32 *fdflags, bool filemap); extern struct collect_image_info memfd_cinfo; extern struct file_desc *collect_memfd(u32 id); extern int apply_memfd_seals(void); diff --git a/criu/memfd.c b/criu/memfd.c index a770c66a11..c7ebf91e74 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -323,7 +323,7 @@ static int memfd_open_inode(struct memfd_restore_inode *inode) return fd; } -int memfd_open(struct file_desc *d, u32 *fdflags) +int memfd_open(struct file_desc *d, u32 *fdflags, bool filemap) { struct memfd_info *mfi; MemfdFileEntry *mfe; @@ -342,6 +342,9 @@ int memfd_open(struct file_desc *d, u32 *fdflags) /* Reopen the fd with original permissions */ flags = fdflags ? *fdflags : mfe->flags; + if (filemap && (flags & O_ACCMODE) == O_RDWR) + return fd; + if (!mfi->inode->was_opened_rw && (flags & O_ACCMODE) == O_RDWR) { /* * If there is only a single RW-opened fd for a memfd, it can @@ -352,7 +355,8 @@ int memfd_open(struct file_desc *d, u32 *fdflags) * and does execveat() from this memfd. */ if (!fcntl(fd, F_SETFL, flags)) { - mfi->inode->was_opened_rw = true; + if (!filemap) + mfi->inode->was_opened_rw = true; return fd; } @@ -367,7 +371,7 @@ int memfd_open(struct file_desc *d, u32 *fdflags) _fd = __open_proc(PROC_SELF, 0, flags, "fd/%d", fd); if (_fd < 0) pr_perror("Can't reopen memfd id=%d", mfe->id); - else if ((flags & O_ACCMODE) == O_RDWR) + else if (!filemap && (flags & O_ACCMODE) == O_RDWR) pr_warn("execveat(fd=%d, ..., AT_EMPTY_PATH) might fail after restore; memfd id=%d\n", _fd, mfe->id); close(fd); @@ -382,7 +386,7 @@ static int memfd_open_fe_fd(struct file_desc *d, int *new_fd) if (inherited_fd(d, new_fd)) return 0; - fd = memfd_open(d, NULL); + fd = memfd_open(d, NULL, false); if (fd < 0) return -1;