Skip to content

Commit

Permalink
memfd: don't reopen file descriptors for memory mappings
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
avagin committed Sep 23, 2023
1 parent 150eecc commit 1a94432
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
2 changes: 1 addition & 1 deletion criu/files-reg.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 3 additions & 1 deletion criu/include/memfd.h
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#ifndef __CR_MEMFD_H__
#define __CR_MEMFD_H__

#include <stdbool.h>
#include <sys/stat.h>

#include "int.h"
#include "common/config.h"

Expand All @@ -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);
Expand Down
12 changes: 8 additions & 4 deletions criu/memfd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
}

Expand All @@ -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);
Expand All @@ -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;

Expand Down

0 comments on commit 1a94432

Please sign in to comment.