From 5dbc24b206cd365db7498dddcd03798c5d8ed4e4 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Mon, 25 Nov 2019 09:50:08 +0300 Subject: [PATCH 01/48] util: introduce the mount_detached_fs helper Signed-off-by: Andrei Vagin --- criu/util.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/criu/util.c b/criu/util.c index 1646ce1c4..a0a49c5a3 100644 --- a/criu/util.c +++ b/criu/util.c @@ -30,6 +30,8 @@ #include "linux/mount.h" +#include "linux/mount.h" + #include "kerndat.h" #include "page.h" #include "util.h" From c1e72aa936bd86fb4cd819e84791e8a9a5c4d572 Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Wed, 18 Dec 2019 23:32:32 +0000 Subject: [PATCH 02/48] memfd: add file support See "man memfd_create" for more information of what memfd is. This adds support for memfd open files, that are not not memory mapped. * We add a new kind of file: MEMFD. * We add two image types MEMFD_FILE, and MEMFD_INODE. MEMFD_FILE contains usual file information (e.g., position). MEMFD_INODE contains the memfd name, and a shmid identifier referring to the content. * We reuse the shmem facilities for dumping memfd content as it would be easier to support incremental checkpoints in the future. Signed-off-by: Nicolas Viennot --- Makefile.config | 2 +- criu/Makefile.crtools | 1 + criu/cr-restore.c | 2 + criu/files.c | 18 +- criu/image-desc.c | 1 + criu/include/image-desc.h | 2 + criu/include/magic.h | 1 + criu/include/memfd.h | 24 +++ criu/include/protobuf-desc.h | 2 + criu/include/shmem.h | 3 + criu/kerndat.c | 3 +- criu/memfd.c | 350 +++++++++++++++++++++++++++++++++++ criu/shmem.c | 66 ++++++- images/Makefile | 1 + images/fdinfo.proto | 3 + images/memfd.proto | 20 ++ lib/py/images/images.py | 2 + scripts/feature-tests.mak | 11 ++ 18 files changed, 503 insertions(+), 9 deletions(-) create mode 100644 criu/include/memfd.h create mode 100644 criu/memfd.c create mode 100644 images/memfd.proto diff --git a/Makefile.config b/Makefile.config index 161365960..98ba5d892 100644 --- a/Makefile.config +++ b/Makefile.config @@ -64,7 +64,7 @@ export DEFINES += $(FEATURE_DEFINES) export CFLAGS += $(FEATURE_DEFINES) FEATURES_LIST := TCP_REPAIR STRLCPY STRLCAT PTRACE_PEEKSIGINFO \ - SETPROCTITLE_INIT MEMFD TCP_REPAIR_WINDOW FSCONFIG + SETPROCTITLE_INIT MEMFD TCP_REPAIR_WINDOW FSCONFIG MEMFD_CREATE # $1 - config name define gen-feature-test diff --git a/criu/Makefile.crtools b/criu/Makefile.crtools index 4588ea5b8..1a6e0b5b5 100644 --- a/criu/Makefile.crtools +++ b/criu/Makefile.crtools @@ -37,6 +37,7 @@ obj-y += libnetlink.o obj-y += log.o obj-y += lsm.o obj-y += mem.o +obj-y += memfd.o obj-y += mount.o obj-y += filesystems.o obj-y += namespaces.o diff --git a/criu/cr-restore.c b/criu/cr-restore.c index e5e8fc9c5..13d1001c9 100644 --- a/criu/cr-restore.c +++ b/criu/cr-restore.c @@ -76,6 +76,7 @@ #include "sigframe.h" #include "fdstore.h" #include "string.h" +#include "memfd.h" #include "parasite-syscall.h" #include "files-reg.h" @@ -289,6 +290,7 @@ static struct collect_image_info *cinfos_files[] = { &fanotify_cinfo, &fanotify_mark_cinfo, &ext_file_cinfo, + &memfd_cinfo, }; /* These images are required to restore namespaces */ diff --git a/criu/files.c b/criu/files.c index e26897870..ea86deaa3 100644 --- a/criu/files.c +++ b/criu/files.c @@ -34,6 +34,7 @@ #include "sk-packet.h" #include "mount.h" #include "signalfd.h" +#include "memfd.h" #include "namespaces.h" #include "tun.h" #include "timerfd.h" @@ -546,13 +547,17 @@ static int dump_one_file(struct pid *pid, int fd, int lfd, struct fd_opts *opts, return -1; p.link = &link; - if (link.name[1] == '/') - return do_dump_gen_file(&p, lfd, ®file_dump_ops, e); - if (check_ns_proc(&link)) - return do_dump_gen_file(&p, lfd, &nsfile_dump_ops, e); + if (is_memfd(p.stat.st_dev, &link.name[1])) + ops = &memfd_dump_ops; + else if (link.name[1] == '/') + ops = ®file_dump_ops; + else if (check_ns_proc(&link)) + ops = &nsfile_dump_ops; + else + return dump_unsupp_fd(&p, lfd, "reg", link.name + 1, e); - return dump_unsupp_fd(&p, lfd, "reg", link.name + 1, e); + return do_dump_gen_file(&p, lfd, ops, e); } if (S_ISFIFO(p.stat.st_mode)) { @@ -1721,6 +1726,9 @@ static int collect_one_file(void *o, ProtobufCMessage *base, struct cr_img *i) case FD_TYPES__TTY: ret = collect_one_file_entry(fe, fe->tty->id, &fe->tty->base, &tty_cinfo); break; + case FD_TYPES__MEMFD: + ret = collect_one_file_entry(fe, fe->memfd->id, &fe->memfd->base, &memfd_cinfo); + break; } return ret; diff --git a/criu/image-desc.c b/criu/image-desc.c index ae5d817fe..b538a76ea 100644 --- a/criu/image-desc.c +++ b/criu/image-desc.c @@ -66,6 +66,7 @@ struct cr_fd_desc_tmpl imgset_template[CR_FD_MAX] = { FD_ENTRY(FS, "fs-%u"), FD_ENTRY(REMAP_FPATH, "remap-fpath"), FD_ENTRY_F(GHOST_FILE, "ghost-file-%x", O_NOBUF), + FD_ENTRY_F(MEMFD_INODE, "memfd-%u", O_NOBUF), FD_ENTRY(TCP_STREAM, "tcp-stream-%x"), FD_ENTRY(MNTS, "mountpoints-%u"), FD_ENTRY(NETDEV, "netdev-%u"), diff --git a/criu/include/image-desc.h b/criu/include/image-desc.h index 6db8bf94f..9ca9643a1 100644 --- a/criu/include/image-desc.h +++ b/criu/include/image-desc.h @@ -106,6 +106,8 @@ enum { CR_FD_FIFO, CR_FD_PIPES, CR_FD_TTY_FILES, + CR_FD_MEMFD_FILE, + CR_FD_MEMFD_INODE, CR_FD_AUTOFS, diff --git a/criu/include/magic.h b/criu/include/magic.h index 1a583f4ed..bdaca968d 100644 --- a/criu/include/magic.h +++ b/criu/include/magic.h @@ -94,6 +94,7 @@ #define BINFMT_MISC_MAGIC 0x67343323 /* Apatity */ #define AUTOFS_MAGIC 0x49353943 /* Sochi */ #define FILES_MAGIC 0x56303138 /* Toropets */ +#define MEMFD_INODE_MAGIC 0x48453499 /* Dnipro */ #define IFADDR_MAGIC RAW_IMAGE_MAGIC #define ROUTE_MAGIC RAW_IMAGE_MAGIC diff --git a/criu/include/memfd.h b/criu/include/memfd.h new file mode 100644 index 000000000..c1d7949cb --- /dev/null +++ b/criu/include/memfd.h @@ -0,0 +1,24 @@ +#ifndef __CR_MEMFD_H__ +#define __CR_MEMFD_H__ + +#include +#include "int.h" +#include "common/config.h" + +extern int is_memfd(dev_t dev, const char *path); +extern const struct fdtype_ops memfd_dump_ops; + +extern struct collect_image_info memfd_cinfo; + +#ifdef CONFIG_HAS_MEMFD_CREATE +# include +#else +# include +# include +static inline int memfd_create(const char *name, unsigned int flags) +{ + return syscall(SYS_memfd_create, name, flags); +} +#endif /* CONFIG_HAS_MEMFD_CREATE */ + +#endif /* __CR_MEMFD_H__ */ diff --git a/criu/include/protobuf-desc.h b/criu/include/protobuf-desc.h index 31f5b9a79..7e0385ef4 100644 --- a/criu/include/protobuf-desc.h +++ b/criu/include/protobuf-desc.h @@ -61,6 +61,8 @@ enum { PB_AUTOFS, PB_GHOST_CHUNK, PB_FILE, + PB_MEMFD_FILE, + PB_MEMFD_INODE, /* 60 */ /* PB_AUTOGEN_STOP */ diff --git a/criu/include/shmem.h b/criu/include/shmem.h index 04ab8d076..9afdb799a 100644 --- a/criu/include/shmem.h +++ b/criu/include/shmem.h @@ -13,8 +13,11 @@ extern int collect_sysv_shmem(unsigned long shmid, unsigned long size); extern int cr_dump_shmem(void); extern int add_shmem_area(pid_t pid, VmaEntry *vma, u64 *map); extern int fixup_sysv_shmems(void); +extern int dump_one_memfd_shmem(int fd, unsigned long shmid, unsigned long size); extern int dump_one_sysv_shmem(void *addr, unsigned long size, unsigned long shmid); extern int restore_sysv_shmem_content(void *addr, unsigned long size, unsigned long shmid); +extern int restore_memfd_shmem_content(int fd, unsigned long shmid, unsigned long size); + #define SYSV_SHMEM_SKIP_FD (0x7fffffff) diff --git a/criu/kerndat.c b/criu/kerndat.c index e0b5731d5..8ac83820b 100644 --- a/criu/kerndat.c +++ b/criu/kerndat.c @@ -42,6 +42,7 @@ #include "vdso.h" #include "kcmp.h" #include "sched.h" +#include "memfd.h" struct kerndat_s kdat = { }; @@ -409,7 +410,7 @@ static bool kerndat_has_memfd_create(void) { int ret; - ret = syscall(SYS_memfd_create, NULL, 0); + ret = memfd_create(NULL, 0); if (ret == -1 && errno == ENOSYS) kdat.has_memfd = false; diff --git a/criu/memfd.c b/criu/memfd.c new file mode 100644 index 000000000..bcef35e75 --- /dev/null +++ b/criu/memfd.c @@ -0,0 +1,350 @@ +#include + +#include "common/compiler.h" +#include "common/lock.h" +#include "memfd.h" +#include "fdinfo.h" +#include "imgset.h" +#include "image.h" +#include "util.h" +#include "log.h" +#include "files.h" +#include "fs-magic.h" +#include "kerndat.h" +#include "files-reg.h" +#include "rst-malloc.h" +#include "fdstore.h" +#include "file-ids.h" +#include "namespaces.h" +#include "shmem.h" + +#include "protobuf.h" +#include "images/memfd.pb-c.h" + +#define MEMFD_PREFIX "/memfd:" +#define MEMFD_PREFIX_LEN (sizeof(MEMFD_PREFIX)-1) + +struct memfd_inode { + struct list_head list; + u32 id; + union { + /* Only for dump */ + struct { + u32 dev; + u32 ino; + }; + /* Only for restore */ + struct { + mutex_t lock; + int fdstore_id; + }; + }; +}; + +static LIST_HEAD(memfd_inodes); + +/* + * Dump only + */ + +static u32 memfd_inode_ids = 1; + +int is_memfd(dev_t dev, const char *path) +{ + /* + * TODO When MAP_HUGETLB is used, the file device is not shmem_dev, + * Note that other parts of CRIU have similar issues, see + * is_anon_shmem_map(). + */ + return dev == kdat.shmem_dev && + !strncmp(path, MEMFD_PREFIX, MEMFD_PREFIX_LEN); +} + +static int dump_memfd_inode(int fd, struct memfd_inode *inode, + const char *name, const struct stat *st) +{ + int ret = -1; + struct cr_img *img = NULL; + MemfdInodeEntry mie = MEMFD_INODE_ENTRY__INIT; + u32 shmid; + + /* + * shmids are chosen as the inode number of the corresponding mmaped + * file. See handle_vma() in proc_parse.c. + * It works for memfd too, because we share the same device as the + * shmem device. + */ + shmid = inode->ino; + + pr_info("Dumping memfd:%s contents (id %#x, shmid: %#x, size: %"PRIu64")\n", + name, inode->id, shmid, st->st_size); + + if (dump_one_memfd_shmem(fd, shmid, st->st_size) < 0) + goto out; + + img = open_image(CR_FD_MEMFD_INODE, O_DUMP, inode->id); + if (!img) + goto out; + + mie.uid = userns_uid(st->st_uid); + mie.gid = userns_gid(st->st_gid); + mie.name = (char *)name; + mie.size = st->st_size; + mie.shmid = shmid; + + if (pb_write_one(img, &mie, PB_MEMFD_INODE)) + goto out; + + ret = 0; + +out: + if (img) + close_image(img); + return ret; +} + +static struct memfd_inode *dump_unique_memfd_inode(int lfd, const char *name, const struct stat *st) +{ + struct memfd_inode *inode; + + list_for_each_entry(inode, &memfd_inodes, list) + if ((inode->dev == st->st_dev) && (inode->ino == st->st_ino)) + return inode; + + inode = xmalloc(sizeof(*inode)); + if (inode == NULL) + return NULL; + + inode->dev = st->st_dev; + inode->ino = st->st_ino; + inode->id = memfd_inode_ids++; + + if (dump_memfd_inode(lfd, inode, name, st)) { + xfree(inode); + return NULL; + } + + list_add_tail(&inode->list, &memfd_inodes); + + return inode; +} + +static int dump_one_memfd(int lfd, u32 id, const struct fd_parms *p) +{ + MemfdFileEntry mfe = MEMFD_FILE_ENTRY__INIT; + FileEntry fe = FILE_ENTRY__INIT; + struct memfd_inode *inode; + struct fd_link _link, *link; + const char *name; + + if (!p->link) { + if (fill_fdlink(lfd, p, &_link)) + return -1; + link = &_link; + } else + link = p->link; + + strip_deleted(link); + name = &link->name[1+MEMFD_PREFIX_LEN]; + + inode = dump_unique_memfd_inode(lfd, name, &p->stat); + if (!inode) + return -1; + + mfe.id = id; + mfe.flags = p->flags; + mfe.pos = p->pos; + mfe.fown = (FownEntry *)&p->fown; + mfe.inode_id = inode->id; + + fe.type = FD_TYPES__MEMFD; + fe.id = mfe.id; + fe.memfd = &mfe; + + return pb_write_one(img_from_set(glob_imgset, CR_FD_FILES), &fe, PB_FILE); +} + +const struct fdtype_ops memfd_dump_ops = { + .type = FD_TYPES__MEMFD, + .dump = dump_one_memfd, +}; + + +/* + * Restore only + */ + +struct memfd_info { + MemfdFileEntry *mfe; + struct file_desc d; + struct memfd_inode *inode; +}; + +static struct memfd_inode *memfd_alloc_inode(int id) +{ + struct memfd_inode *inode; + + list_for_each_entry(inode, &memfd_inodes, list) + if (inode->id == id) + return inode; + + inode = shmalloc(sizeof(*inode)); + if (!inode) + return NULL; + + inode->id = id; + mutex_init(&inode->lock); + inode->fdstore_id = -1; + + list_add_tail(&inode->list, &memfd_inodes); + return inode; +} + +extern int restore_memfd_shm(int fd, u64 id, u64 size); +static int memfd_open_inode_nocache(struct memfd_inode *inode) +{ + MemfdInodeEntry *mie = NULL; + struct cr_img *img = NULL; + int fd = -1; + int ret = -1; + int flags; + + img = open_image(CR_FD_MEMFD_INODE, O_RSTR, inode->id); + if (!img) + goto out; + + if (pb_read_one(img, &mie, PB_MEMFD_INODE) < 0) + goto out; + + fd = memfd_create(mie->name, 0); + if (fd < 0) { + pr_perror("Can't create memfd:%s", mie->name); + goto out; + } + + if (restore_memfd_shmem_content(fd, mie->shmid, mie->size)) + goto out; + + if (fchown(fd, mie->uid, mie->gid)) { + pr_perror("Can't change uid %d gid %d of memfd:%s", + (int)mie->uid, (int)mie->gid, mie->name); + goto out; + } + + inode->fdstore_id = fdstore_add(fd); + if (inode->fdstore_id < 0) + goto out; + + ret = fd; + fd = -1; + +out: + if (fd != -1) + close(fd); + if (img) + close_image(img); + if (mie) + memfd_inode_entry__free_unpacked(mie, NULL); + return ret; +} + +static int memfd_open_inode(struct memfd_inode *inode) +{ + int fd; + + if (inode->fdstore_id != -1) + return fdstore_get(inode->fdstore_id); + + mutex_lock(&inode->lock); + if (inode->fdstore_id != -1) + fd = fdstore_get(inode->fdstore_id); + else + fd = memfd_open_inode_nocache(inode); + mutex_unlock(&inode->lock); + + return fd; +} + +static int memfd_open(struct file_desc *d, u32 *fdflags) +{ + char lpath[PSFDS]; + struct memfd_info *mfi; + MemfdFileEntry *mfe; + int fd, _fd; + u32 flags; + + mfi = container_of(d, struct memfd_info, d); + mfe = mfi->mfe; + + pr_info("Restoring memfd id=%d\n", mfe->id); + + fd = memfd_open_inode(mfi->inode); + if (fd < 0) + goto err; + + /* Reopen the fd with original permissions */ + sprintf(lpath, "/proc/self/fd/%d", fd); + flags = fdflags ? *fdflags : mfe->flags; + /* + * Ideally we should call compat version open() to not force the + * O_LARGEFILE file flag with regular open(). It doesn't seem that + * important though. + */ + _fd = open(lpath, flags); + if (_fd < 0) { + pr_perror("Can't reopen memfd id=%d", mfe->id); + goto err; + } + close(fd); + fd = _fd; + + if (restore_fown(fd, mfe->fown) < 0) + goto err; + + if (lseek(fd, mfe->pos, SEEK_SET) < 0) { + pr_perror("Can't restore file position of memfd id=%d", mfe->id); + goto err; + } + + return fd; + +err: + if (fd >= 0) + close(fd); + return -1; +} + +static int memfd_open_fe_fd(struct file_desc *fd, int *new_fd) +{ + int tmp; + + tmp = memfd_open(fd, NULL); + if (tmp < 0) + return -1; + *new_fd = tmp; + return 0; +} + +static struct file_desc_ops memfd_desc_ops = { + .type = FD_TYPES__MEMFD, + .open = memfd_open_fe_fd, +}; + +static int collect_one_memfd(void *o, ProtobufCMessage *msg, struct cr_img *i) +{ + struct memfd_info *info = o; + + info->mfe = pb_msg(msg, MemfdFileEntry); + info->inode = memfd_alloc_inode(info->mfe->inode_id); + if (!info->inode) + return -1; + + return file_desc_add(&info->d, info->mfe->id, &memfd_desc_ops); +} + +struct collect_image_info memfd_cinfo = { + .fd_type = CR_FD_MEMFD_FILE, + .pb_type = PB_MEMFD_FILE, + .priv_size = sizeof(struct memfd_info), + .collect = collect_one_memfd, +}; diff --git a/criu/shmem.c b/criu/shmem.c index cee47dba7..29383e79a 100644 --- a/criu/shmem.c +++ b/criu/shmem.c @@ -23,6 +23,7 @@ #include "types.h" #include "page.h" #include "util.h" +#include "memfd.h" #include "protobuf.h" #include "images/pagemap.pb-c.h" @@ -490,7 +491,7 @@ static int do_restore_shmem_content(void *addr, unsigned long size, unsigned lon return ret; } -static int restore_shmem_content(void *addr, struct shmem_info *si) +int restore_shmem_content(void *addr, struct shmem_info *si) { return do_restore_shmem_content(addr, si->size, si->shmid); } @@ -500,6 +501,41 @@ int restore_sysv_shmem_content(void *addr, unsigned long size, unsigned long shm return do_restore_shmem_content(addr, round_up(size, PAGE_SIZE), shmid); } +int restore_memfd_shmem_content(int fd, unsigned long shmid, unsigned long size) +{ + void *addr = NULL; + int ret = 1; + + if (size == 0) + return 0; + + if (ftruncate(fd, size) < 0) { + pr_perror("Can't resize shmem 0x%lx size=%ld", shmid, size); + goto out; + } + + addr = mmap(NULL, size, PROT_WRITE | PROT_READ, MAP_SHARED, fd, 0); + if (addr == MAP_FAILED) { + pr_perror("Can't mmap shmem 0x%lx size=%ld", shmid, size); + goto out; + } + + /* + * do_restore_shmem_content needs size to be page aligned. + */ + if (do_restore_shmem_content(addr, round_up(size, PAGE_SIZE), shmid) < 0) { + pr_err("Can't restore shmem content\n"); + goto out; + } + + ret = 0; + +out: + if (addr) + munmap(addr, size); + return ret; +} + static int open_shmem(int pid, struct vma_area *vma) { VmaEntry *vi = vma->e; @@ -532,7 +568,7 @@ static int open_shmem(int pid, struct vma_area *vma) flags = MAP_SHARED; if (kdat.has_memfd) { - f = syscall(SYS_memfd_create, "", 0); + f = memfd_create("", 0); if (f < 0) { pr_perror("Unable to create memfd"); goto err; @@ -779,6 +815,32 @@ err: return ret; } +int dump_one_memfd_shmem(int fd, unsigned long shmid, unsigned long size) +{ + int ret = -1; + void *addr; + struct shmem_info si; + + if (size == 0) + return 0; + + memset(&si, 0, sizeof(si)); + si.shmid = shmid; + si.size = size; + + addr = mmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0); + if (addr == MAP_FAILED) { + pr_perror("Can't mmap shmem 0x%lx", shmid); + goto err; + } + + ret = do_dump_one_shmem(fd, addr, &si); + + munmap(addr, size); +err: + return ret; +} + int dump_one_sysv_shmem(void *addr, unsigned long size, unsigned long shmid) { int fd, ret; diff --git a/images/Makefile b/images/Makefile index edaab0633..e7f0580cf 100644 --- a/images/Makefile +++ b/images/Makefile @@ -63,6 +63,7 @@ proto-obj-y += sysctl.o proto-obj-y += autofs.o proto-obj-y += macvlan.o proto-obj-y += sit.o +proto-obj-y += memfd.o CFLAGS += -iquote $(obj)/ diff --git a/images/fdinfo.proto b/images/fdinfo.proto index 77e375aa9..d966d5bc5 100644 --- a/images/fdinfo.proto +++ b/images/fdinfo.proto @@ -16,6 +16,7 @@ import "sk-unix.proto"; import "fifo.proto"; import "pipe.proto"; import "tty.proto"; +import "memfd.proto"; enum fd_types { UND = 0; @@ -36,6 +37,7 @@ enum fd_types { TUNF = 15; EXT = 16; TIMERFD = 17; + MEMFD = 18; /* Any number above the real used. Not stored to image */ CTL_TTY = 65534; @@ -70,4 +72,5 @@ message file_entry { optional fifo_entry fifo = 17; optional pipe_entry pipe = 18; optional tty_file_entry tty = 19; + optional memfd_file_entry memfd = 20; } diff --git a/images/memfd.proto b/images/memfd.proto new file mode 100644 index 000000000..8eccd6f4f --- /dev/null +++ b/images/memfd.proto @@ -0,0 +1,20 @@ +syntax = "proto2"; + +import "opts.proto"; +import "fown.proto"; + +message memfd_file_entry { + required uint32 id = 1; + required uint32 flags = 2 [(criu).flags = "rfile.flags"]; + required uint64 pos = 3; + required fown_entry fown = 4; + required uint32 inode_id = 5; +}; + +message memfd_inode_entry { + required string name = 1; + required uint32 uid = 2; + required uint32 gid = 3; + required uint64 size = 4; + required uint32 shmid = 5; +}; diff --git a/lib/py/images/images.py b/lib/py/images/images.py index 3eedfca69..dca080657 100644 --- a/lib/py/images/images.py +++ b/lib/py/images/images.py @@ -522,6 +522,8 @@ handlers = { 'AUTOFS': entry_handler(pb.autofs_entry), 'FILES': entry_handler(pb.file_entry), 'CPUINFO': entry_handler(pb.cpuinfo_entry), + 'MEMFD_FILE': entry_handler(pb.memfd_file_entry), + 'MEMFD_INODE': entry_handler(pb.memfd_inode_entry), } diff --git a/scripts/feature-tests.mak b/scripts/feature-tests.mak index 6f67c6035..21b390092 100644 --- a/scripts/feature-tests.mak +++ b/scripts/feature-tests.mak @@ -171,3 +171,14 @@ int main(int argc, char **argv) } endef + +define FEATURE_TEST_MEMFD_CREATE + +#include +#include + +int main(void) +{ + return memfd_create(NULL, 0); +} +endef From 875ac4d03f9034adb88eec5875d63d0561c48107 Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Wed, 18 Dec 2019 23:32:32 +0000 Subject: [PATCH 03/48] files: increase path buffer size in inherited_fd() Prepare memfd to use inherited_fd(), needing long path names support. Signed-off-by: Nicolas Viennot --- criu/files.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/criu/files.c b/criu/files.c index ea86deaa3..789b08a4c 100644 --- a/criu/files.c +++ b/criu/files.c @@ -1608,7 +1608,7 @@ int inherit_fd_lookup_id(char *id) bool inherited_fd(struct file_desc *d, int *fd_p) { - char buf[32], *id_str; + char buf[PATH_MAX], *id_str; int i_fd; if (!d->ops->name) From b25684e24ae7643f2a8da73617c22a44dc9023ca Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Wed, 18 Dec 2019 23:32:32 +0000 Subject: [PATCH 04/48] memfd: add --inherit-fd support Upon file restore, inherited_fd() is called to check for a user-defined inerit-fd override. Note that the MEMFD_INODE image is read at each invocation (memfd name is not cached). Signed-off-by: Nicolas Viennot --- criu/crtools.c | 1 + criu/memfd.c | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/criu/crtools.c b/criu/crtools.c index 3cd40e87d..7f72dde27 100644 --- a/criu/crtools.c +++ b/criu/crtools.c @@ -392,6 +392,7 @@ usage: " pipe[inode]\n" " socket[inode]\n" " file[mnt_id:inode]\n" +" /memfd:name\n" " path/to/file\n" " --empty-ns net Create a namespace, but don't restore its properties\n" " (assuming it will be restored by action scripts)\n" diff --git a/criu/memfd.c b/criu/memfd.c index bcef35e75..36b3be8df 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -276,6 +276,9 @@ static int memfd_open(struct file_desc *d, u32 *fdflags) mfi = container_of(d, struct memfd_info, d); mfe = mfi->mfe; + if (inherited_fd(d, &fd)) + return fd; + pr_info("Restoring memfd id=%d\n", mfe->id); fd = memfd_open_inode(mfi->inode); @@ -325,9 +328,42 @@ static int memfd_open_fe_fd(struct file_desc *fd, int *new_fd) return 0; } +static char *memfd_d_name(struct file_desc *d, char *buf, size_t s) +{ + MemfdInodeEntry *mie = NULL; + struct cr_img *img = NULL; + struct memfd_info *mfi; + char *ret = NULL; + + mfi = container_of(d, struct memfd_info, d); + + img = open_image(CR_FD_MEMFD_INODE, O_RSTR, mfi->inode->id); + if (!img) + goto out; + + if (pb_read_one(img, &mie, PB_MEMFD_INODE) < 0) + goto out; + + if (snprintf(buf, s, "%s%s", MEMFD_PREFIX, mie->name) >= s) { + pr_err("Buffer too small for memfd name %s\n", mie->name); + goto out; + } + + ret = buf; + +out: + if (img) + close_image(img); + if (mie) + memfd_inode_entry__free_unpacked(mie, NULL); + + return ret; +} + static struct file_desc_ops memfd_desc_ops = { .type = FD_TYPES__MEMFD, .open = memfd_open_fe_fd, + .name = memfd_d_name, }; static int collect_one_memfd(void *o, ProtobufCMessage *msg, struct cr_img *i) From 29a1a88bcebaf9d83591077d2bec424da82c0e71 Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Wed, 18 Dec 2019 23:32:32 +0000 Subject: [PATCH 05/48] memfd: add memory mapping support * During checkpoint, we add a vma flags: VMA_AREA_MEMFD to denote memfd regions. * Even though memfd is backed by the shmem device, we use the file semantics of memfd (via /proc/map_files/) which we already have support for. Signed-off-by: Nicolas Viennot --- criu/cr-dump.c | 6 +++++- criu/files-reg.c | 11 +++++++++-- criu/include/image.h | 1 + criu/include/memfd.h | 6 ++++++ criu/memfd.c | 19 ++++++++++++++++++- criu/proc_parse.c | 35 +++++++++++++++++++++++++++++++++++ 6 files changed, 74 insertions(+), 4 deletions(-) diff --git a/criu/cr-dump.c b/criu/cr-dump.c index 88323af92..6aa114c2d 100644 --- a/criu/cr-dump.c +++ b/criu/cr-dump.c @@ -80,6 +80,7 @@ #include "fault-injection.h" #include "dump.h" #include "eventpoll.h" +#include "memfd.h" /* * Architectures can overwrite this function to restore register sets that @@ -414,7 +415,10 @@ static int dump_filemap(struct vma_area *vma_area, int fd) /* Flags will be set during restore in open_filmap() */ - ret = dump_one_reg_file_cond(fd, &id, &p); + if (vma->status & VMA_AREA_MEMFD) + ret = dump_one_memfd_cond(fd, &id, &p); + else + ret = dump_one_reg_file_cond(fd, &id, &p); vma->shmid = id; return ret; diff --git a/criu/files-reg.c b/criu/files-reg.c index 90fb7dd7f..b0dad78e6 100644 --- a/criu/files-reg.c +++ b/criu/files-reg.c @@ -35,6 +35,7 @@ #include "pstree.h" #include "fault-injection.h" #include "external.h" +#include "memfd.h" #include "protobuf.h" #include "util.h" @@ -1879,7 +1880,10 @@ static int open_filemap(int pid, struct vma_area *vma) flags = vma->e->fdflags; if (ctx.flags != flags || ctx.desc != vma->vmfd) { - ret = open_path(vma->vmfd, do_open_reg_noseek_flags, &flags); + if (vma->e->status & VMA_AREA_MEMFD) + ret = memfd_open(vma->vmfd, &flags); + else + ret = open_path(vma->vmfd, do_open_reg_noseek_flags, &flags); if (ret < 0) return ret; @@ -1909,7 +1913,10 @@ int collect_filemap(struct vma_area *vma) vma->e->fdflags = O_RDONLY; } - fd = collect_special_file(vma->e->shmid); + if (vma->e->status & VMA_AREA_MEMFD) + fd = collect_memfd(vma->e->shmid); + else + fd = collect_special_file(vma->e->shmid); if (!fd) return -1; diff --git a/criu/include/image.h b/criu/include/image.h index 2baa39496..1c7cc5471 100644 --- a/criu/include/image.h +++ b/criu/include/image.h @@ -83,6 +83,7 @@ #define VMA_AREA_SOCKET (1 << 11) #define VMA_AREA_VVAR (1 << 12) #define VMA_AREA_AIORING (1 << 13) +#define VMA_AREA_MEMFD (1 << 14) #define VMA_CLOSE (1 << 28) #define VMA_NO_PROT_WRITE (1 << 29) diff --git a/criu/include/memfd.h b/criu/include/memfd.h index c1d7949cb..0a9aeff2f 100644 --- a/criu/include/memfd.h +++ b/criu/include/memfd.h @@ -5,10 +5,16 @@ #include "int.h" #include "common/config.h" +struct fd_parms; +struct file_desc; + extern int is_memfd(dev_t dev, const char *path); +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 struct collect_image_info memfd_cinfo; +extern struct file_desc *collect_memfd(u32 id); #ifdef CONFIG_HAS_MEMFD_CREATE # include diff --git a/criu/memfd.c b/criu/memfd.c index 36b3be8df..1cca96a32 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -164,6 +164,13 @@ static int dump_one_memfd(int lfd, u32 id, const struct fd_parms *p) return pb_write_one(img_from_set(glob_imgset, CR_FD_FILES), &fe, PB_FILE); } +int dump_one_memfd_cond(int lfd, u32 *id, struct fd_parms *parms) +{ + if (fd_id_generate_special(parms, id)) + return dump_one_memfd(lfd, *id, parms); + return 0; +} + const struct fdtype_ops memfd_dump_ops = { .type = FD_TYPES__MEMFD, .dump = dump_one_memfd, @@ -265,7 +272,7 @@ static int memfd_open_inode(struct memfd_inode *inode) return fd; } -static int memfd_open(struct file_desc *d, u32 *fdflags) +int memfd_open(struct file_desc *d, u32 *fdflags) { char lpath[PSFDS]; struct memfd_info *mfi; @@ -384,3 +391,13 @@ struct collect_image_info memfd_cinfo = { .priv_size = sizeof(struct memfd_info), .collect = collect_one_memfd, }; + +struct file_desc *collect_memfd(u32 id) { + struct file_desc *fdesc; + + fdesc = find_file_desc_raw(FD_TYPES__MEMFD, id); + if (fdesc == NULL) + pr_err("No entry for memfd %#x\n", id); + + return fdesc; +} diff --git a/criu/proc_parse.c b/criu/proc_parse.c index fa7644992..468afcdf3 100644 --- a/criu/proc_parse.c +++ b/criu/proc_parse.c @@ -41,6 +41,7 @@ #include "timerfd.h" #include "path.h" #include "fault-injection.h" +#include "memfd.h" #include "protobuf.h" #include "images/fdinfo.pb-c.h" @@ -303,6 +304,26 @@ static int vma_get_mapfile_user(const char *fname, struct vma_area *vma, } vfi_dev = makedev(vfi->dev_maj, vfi->dev_min); + + if (is_memfd(vfi_dev, fname)) { + struct fd_link link; + link.len = strlen(fname); + strlcpy(link.name, fname, sizeof(link.name)); + strip_deleted(&link); + + /* + * The error EPERM will be shown in the following pr_perror(). + * It comes from the previous open() call. + */ + pr_perror("Can't open mapped [%s]", link.name); + + /* + * TODO Perhaps we could do better than failing and dump the + * memory like what is being done in shmem.c + */ + return -1; + } + if (is_anon_shmem_map(vfi_dev)) { if (!(vma->e->flags & MAP_SHARED)) return -1; @@ -578,7 +599,20 @@ static int handle_vma(pid_t pid, struct vma_area *vma_area, /* * /dev/zero stands for anon-shared mapping * otherwise it's some file mapping. + * + * We treat memfd mappings as regular file mappings because + * their backing can be seen as files, which is easy to + * support. So even though memfd is an anonymous shmem, we + * treat it differently. + * Note: maybe we should revisit this as /proc/map_files/ + * may not always be accessible. */ + + if (is_memfd(st_buf->st_dev, file_path)) { + vma_area->e->status |= VMA_AREA_MEMFD; + goto normal_file; + } + if (is_anon_shmem_map(st_buf->st_dev)) { if (!(vma_area->e->flags & MAP_SHARED)) goto err_bogus_mapping; @@ -594,6 +628,7 @@ static int handle_vma(pid_t pid, struct vma_area *vma_area, vma_area->e->shmid += FI_HUGE_ANON_SHMEM_ID_BASE; } } else { +normal_file: if (vma_area->e->flags & MAP_PRIVATE) vma_area->e->status |= VMA_FILE_PRIVATE; else From 56d8e2455fb86b885775db6c236cbb04ba403f4d Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Wed, 18 Dec 2019 23:32:32 +0000 Subject: [PATCH 06/48] memfd: add seals support See "man fcntl" for more information about seals. memfd are the only files that can be sealed, currently. For this reason, we dump the seal values in the MEMFD_INODE image. Restoring seals must be done carefully as the seal F_SEAL_FUTURE_WRITE prevents future write access. This means that any memory mapping with write access must be restored before restoring the seals. Signed-off-by: Nicolas Viennot --- criu/cr-restore.c | 4 +++ criu/include/fcntl.h | 8 ++++++ criu/include/memfd.h | 1 + criu/memfd.c | 59 +++++++++++++++++++++++++++++++++++++++- images/memfd.proto | 1 + lib/py/images/pb2dict.py | 9 ++++++ 6 files changed, 81 insertions(+), 1 deletion(-) diff --git a/criu/cr-restore.c b/criu/cr-restore.c index 13d1001c9..f50448cd2 100644 --- a/criu/cr-restore.c +++ b/criu/cr-restore.c @@ -2232,6 +2232,10 @@ skip_ns_bouncing: if (ret < 0) goto out_kill; + ret = apply_memfd_seals(); + if (ret < 0) + goto out_kill; + /* * Zombies die after CR_STATE_RESTORE which is switched * by root task, not by us. See comment before CR_STATE_FORKING diff --git a/criu/include/fcntl.h b/criu/include/fcntl.h index d9c5c5e7b..ea9d48c72 100644 --- a/criu/include/fcntl.h +++ b/criu/include/fcntl.h @@ -34,6 +34,14 @@ struct f_owner_ex { # define F_GETPIPE_SZ (F_LINUX_SPECIFIC_BASE + 8) #endif +#ifndef F_ADD_SEALS +# define F_ADD_SEALS (F_LINUX_SPECIFIC_BASE + 9) +#endif + +#ifndef F_GET_SEALS +# define F_GET_SEALS (F_LINUX_SPECIFIC_BASE + 10) +#endif + #ifndef O_PATH # define O_PATH 010000000 #endif diff --git a/criu/include/memfd.h b/criu/include/memfd.h index 0a9aeff2f..2d8eda545 100644 --- a/criu/include/memfd.h +++ b/criu/include/memfd.h @@ -15,6 +15,7 @@ extern const struct fdtype_ops memfd_dump_ops; extern int memfd_open(struct file_desc *d, u32 *fdflags); extern struct collect_image_info memfd_cinfo; extern struct file_desc *collect_memfd(u32 id); +extern int apply_memfd_seals(void); #ifdef CONFIG_HAS_MEMFD_CREATE # include diff --git a/criu/memfd.c b/criu/memfd.c index 1cca96a32..d17c10fb7 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -1,4 +1,5 @@ #include +#include #include "common/compiler.h" #include "common/lock.h" @@ -24,6 +25,13 @@ #define MEMFD_PREFIX "/memfd:" #define MEMFD_PREFIX_LEN (sizeof(MEMFD_PREFIX)-1) +#define F_SEAL_SEAL 0x0001 /* prevent further seals from being set */ +#define F_SEAL_SHRINK 0x0002 /* prevent file from shrinking */ +#define F_SEAL_GROW 0x0004 /* prevent file from growing */ +#define F_SEAL_WRITE 0x0008 /* prevent writes */ +/* Linux 5.1+ */ +#define F_SEAL_FUTURE_WRITE 0x0010 /* prevent future writes while mapped */ + struct memfd_inode { struct list_head list; u32 id; @@ -37,6 +45,7 @@ struct memfd_inode { struct { mutex_t lock; int fdstore_id; + unsigned int pending_seals; }; }; }; @@ -92,6 +101,10 @@ static int dump_memfd_inode(int fd, struct memfd_inode *inode, mie.size = st->st_size; mie.shmid = shmid; + mie.seals = fcntl(fd, F_GET_SEALS); + if (mie.seals == -1) + goto out; + if (pb_write_one(img, &mie, PB_MEMFD_INODE)) goto out; @@ -187,6 +200,8 @@ struct memfd_info { struct memfd_inode *inode; }; +static int memfd_open_inode(struct memfd_inode *inode); + static struct memfd_inode *memfd_alloc_inode(int id) { struct memfd_inode *inode; @@ -202,6 +217,7 @@ static struct memfd_inode *memfd_alloc_inode(int id) inode->id = id; mutex_init(&inode->lock); inode->fdstore_id = -1; + inode->pending_seals = 0; list_add_tail(&inode->list, &memfd_inodes); return inode; @@ -223,7 +239,16 @@ static int memfd_open_inode_nocache(struct memfd_inode *inode) if (pb_read_one(img, &mie, PB_MEMFD_INODE) < 0) goto out; - fd = memfd_create(mie->name, 0); + if (mie->seals == F_SEAL_SEAL) { + inode->pending_seals = 0; + flags = 0; + } else { + /* Seals are applied later due to F_SEAL_FUTURE_WRITE */ + inode->pending_seals = mie->seals; + flags = MFD_ALLOW_SEALING; + } + + fd = memfd_create(mie->name, flags); if (fd < 0) { pr_perror("Can't create memfd:%s", mie->name); goto out; @@ -401,3 +426,35 @@ struct file_desc *collect_memfd(u32 id) { return fdesc; } + +int apply_memfd_seals(void) +{ + /* + * We apply the seals after all the mappings are done because the seal + * F_SEAL_FUTURE_WRITE prevents future write access (added in + * Linux 5.1). Thus we must make sure all writable mappings are opened + * before applying this seal. + */ + + int ret, fd; + struct memfd_inode *inode; + + list_for_each_entry(inode, &memfd_inodes, list) { + if (!inode->pending_seals) + continue; + + fd = memfd_open_inode(inode); + if (fd < 0) + return -1; + + ret = fcntl(fd, F_ADD_SEALS, inode->pending_seals); + close(fd); + + if (ret < 0) { + pr_perror("Cannot apply seals on memfd"); + return -1; + } + } + + return 0; +} diff --git a/images/memfd.proto b/images/memfd.proto index 8eccd6f4f..546ffc2ab 100644 --- a/images/memfd.proto +++ b/images/memfd.proto @@ -17,4 +17,5 @@ message memfd_inode_entry { required uint32 gid = 3; required uint64 size = 4; required uint32 shmid = 5; + required uint32 seals = 6 [(criu).flags = "seals.flags"]; }; diff --git a/lib/py/images/pb2dict.py b/lib/py/images/pb2dict.py index a89850a1d..40a6036cf 100644 --- a/lib/py/images/pb2dict.py +++ b/lib/py/images/pb2dict.py @@ -124,6 +124,14 @@ rfile_flags_map = [ ('O_CLOEXEC', 0o02000000), ] +seals_flags_map = [ + ('F_SEAL_SEAL', 0x0001), + ('F_SEAL_SHRINK', 0x0002), + ('F_SEAL_GROW', 0x0004), + ('F_SEAL_WRITE', 0x0008), + ('F_SEAL_FUTURE_WRITE', 0x0010), +] + pmap_flags_map = [ ('PE_PARENT', 1 << 0), ('PE_LAZY', 1 << 1), @@ -136,6 +144,7 @@ flags_maps = { 'mmap.status': mmap_status_map, 'rfile.flags': rfile_flags_map, 'pmap.flags': pmap_flags_map, + 'seals.flags': seals_flags_map, } gen_maps = { From b133c375ad2d21cf6a1a9e96e7dab3741c966fbe Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Tue, 4 Feb 2020 16:39:53 +0000 Subject: [PATCH 07/48] inhfd_test: add support for non-pair files File pairs naturally block on read() until the write() happen (or the writer is closed). This is not the case for regular files, so we take extra precaution for these. Also cleaned-up an extra my_file.close() Signed-off-by: Nicolas Viennot --- test/zdtm.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/zdtm.py b/test/zdtm.py index 3fc57ba55..4110b5142 100755 --- a/test/zdtm.py +++ b/test/zdtm.py @@ -691,9 +691,14 @@ class inhfd_test: i = 0 for _, peer_file in self.__files: msg = self.__get_message(i) - my_file.close() try: - data = peer_file.read(16) + # File pairs naturally block on read() until the write() + # happen (or the writer is closed). This is not the case for + # regular files, so we loop. + data = b'' + while not data: + data = peer_file.read(16) + time.sleep(0.1) except Exception as e: print("Unable to read a peer file: %s" % e) sys.exit(1) From 2dd105b8dfb23399e18ab4e3f7d13b00c19ad910 Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Fri, 20 Dec 2019 21:56:38 -0500 Subject: [PATCH 08/48] memfd: add tests Testing for all the memfd features, namely support for CR of: * the same fd shared by multiple processes * the same file shared by multiple processes * the memfd content * file flags and fd flags * mmaps, MAP_SHARED and MAP_PRIVATE * seals, excluding F_SEAL_FUTURE_WRITE because this feature only exists in recent kernels (5.1 and up) * inherited fd Signed-off-by: Nicolas Viennot --- test/inhfd/memfd.py | 28 +++++++++ test/inhfd/memfd.py.checkskip | 7 +++ test/inhfd/memfd.py.desc | 1 + test/zdtm/static/Makefile | 4 ++ test/zdtm/static/memfd00.c | 103 ++++++++++++++++++++++++++++++ test/zdtm/static/memfd01.c | 114 ++++++++++++++++++++++++++++++++++ test/zdtm/static/memfd02.c | 87 ++++++++++++++++++++++++++ test/zdtm/static/memfd03.c | 97 +++++++++++++++++++++++++++++ 8 files changed, 441 insertions(+) create mode 100755 test/inhfd/memfd.py create mode 100755 test/inhfd/memfd.py.checkskip create mode 100644 test/inhfd/memfd.py.desc create mode 100644 test/zdtm/static/memfd00.c create mode 100644 test/zdtm/static/memfd01.c create mode 100644 test/zdtm/static/memfd02.c create mode 100644 test/zdtm/static/memfd03.c diff --git a/test/inhfd/memfd.py b/test/inhfd/memfd.py new file mode 100755 index 000000000..d9ce01e41 --- /dev/null +++ b/test/inhfd/memfd.py @@ -0,0 +1,28 @@ +import os +import ctypes +libc = ctypes.CDLL(None) + + +def memfd_create(name, flags): + return libc.memfd_create(name.encode('utf8'), flags) + + +def create_fds(): + def create_memfd_pair(name): + fd = memfd_create(name, 0) + fw = open('/proc/self/fd/{}'.format(fd), 'wb') + fr = open('/proc/self/fd/{}'.format(fd), 'rb') + os.close(fd) + return (fw, fr) + + return [create_memfd_pair("name{}".format(i)) for i in range(10)] + + +def filename(f): + name = os.readlink('/proc/self/fd/{}'.format(f.fileno())) + name = name.replace(' (deleted)', '') + return name + + +def dump_opts(sockf): + return [] diff --git a/test/inhfd/memfd.py.checkskip b/test/inhfd/memfd.py.checkskip new file mode 100755 index 000000000..252778969 --- /dev/null +++ b/test/inhfd/memfd.py.checkskip @@ -0,0 +1,7 @@ +#!/usr/bin/env python + +import ctypes +libc = ctypes.CDLL(None) + +# libc may not have memfd_create (e.g., centos on travis) +libc.memfd_create("test".encode('utf8'), 0) diff --git a/test/inhfd/memfd.py.desc b/test/inhfd/memfd.py.desc new file mode 100644 index 000000000..10666c823 --- /dev/null +++ b/test/inhfd/memfd.py.desc @@ -0,0 +1 @@ +{ 'flavor': 'h' } diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile index 5ca05ee9e..5afd18cd6 100644 --- a/test/zdtm/static/Makefile +++ b/test/zdtm/static/Makefile @@ -220,6 +220,10 @@ TST_NOFILE := \ child_subreaper \ child_subreaper_existing_child \ child_subreaper_and_reparent \ + memfd00 \ + memfd01 \ + memfd02 \ + memfd03 \ # jobctl00 \ ifneq ($(ARCH),arm) diff --git a/test/zdtm/static/memfd00.c b/test/zdtm/static/memfd00.c new file mode 100644 index 000000000..6b56eca01 --- /dev/null +++ b/test/zdtm/static/memfd00.c @@ -0,0 +1,103 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "zdtmtst.h" + +const char *test_doc = "memfd file descriptor"; +const char *test_author = "Nicolas Viennot "; + +#define err(exitcode, msg, ...) ({ pr_perror(msg, ##__VA_ARGS__); exit(exitcode); }) + +static int _memfd_create(const char *name, unsigned int flags) +{ + return syscall(SYS_memfd_create, name, flags); +} + +int main(int argc, char *argv[]) +{ + int fd, fl_flags1, fl_flags2, fd_flags1, fd_flags2; + struct statfs statfs1, statfs2; + off_t pos1, pos2; + char buf[5]; + + test_init(argc, argv); + + fd = _memfd_create("somename", MFD_CLOEXEC); + if (fd < 0) + err(1, "Can't call memfd_create"); + + if (fcntl(fd, F_SETFL, O_APPEND) < 0) + err(1, "Can't get fl flags"); + + if ((fl_flags1 = fcntl(fd, F_GETFL)) == -1) + err(1, "Can't get fl flags"); + + if ((fd_flags1 = fcntl(fd, F_GETFD)) == -1) + err(1, "Can't get fd flags"); + + if (fstatfs(fd, &statfs1) < 0) + err(1, "statfs issue"); + + if (write(fd, "hello", 5) != 5) + err(1, "write error"); + + pos1 = 3; + if (lseek(fd, pos1, SEEK_SET) < 0) + err(1, "seek error"); + + test_daemon(); + test_waitsig(); + + if ((fl_flags2 = fcntl(fd, F_GETFL)) == -1) + err(1, "Can't get fl flags"); + + if (fl_flags1 != fl_flags2) { + fail("fl flags differs"); + return 1; + } + + if ((fd_flags2 = fcntl(fd, F_GETFD)) == -1) + err(1, "Can't get fd flags"); + + if (fd_flags1 != fd_flags2) { + fail("fd flags differs"); + return 1; + } + + if (fstatfs(fd, &statfs2) < 0) + err(1, "statfs issue"); + + if (statfs1.f_type != statfs2.f_type) { + fail("statfs.f_type differs"); + return 1; + } + + pos2 = lseek(fd, 0, SEEK_CUR); + if (pos1 != pos2) { + fail("position differs"); + return 1; + } + + if (pread(fd, buf, sizeof(buf), 0) != sizeof(buf)) { + fail("read problem"); + return 1; + } + + if (memcmp(buf, "hello", sizeof(buf))) { + fail("content mismatch"); + return 1; + } + + pass(); + + return 0; +} diff --git a/test/zdtm/static/memfd01.c b/test/zdtm/static/memfd01.c new file mode 100644 index 000000000..7a7853642 --- /dev/null +++ b/test/zdtm/static/memfd01.c @@ -0,0 +1,114 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "zdtmtst.h" + +const char *test_doc = "memfd with different file pointer"; +const char *test_author = "Nicolas Viennot "; + +#define err(exitcode, msg, ...) ({ pr_perror(msg, ##__VA_ARGS__); exit(exitcode); }) + +static int _memfd_create(const char *name, unsigned int flags) +{ + return syscall(SYS_memfd_create, name, flags); +} + +int main(int argc, char *argv[]) +{ + pid_t pid, pid_child; + int fd, ret, status; + task_waiter_t t; + + test_init(argc, argv); + + task_waiter_init(&t); + + fd = _memfd_create("somename", MFD_CLOEXEC); + if (fd < 0) + err(1, "Can't call memfd_create"); + + pid = getpid(); + + pid_child = fork(); + if (pid_child < 0) + err(1, "Can't fork"); + + if (!pid_child) { + char fdpath[100]; + char buf[1]; + int fl_flags1, fl_flags2, fd_flags1, fd_flags2; + + snprintf(fdpath, sizeof(fdpath), "/proc/%d/fd/%d", pid, fd); + /* + * We pass O_LARGEFILE because in compat mode, our file + * descriptor does not get O_LARGEFILE automatically, but the + * restorer using non-compat open() is forced O_LARGEFILE. + * This creates a flag difference, which we don't want to deal + * with this at the moment. + */ + fd = open(fdpath, O_RDONLY | O_LARGEFILE); + if (fd < 0) + err(1, "Can't open memfd via proc"); + + if ((fl_flags1 = fcntl(fd, F_GETFL)) == -1) + err(1, "Can't get fl flags"); + + if ((fd_flags1 = fcntl(fd, F_GETFD)) == -1) + err(1, "Can't get fd flags"); + + task_waiter_complete(&t, 1); + // checkpoint-restore happens here + task_waiter_wait4(&t, 2); + + if (read(fd, buf, 1) != 1) + err(1, "Can't read"); + + if ((fl_flags2 = fcntl(fd, F_GETFL)) == -1) + err(1, "Can't get fl flags"); + + if (fl_flags1 != fl_flags2) + err(1, "fl flags differs"); + + if ((fd_flags2 = fcntl(fd, F_GETFD)) == -1) + err(1, "Can't get fd flags"); + + if (fd_flags1 != fd_flags2) + err(1, "fd flags differs"); + + if (buf[0] != 'x') + err(1, "Read incorrect"); + + return 0; + } + + task_waiter_wait4(&t, 1); + + test_daemon(); + test_waitsig(); + + if (write(fd, "x", 1) != 1) + err(1, "Can't write"); + + task_waiter_complete(&t, 2); + + ret = wait(&status); + if (ret == -1 || !WIFEXITED(status) || WEXITSTATUS(status)) { + kill(pid, SIGKILL); + fail("child had issue"); + return 1; + } + + pass(); + + return 0; +} diff --git a/test/zdtm/static/memfd02.c b/test/zdtm/static/memfd02.c new file mode 100644 index 000000000..1843e9c9a --- /dev/null +++ b/test/zdtm/static/memfd02.c @@ -0,0 +1,87 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "zdtmtst.h" + +const char *test_doc = "memfd mmap"; +const char *test_author = "Nicolas Viennot "; + +#define err(exitcode, msg, ...) ({ pr_perror(msg, ##__VA_ARGS__); exit(exitcode); }) + +static int _memfd_create(const char *name, unsigned int flags) +{ + return syscall(SYS_memfd_create, name, flags); +} + +int main(int argc, char *argv[]) +{ +#define LEN 6 + int fd; + void *addr_shared, *addr_private; + char buf[LEN]; + + test_init(argc, argv); + + fd = _memfd_create("somename", MFD_CLOEXEC); + if (fd < 0) + err(1, "Can't call memfd_create"); + + if (ftruncate(fd, LEN) < 0) + err(1, "Can't truncate"); + + addr_shared = mmap(NULL, LEN, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if (addr_shared == MAP_FAILED) + err(1, "Can't mmap"); + + write(fd, "write1", LEN); + + addr_private = mmap(NULL, LEN, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0); + if (addr_private == MAP_FAILED) + err(1, "Can't mmap"); + + test_daemon(); + test_waitsig(); + + if (memcmp(addr_shared, "write1", LEN)) { + fail("content mismatch (shared)"); + return 1; + } + + strcpy(addr_shared, "write2"); + + if (pread(fd, buf, LEN, 0) != LEN) { + fail("read problem"); + return 1; + } + + if (memcmp(buf, "write2", LEN)) { + fail("content mismatch (shared)"); + return 1; + } + + if (memcmp(addr_private, "write2", LEN)) { + fail("content mismatch (private)"); + return 1; + } + + strcpy(addr_private, "write3"); + + if (memcmp(addr_shared, "write2", LEN)) { + fail("content mismatch (shared)"); + return 1; + } + + pass(); + + return 0; +} diff --git a/test/zdtm/static/memfd03.c b/test/zdtm/static/memfd03.c new file mode 100644 index 000000000..faedf9383 --- /dev/null +++ b/test/zdtm/static/memfd03.c @@ -0,0 +1,97 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "zdtmtst.h" + +const char *test_doc = "memfd seals"; +const char *test_author = "Nicolas Viennot "; + +#define err(exitcode, msg, ...) ({ pr_perror(msg, ##__VA_ARGS__); exit(exitcode); }) + +static int _memfd_create(const char *name, unsigned int flags) +{ + return syscall(SYS_memfd_create, name, flags); +} + + +#ifndef F_LINUX_SPECIFIC_BASE +# define F_LINUX_SPECIFIC_BASE 1024 +#endif + +#ifndef F_ADD_SEALS + #define F_ADD_SEALS (F_LINUX_SPECIFIC_BASE + 9) +#endif + +#ifndef F_GET_SEALS + #define F_GET_SEALS (F_LINUX_SPECIFIC_BASE + 10) +#endif + + +#ifndef F_SEAL_SEAL +#define F_SEAL_SEAL 0x0001 /* prevent further seals from being set */ +#define F_SEAL_SHRINK 0x0002 /* prevent file from shrinking */ +#define F_SEAL_GROW 0x0004 /* prevent file from growing */ +#define F_SEAL_WRITE 0x0008 /* prevent writes */ +#endif + +int main(int argc, char *argv[]) +{ +#define LEN 5 + int fd, fd2; + void *addr_write, *addr_read; + char fdpath[100]; + + test_init(argc, argv); + + fd = _memfd_create("somename", MFD_ALLOW_SEALING | MFD_CLOEXEC); + if (fd < 0) + err(1, "Can't call memfd_create"); + + if (write(fd, "hello", LEN) != LEN) + err(1, "Can't write"); + + if (fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE) < 0) + err(1, "Can't add seals"); + + test_daemon(); + test_waitsig(); + + snprintf(fdpath, sizeof(fdpath), "/proc/self/fd/%d", fd); + fd2 = open(fdpath, O_RDWR); + if (fd2 < 0) + err(1, "Can't open memfd via proc"); + + if (fcntl(fd, F_GET_SEALS) != F_SEAL_WRITE) { + fail("Seals are different"); + return 1; + } + + addr_write = mmap(NULL, LEN, PROT_WRITE, MAP_SHARED, fd2, 0); + if (addr_write != MAP_FAILED) { + fail("Should not be able to get write access"); + return 1; + } + + addr_read = mmap(NULL, 1, PROT_READ, MAP_PRIVATE, fd2, 0); + if (addr_read == MAP_FAILED) + err(1, "Can't mmap"); + + if (memcmp(addr_read, "hello", LEN)) { + fail("Mapping has bad data"); + return 1; + } + + pass(); + + return 0; +} From ec116449544cd2f062b7523c2eadc6d791baa0ac Mon Sep 17 00:00:00 2001 From: Dmitry Safonov Date: Thu, 23 Jan 2020 16:39:28 +0000 Subject: [PATCH 09/48] criu: Use strlcpy() instead of strncpy() gcc8 in Fedora Rawhide has a new useful warning: > criu/img-remote.c: In function 'push_snapshot_id': > criu/img-remote.c:1099:2: error: 'strncpy' specified bound 4096 equals destination size [-Werror=stringop-truncation] > 1099 | strncpy(rn.snapshot_id, snapshot_id, PATH_MAX); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ From man 3 strncpy: > Warning: If there is no null byte among the first n bytes of src, > the string placed in dest will not be null-terminated. Signed-off-by: Dmitry Safonov --- criu/files-reg.c | 3 ++- criu/files.c | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/criu/files-reg.c b/criu/files-reg.c index b0dad78e6..c2a55aeb3 100644 --- a/criu/files-reg.c +++ b/criu/files-reg.c @@ -33,6 +33,7 @@ #include "namespaces.h" #include "proc_parse.h" #include "pstree.h" +#include "string.h" #include "fault-injection.h" #include "external.h" #include "memfd.h" @@ -457,7 +458,7 @@ static int open_remap_ghost(struct reg_file_info *rfi, gf->remap.rmnt_id = rfi->rfe->mnt_id; if (S_ISDIR(gfe->mode)) - strncpy(gf->remap.rpath, rfi->path, PATH_MAX); + strlcpy(gf->remap.rpath, rfi->path, PATH_MAX); else ghost_path(gf->remap.rpath, PATH_MAX, rfi, rpe); diff --git a/criu/files.c b/criu/files.c index 789b08a4c..f7963bf54 100644 --- a/criu/files.c +++ b/criu/files.c @@ -45,6 +45,7 @@ #include "autofs.h" #include "parasite.h" #include "parasite-syscall.h" +#include "string.h" #include "kerndat.h" #include "fdstore.h" @@ -291,8 +292,7 @@ static int fixup_overlayfs(struct fd_parms *p, struct fd_link *link) char buf[PATH_MAX]; int n; - strncpy(buf, link->name, PATH_MAX); - buf[PATH_MAX - 1] = 0; + strlcpy(buf, link->name, PATH_MAX); n = snprintf(link->name, PATH_MAX, "%s/%s", m->mountpoint, buf + 2); if (n >= PATH_MAX) { pr_err("Not enough space to replace %s\n", buf); From bc49927bbc28b41e4b2759d42dc24f1d66e22df3 Mon Sep 17 00:00:00 2001 From: Dmitry Safonov Date: Thu, 6 Feb 2020 18:01:00 +0000 Subject: [PATCH 10/48] criu: Make use strlcpy() to copy into allocated strings strncpy() with n == strlen(src) won't put NULL-terminator in dst. Signed-off-by: Dmitry Safonov --- criu/cr-restore.c | 1 + 1 file changed, 1 insertion(+) diff --git a/criu/cr-restore.c b/criu/cr-restore.c index f50448cd2..85105a18e 100644 --- a/criu/cr-restore.c +++ b/criu/cr-restore.c @@ -77,6 +77,7 @@ #include "fdstore.h" #include "string.h" #include "memfd.h" +#include "string.h" #include "parasite-syscall.h" #include "files-reg.h" From 3eab205bae1d31f00922d1e717a1cd56c1cb7177 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 6 Feb 2020 21:20:20 -0800 Subject: [PATCH 11/48] python: sort imports 202 Additional newline in a group of imports. I100 Import statements are in the wrong order. Signed-off-by: Andrei Vagin --- test/inhfd/memfd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/inhfd/memfd.py b/test/inhfd/memfd.py index d9ce01e41..b06e35068 100755 --- a/test/inhfd/memfd.py +++ b/test/inhfd/memfd.py @@ -1,5 +1,5 @@ -import os import ctypes +import os libc = ctypes.CDLL(None) From 38793699e7f0e97d14b6b17e83e8bc071c3ce283 Mon Sep 17 00:00:00 2001 From: Pavel Tikhomirov Date: Fri, 21 Feb 2020 12:14:38 +0300 Subject: [PATCH 12/48] test/jenkins: remove empty line at the end of file Signed-off-by: Pavel Tikhomirov --- test/jenkins/criu-fault.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/test/jenkins/criu-fault.sh b/test/jenkins/criu-fault.sh index 4e3790e59..c27dd3738 100755 --- a/test/jenkins/criu-fault.sh +++ b/test/jenkins/criu-fault.sh @@ -27,4 +27,3 @@ fi ./test/zdtm.py run -t zdtm/static/maps04 --fault 131 --keep-going --report report --pre 2:1 || fail ./test/zdtm.py run -t zdtm/transition/maps008 --fault 131 --keep-going --report report --pre 2:1 || fail ./test/zdtm.py run -t zdtm/static/maps01 --fault 132 -f h || fail - From cdd08cdff8692aad9d05dd83e2ab24379cd83393 Mon Sep 17 00:00:00 2001 From: Mike Rapoport Date: Wed, 26 Feb 2020 12:25:37 +0200 Subject: [PATCH 13/48] uffd: use userns_call() to execute ioctl(UFFDIO_API) In the recent kernels the userfaultfd support for FORK events is limited to CAP_SYS_PTRACE. That causes the followong error when the ioctl(UFFDIO_API) is executed from non-privilieged userns: Error (criu/uffd.c:273): uffd: Failed to get uffd API: Operation not permitted Wrapping the call to ioctl(UFFDIO_API) in userns_call() resolves the issue. Fixes: #964 Signed-off-by: Mike Rapoport --- criu/uffd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/criu/uffd.c b/criu/uffd.c index c47b35b1f..99373c04d 100644 --- a/criu/uffd.c +++ b/criu/uffd.c @@ -40,6 +40,7 @@ #include "tls.h" #include "fdstore.h" #include "util.h" +#include "namespaces.h" #undef LOG_PREFIX #define LOG_PREFIX "uffd: " @@ -254,6 +255,13 @@ bool uffd_noncooperative(void) return (kdat.uffd_features & features) == features; } +static int uffd_api_ioctl(void *arg, int fd, pid_t pid) +{ + struct uffdio_api *uffdio_api = arg; + + return ioctl(fd, UFFDIO_API, uffdio_api); +} + int uffd_open(int flags, unsigned long *features) { struct uffdio_api uffdio_api = { 0 }; @@ -269,7 +277,8 @@ int uffd_open(int flags, unsigned long *features) if (features) uffdio_api.features = *features; - if (ioctl(uffd, UFFDIO_API, &uffdio_api)) { + if (userns_call(uffd_api_ioctl, 0, &uffdio_api, sizeof(uffdio_api), + uffd)) { pr_perror("Failed to get uffd API"); goto err; } From 48f3b6516b384f9c4f240aff76f671697198884b Mon Sep 17 00:00:00 2001 From: Radostin Stoyanov Date: Tue, 18 Feb 2020 19:45:08 +0000 Subject: [PATCH 14/48] criu(8): Add documentation for --enable-fs This option was introduced with: https://github.com/checkpoint-restore/criu/commit/e2c38245c613df5e36dcf0253c7652f928e46abf v2: (comment from Pavel Tikhomirov) --enable-fs does not fit with --external dev[]:, see try_resolve_ext_mount, external dev mounts only determined for FSTYPE__UNSUPPORTED. Signed-off-by: Radostin Stoyanov --- Documentation/criu.txt | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Documentation/criu.txt b/Documentation/criu.txt index a6b9f7fae..0ac29103a 100644 --- a/Documentation/criu.txt +++ b/Documentation/criu.txt @@ -90,6 +90,19 @@ The following levels are available: *-L*, *--libdir* 'path':: Path to plugins directory. +*--enable-fs* ['fs'[,'fs'...]]:: + Specify a comma-separated list of filesystem names that should + be auto-detected. The value 'all' enables auto-detection for + all filesystems. ++ +Note: This option is not safe, use at your own risk. +Auto-detecting a filesystem mount assumes that the mountpoint can +be restored with *mount(src, mountpoint, flags, options)*. When used, +*dump* is expected to always succeed if a mountpoint is to be +auto-detected, however *restore* may fail (or do something wrong) +if the assumption for restore logic is incorrect. This option is +not compatable with *--external* *dev*. + *--action-script* 'script':: Add an external action script to be executed at certain stages. The environment variable *CRTOOLS_SCRIPT_ACTION* is available From bb032cc3e218c4aee5394642caa59be3909b259a Mon Sep 17 00:00:00 2001 From: Radostin Stoyanov Date: Tue, 18 Feb 2020 19:53:36 +0000 Subject: [PATCH 15/48] criu(8): Convert tabs to spaces Signed-off-by: Radostin Stoyanov --- Documentation/criu.txt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Documentation/criu.txt b/Documentation/criu.txt index 0ac29103a..ab63e461c 100644 --- a/Documentation/criu.txt +++ b/Documentation/criu.txt @@ -477,7 +477,7 @@ The 'mode' may be one of the following: *soft*::: Restore cgroup properties if only cgroup has been created by *criu*, otherwise do not restore properties. This is the - default if mode is unspecified. + default if mode is unspecified. *full*::: Always restore all cgroups and their properties. @@ -575,17 +575,17 @@ check* always checks Category 1 features unless *--feature* is specified which only checks a specified feature. *Category 1*::: Absolutely required. These are features like support for - */proc/PID/map_files*, *NETLINK_SOCK_DIAG* socket - monitoring, */proc/sys/kernel/ns_last_pid* etc. + */proc/PID/map_files*, *NETLINK_SOCK_DIAG* socket + monitoring, */proc/sys/kernel/ns_last_pid* etc. *Category 2*::: Required only for specific cases. These are features - like AIO remap, */dev/net/tun* and others that are only - required if a process being dumped or restored - is using those. + like AIO remap, */dev/net/tun* and others that are only + required if a process being dumped or restored + is using those. *Category 3*::: Experimental. These are features like *task-diag* that - are used for experimental purposes (mostly - during development). + are used for experimental purposes (mostly + during development). If there are no errors or warnings, *criu* prints "Looks good." and its exit code is 0. From 563c5e5e763949de7b1c48bd04c777db17d768e1 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 4 Feb 2020 09:10:49 +0000 Subject: [PATCH 16/48] seize: prepare for cgroupv2 freezer The cgroupv2 freezer does not return the same strings as v1. Instead of THAWED and FROZEN v2 returns 0 and 1 (strings). This prepares the seize code to use 0 and 1 everywhere and THAWED and FROZEN only for v1 specific code paths. Signed-off-by: Adrian Reber --- criu/seize.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/criu/seize.c b/criu/seize.c index fd314666f..14cd82417 100644 --- a/criu/seize.c +++ b/criu/seize.c @@ -30,7 +30,14 @@ static const char frozen[] = "FROZEN"; static const char freezing[] = "FREEZING"; static const char thawed[] = "THAWED"; -static const char *get_freezer_state(int fd) +enum freezer_state { + FREEZER_ERROR = -1, + THAWED, + FROZEN, + FREEZING +}; + +static enum freezer_state get_freezer_state(int fd) { char state[32]; int ret; @@ -52,15 +59,15 @@ static const char *get_freezer_state(int fd) pr_debug("freezer.state=%s\n", state); if (strcmp(state, frozen) == 0) - return frozen; + return FROZEN; else if (strcmp(state, freezing) == 0) - return freezing; + return FREEZING; else if (strcmp(state, thawed) == 0) - return thawed; + return THAWED; pr_err("Unknown freezer state: %s\n", state); err: - return NULL; + return FREEZER_ERROR; } static bool freezer_thawed; @@ -98,7 +105,7 @@ static int freezer_restore_state(void) static int processes_to_wait; static pid_t *processes_to_wait_pids; -static int seize_cgroup_tree(char *root_path, const char *state) +static int seize_cgroup_tree(char *root_path, enum freezer_state state) { DIR *dir; struct dirent *de; @@ -134,7 +141,7 @@ static int seize_cgroup_tree(char *root_path, const char *state) if (!compel_interrupt_task(pid)) { pr_debug("SEIZE %d: success\n", pid); processes_to_wait++; - } else if (state == frozen) { + } else if (state == FROZEN) { char buf[] = "/proc/XXXXXXXXXX/exe"; struct stat st; @@ -332,7 +339,7 @@ static int freeze_processes(void) { int fd, exit_code = -1; char path[PATH_MAX]; - const char *state = thawed; + enum freezer_state state = THAWED; static const unsigned long step_ms = 100; unsigned long nr_attempts = (opts.timeout * 1000000) / step_ms; @@ -361,11 +368,11 @@ static int freeze_processes(void) return -1; } state = get_freezer_state(fd); - if (!state) { + if (state == FREEZER_ERROR) { close(fd); return -1; } - if (state == thawed) { + if (state == THAWED) { freezer_thawed = true; lseek(fd, 0, SEEK_SET); @@ -384,12 +391,12 @@ static int freeze_processes(void) */ for (; i <= nr_attempts; i++) { state = get_freezer_state(fd); - if (!state) { + if (state == FREEZER_ERROR) { close(fd); return -1; } - if (state == frozen) + if (state == FROZEN) break; if (alarm_timeouted()) goto err; From 9f902e0c6b74ce2d7abf6a632d3abdf6b1370751 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 4 Feb 2020 09:53:51 +0000 Subject: [PATCH 17/48] seize: factor out opening and writing the freezer state More preparations for cgroupv2 freezer. Factor our the freezer state opening and writing to have one location where to handle v1 and v2 differences. Signed-off-by: Adrian Reber --- criu/seize.c | 81 +++++++++++++++++++++++++++++++++++----------------- 1 file changed, 55 insertions(+), 26 deletions(-) diff --git a/criu/seize.c b/criu/seize.c index 14cd82417..b53707e44 100644 --- a/criu/seize.c +++ b/criu/seize.c @@ -20,6 +20,7 @@ #include "seccomp.h" #include "seize.h" #include "stats.h" +#include "string.h" #include "xmalloc.h" #include "util.h" #include @@ -77,13 +78,39 @@ const char *get_real_freezer_state(void) return freezer_thawed ? thawed : frozen; } -static int freezer_restore_state(void) +static int freezer_write_state(int fd, enum freezer_state new_state) { - int fd; - char path[PATH_MAX]; + char state[32]; + int ret; - if (!opts.freeze_cgroup || freezer_thawed) - return 0; + if (new_state == THAWED) { + if (strlcpy(state, thawed, sizeof(state)) >= sizeof(state)) + return -1; + } else if (new_state == FROZEN) { + if (strlcpy(state, frozen, sizeof(state)) >= sizeof(state)) + return -1; + } else { + return -1; + } + + ret = lseek(fd, 0, SEEK_SET); + if (ret < 0) { + pr_perror("Unable to seek freezer FD"); + return -1; + } + if (write(fd, state, sizeof(state)) != sizeof(state)) { + pr_perror("Unable to %s tasks", + (new_state == THAWED) ? "thaw" : "freeze"); + return -1; + } + + return 0; +} + +static int freezer_open(void) +{ + char path[PATH_MAX]; + int fd; snprintf(path, sizeof(path), "%s/freezer.state", opts.freeze_cgroup); fd = open(path, O_RDWR); @@ -92,13 +119,24 @@ static int freezer_restore_state(void) return -1; } - if (write(fd, frozen, sizeof(frozen)) != sizeof(frozen)) { - pr_perror("Unable to freeze tasks"); - close(fd); + return fd; +} + +static int freezer_restore_state(void) +{ + int fd; + int ret; + + if (!opts.freeze_cgroup || freezer_thawed) + return 0; + + fd = freezer_open(); + if (fd < 0) return -1; - } + + ret = freezer_write_state(fd, FROZEN); close(fd); - return 0; + return ret; } /* A number of tasks in a freezer cgroup which are not going to be dumped */ @@ -338,7 +376,6 @@ static int log_unfrozen_stacks(char *root) static int freeze_processes(void) { int fd, exit_code = -1; - char path[PATH_MAX]; enum freezer_state state = THAWED; static const unsigned long step_ms = 100; @@ -361,12 +398,10 @@ static int freeze_processes(void) pr_debug("freezing processes: %lu attempts with %lu ms steps\n", nr_attempts, step_ms); - snprintf(path, sizeof(path), "%s/freezer.state", opts.freeze_cgroup); - fd = open(path, O_RDWR); - if (fd < 0) { - pr_perror("Unable to open %s", path); + fd = freezer_open(); + if (fd < 0) return -1; - } + state = get_freezer_state(fd); if (state == FREEZER_ERROR) { close(fd); @@ -375,9 +410,7 @@ static int freeze_processes(void) if (state == THAWED) { freezer_thawed = true; - lseek(fd, 0, SEEK_SET); - if (write(fd, frozen, sizeof(frozen)) != sizeof(frozen)) { - pr_perror("Unable to freeze tasks"); + if (freezer_write_state(fd, FROZEN)) { close(fd); return -1; } @@ -427,13 +460,9 @@ static int freeze_processes(void) } err: - if (exit_code == 0 || freezer_thawed) { - lseek(fd, 0, SEEK_SET); - if (write(fd, thawed, sizeof(thawed)) != sizeof(thawed)) { - pr_perror("Unable to thaw tasks"); - exit_code = -1; - } - } + if (exit_code == 0 || freezer_thawed) + exit_code = freezer_write_state(fd, THAWED); + if (close(fd)) { pr_perror("Unable to thaw tasks"); return -1; From 10416bcbcb3c2ab4732971083f99d9390f8d168b Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Tue, 4 Feb 2020 13:38:42 +0000 Subject: [PATCH 18/48] seize: support cgroup v2 freezer This adds support to checkpoint processes using the cgroup v2 freezer. Signed-off-by: Adrian Reber --- criu/seize.c | 150 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 133 insertions(+), 17 deletions(-) diff --git a/criu/seize.c b/criu/seize.c index b53707e44..0ba2d9b1d 100644 --- a/criu/seize.c +++ b/criu/seize.c @@ -38,7 +38,10 @@ enum freezer_state { FREEZING }; -static enum freezer_state get_freezer_state(int fd) +/* Track if we are running on cgroup v2 system. */ +static bool cgroup_v2 = false; + +static enum freezer_state get_freezer_v1_state(int fd) { char state[32]; int ret; @@ -71,6 +74,70 @@ err: return FREEZER_ERROR; } +static enum freezer_state get_freezer_v2_state(int fd) +{ + int exit_code = FREEZER_ERROR; + char path[PATH_MAX]; + FILE *event; + char state; + int ret; + + /* + * cgroupv2 freezer uses cgroup.freeze to control the state. The file + * can return 0 or 1. 1 means the cgroup is frozen; 0 means it is not + * frozen. Writing 1 to an unfrozen cgroup can freeze it. Freezing can + * take some time and if the cgroup has finished freezing can be + * seen in cgroup.events: frozen 0|1. + */ + + ret = lseek(fd, 0, SEEK_SET); + if (ret < 0) { + pr_perror("Unable to seek freezer FD"); + goto out; + } + ret = read(fd, &state, 1); + if (ret <= 0) { + pr_perror("Unable to read from freezer FD"); + goto out; + } + pr_debug("cgroup.freeze=%c\n", state); + if (state == '0') { + exit_code = THAWED; + goto out; + } + + snprintf(path, sizeof(path), "%s/cgroup.events", opts.freeze_cgroup); + event = fopen(path, "r"); + if (event == NULL) { + pr_perror("Unable to open %s", path); + goto out; + } + while (fgets(path, sizeof(path), event)) { + if (strncmp(path, "frozen", 6) != 0) { + continue; + } else if (strncmp(path, "frozen 0", 8) == 0) { + exit_code = FREEZING; + goto close; + } else if (strncmp(path, "frozen 1", 8) == 0) { + exit_code = FROZEN; + goto close; + } + } + + pr_err("Unknown freezer state: %c\n", state); +close: + fclose(event); +out: + return exit_code; +} + +static enum freezer_state get_freezer_state(int fd) +{ + if (cgroup_v2) + return get_freezer_v2_state(fd); + return get_freezer_v1_state(fd); +} + static bool freezer_thawed; const char *get_real_freezer_state(void) @@ -80,15 +147,23 @@ const char *get_real_freezer_state(void) static int freezer_write_state(int fd, enum freezer_state new_state) { - char state[32]; + char state[32] = {0}; int ret; if (new_state == THAWED) { - if (strlcpy(state, thawed, sizeof(state)) >= sizeof(state)) - return -1; + if (cgroup_v2) + state[0] = '0'; + else + if (strlcpy(state, thawed, sizeof(state)) >= + sizeof(state)) + return -1; } else if (new_state == FROZEN) { - if (strlcpy(state, frozen, sizeof(state)) >= sizeof(state)) - return -1; + if (cgroup_v2) + state[0] = '1'; + else + if (strlcpy(state, frozen, sizeof(state)) >= + sizeof(state)) + return -1; } else { return -1; } @@ -109,10 +184,13 @@ static int freezer_write_state(int fd, enum freezer_state new_state) static int freezer_open(void) { + const char freezer_v1[] = "freezer.state"; + const char freezer_v2[] = "cgroup.freeze"; char path[PATH_MAX]; int fd; - snprintf(path, sizeof(path), "%s/freezer.state", opts.freeze_cgroup); + snprintf(path, sizeof(path), "%s/%s", opts.freeze_cgroup, + cgroup_v2 ? freezer_v2 : freezer_v1); fd = open(path, O_RDWR); if (fd < 0) { pr_perror("Unable to open %s", path); @@ -139,6 +217,22 @@ static int freezer_restore_state(void) return ret; } +static FILE *freezer_open_thread_list(char *root_path) +{ + char path[PATH_MAX]; + FILE *f; + + snprintf(path, sizeof(path), "%s/%s", root_path, + cgroup_v2 ? "cgroup.threads" : "tasks"); + f = fopen(path, "r"); + if (f == NULL) { + pr_perror("Unable to open %s", path); + return NULL; + } + + return f; +} + /* A number of tasks in a freezer cgroup which are not going to be dumped */ static int processes_to_wait; static pid_t *processes_to_wait_pids; @@ -154,12 +248,10 @@ static int seize_cgroup_tree(char *root_path, enum freezer_state state) * New tasks can appear while a freezer state isn't * frozen, so we need to catch all new tasks. */ - snprintf(path, sizeof(path), "%s/tasks", root_path); - f = fopen(path, "r"); - if (f == NULL) { - pr_perror("Unable to open %s", path); + f = freezer_open_thread_list(root_path); + if (f == NULL) return -1; - } + while (fgets(path, sizeof(path), f)) { pid_t pid; int ret; @@ -306,12 +398,10 @@ static int log_unfrozen_stacks(char *root) char path[PATH_MAX]; FILE *f; - snprintf(path, sizeof(path), "%s/tasks", root); - f = fopen(path, "r"); - if (f == NULL) { - pr_perror("Unable to open %s", path); + f = freezer_open_thread_list(root); + if (f == NULL) return -1; - } + while (fgets(path, sizeof(path), f)) { pid_t pid; int ret, stack; @@ -820,6 +910,27 @@ err_close: return -1; } +static int cgroup_version(void) +{ + char path[PATH_MAX]; + + snprintf(path, sizeof(path), "%s/freezer.state", opts.freeze_cgroup); + if (access(path, F_OK) == 0) { + cgroup_v2 = false; + return 0; + } + + snprintf(path, sizeof(path), "%s/cgroup.freeze", opts.freeze_cgroup); + if (access(path, F_OK) == 0) { + cgroup_v2 = true; + return 0; + } + + pr_err("Neither a cgroupv1 (freezer.state) or cgroupv2 (cgroup.freeze) control file found.\n"); + + return -1; +} + int collect_pstree(void) { pid_t pid = root_item->pid->real; @@ -835,6 +946,11 @@ int collect_pstree(void) */ alarm(opts.timeout); + if (opts.freeze_cgroup && cgroup_version()) + goto err; + + pr_debug("Detected cgroup V%d freezer\n", cgroup_v2 ? 2 : 1); + if (opts.freeze_cgroup && freeze_processes()) goto err; From 4129d3262ad2d2ac6875c2c86d565528969d8e72 Mon Sep 17 00:00:00 2001 From: Adrian Reber Date: Wed, 5 Feb 2020 09:39:32 +0000 Subject: [PATCH 19/48] cgroup2: add minimal cgroup2 support The runc test cases are (sometimes) mounting a cgroup inside of the container. For these tests to succeed, let CRIU know that cgroup2 exists and how to restore such a mount. This does not fix any specific cgroup2 settings, it just enables CRIU to mount cgroup2 in the restored container. Signed-off-by: Adrian Reber --- criu/filesystems.c | 5 +++++ images/mnt.proto | 2 ++ 2 files changed, 7 insertions(+) diff --git a/criu/filesystems.c b/criu/filesystems.c index 1e4550b37..d76b18291 100644 --- a/criu/filesystems.c +++ b/criu/filesystems.c @@ -747,6 +747,11 @@ static struct fstype fstypes[] = { .code = FSTYPE__CGROUP, .parse = cgroup_parse, .sb_equal = cgroup_sb_equal, + }, { + .name = "cgroup2", + .code = FSTYPE__CGROUP2, + .parse = cgroup_parse, + .sb_equal = cgroup_sb_equal, }, { .name = "aufs", .code = FSTYPE__AUFS, diff --git a/images/mnt.proto b/images/mnt.proto index 4160acbf6..8983395ae 100644 --- a/images/mnt.proto +++ b/images/mnt.proto @@ -28,6 +28,8 @@ enum fstype { // RPC_PIPEFS = 20; // NFS = 21; // NFS4 = 22; + + CGROUP2 = 23; }; message mnt_entry { From ffe0896ed01790e62cd617cf01bc6a4076fa4e87 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sun, 1 Mar 2020 04:26:12 +0300 Subject: [PATCH 20/48] fs: use __open_proc instead of open("/proc/...", ... ) Processes can run in a mount namespace without /proc. Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin --- criu/files-reg.c | 4 +--- criu/memfd.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/criu/files-reg.c b/criu/files-reg.c index c2a55aeb3..0d0076666 100644 --- a/criu/files-reg.c +++ b/criu/files-reg.c @@ -778,14 +778,12 @@ static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_de if (S_ISREG(st->st_mode)) { int fd, ret; - char lpath[PSFDS]; /* * Reopen file locally since it may have no read * permissions when drained */ - sprintf(lpath, "/proc/self/fd/%d", _fd); - fd = open(lpath, O_RDONLY); + fd = open_proc(PROC_SELF, "fd/%d", _fd); if (fd < 0) { pr_perror("Can't open ghost original file"); goto err_out; diff --git a/criu/memfd.c b/criu/memfd.c index d17c10fb7..30ccdf22c 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -299,7 +299,6 @@ static int memfd_open_inode(struct memfd_inode *inode) int memfd_open(struct file_desc *d, u32 *fdflags) { - char lpath[PSFDS]; struct memfd_info *mfi; MemfdFileEntry *mfe; int fd, _fd; @@ -318,14 +317,13 @@ int memfd_open(struct file_desc *d, u32 *fdflags) goto err; /* Reopen the fd with original permissions */ - sprintf(lpath, "/proc/self/fd/%d", fd); flags = fdflags ? *fdflags : mfe->flags; /* * Ideally we should call compat version open() to not force the * O_LARGEFILE file flag with regular open(). It doesn't seem that * important though. */ - _fd = open(lpath, flags); + _fd = __open_proc(getpid(), 0, flags, "fd/%d", fd); if (_fd < 0) { pr_perror("Can't reopen memfd id=%d", mfe->id); goto err; From fce196d88df8363666922ac2fa2d6e23bb774289 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sat, 29 Feb 2020 09:51:33 +0300 Subject: [PATCH 21/48] memfd: don't corrupt a state of the dumped fd Right now, criu uses a dumped fd to dump content of a memfd "file". Here are two reasons why we should not do this: * a state of a dumped fd doesn't have to be changed, but now criu calls lseek on it. This can be workarounded by using pread. * a dumped descriptor can be write-only. Reported-by: Mr Jenkins Cc: Nicolas Viennot Signed-off-by: Andrei Vagin --- criu/memfd.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/criu/memfd.c b/criu/memfd.c index 30ccdf22c..983e01b38 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -119,6 +119,7 @@ out: static struct memfd_inode *dump_unique_memfd_inode(int lfd, const char *name, const struct stat *st) { struct memfd_inode *inode; + int fd; list_for_each_entry(inode, &memfd_inodes, list) if ((inode->dev == st->st_dev) && (inode->ino == st->st_ino)) @@ -132,11 +133,19 @@ static struct memfd_inode *dump_unique_memfd_inode(int lfd, const char *name, co inode->ino = st->st_ino; inode->id = memfd_inode_ids++; - if (dump_memfd_inode(lfd, inode, name, st)) { + fd = open_proc(PROC_SELF, "fd/%d", lfd); + if (fd < 0) { xfree(inode); return NULL; } + if (dump_memfd_inode(fd, inode, name, st)) { + close(fd); + xfree(inode); + return NULL; + } + close(fd); + list_add_tail(&inode->list, &memfd_inodes); return inode; From 58fd63042c925c6422f5453b9a13147ebd2a6769 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sun, 1 Mar 2020 01:04:20 +0300 Subject: [PATCH 22/48] zdtm/inhfd: force python to read new data from a file python 2.7 doesn't call the read system call if it's read file to the end once. The next seek allows to workaround this problem. inhfd/memfd.py hangs due to this issue. Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin --- test/zdtm.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/zdtm.py b/test/zdtm.py index 4110b5142..0bd7b84cc 100755 --- a/test/zdtm.py +++ b/test/zdtm.py @@ -697,7 +697,10 @@ class inhfd_test: # regular files, so we loop. data = b'' while not data: - data = peer_file.read(16) + # In python 2.7, peer_file.read() doesn't call the read + # system call if it's read file to the end once. The + # next seek allows to workaround this problem. + data = os.read(peer_file.fileno(), 16) time.sleep(0.1) except Exception as e: print("Unable to read a peer file: %s" % e) From f167d1f4e9eb24dfbda077746f25cfdf8a2b59b9 Mon Sep 17 00:00:00 2001 From: Cyrill Gorcunov Date: Fri, 7 Feb 2020 15:59:45 +0300 Subject: [PATCH 23/48] fown: Don't fail on dumping files opened with O_PATH O_PATH opened files are special: they have empty file operations in kernel space, so there not that much we can do with them, even setting position is not allowed. Same applies to a signal number for owner settings. Signed-off-by: Cyrill Gorcunov Co-developed-by: Alexander Mikhalitsyn Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) --- criu/files-reg.c | 16 +++++++--- criu/files.c | 5 ++- criu/pie/parasite.c | 75 +++++++++++++++++++++++++++++---------------- 3 files changed, 64 insertions(+), 32 deletions(-) diff --git a/criu/files-reg.c b/criu/files-reg.c index 0d0076666..d1d1ee5af 100644 --- a/criu/files-reg.c +++ b/criu/files-reg.c @@ -1776,11 +1776,17 @@ static int do_open_reg(int ns_root_fd, struct reg_file_info *rfi, void *arg) if (fd < 0) return fd; - if ((rfi->rfe->pos != -1ULL) && - lseek(fd, rfi->rfe->pos, SEEK_SET) < 0) { - pr_perror("Can't restore file pos"); - close(fd); - return -1; + /* + * O_PATH opened files carry empty fops in kernel, + * just ignore positioning at all. + */ + if (!(rfi->rfe->flags & O_PATH)) { + if (rfi->rfe->pos != -1ULL && + lseek(fd, rfi->rfe->pos, SEEK_SET) < 0) { + pr_perror("Can't restore file pos"); + close(fd); + return -1; + } } return fd; diff --git a/criu/files.c b/criu/files.c index f7963bf54..3f1d77931 100644 --- a/criu/files.c +++ b/criu/files.c @@ -399,7 +399,10 @@ static int fill_fd_params(struct pid *owner_pid, int fd, int lfd, pr_info("%d fdinfo %d: pos: %#16"PRIx64" flags: %16o/%#x\n", owner_pid->real, fd, p->pos, p->flags, (int)p->fd_flags); - ret = fcntl(lfd, F_GETSIG, 0); + if (p->flags & O_PATH) + ret = 0; + else + ret = fcntl(lfd, F_GETSIG, 0); if (ret < 0) { pr_perror("Can't get owner signum on %d", lfd); return -1; diff --git a/criu/pie/parasite.c b/criu/pie/parasite.c index 387a976da..64b5bbb3e 100644 --- a/criu/pie/parasite.c +++ b/criu/pie/parasite.c @@ -317,15 +317,60 @@ grps_err: return -1; } +static int fill_fds_fown(int fd, struct fd_opts *p) +{ + int flags, ret; + struct f_owner_ex owner_ex; + uint32_t v[2]; + + /* + * For O_PATH opened files there is no owner at all. + */ + flags = sys_fcntl(fd, F_GETFL, 0); + if (flags < 0) { + pr_err("fcntl(%d, F_GETFL) -> %d\n", fd, flags); + return -1; + } + if (flags & O_PATH) { + p->fown.pid = 0; + return 0; + } + + ret = sys_fcntl(fd, F_GETOWN_EX, (long)&owner_ex); + if (ret) { + pr_err("fcntl(%d, F_GETOWN_EX) -> %d\n", fd, ret); + return -1; + } + + /* + * Simple case -- nothing is changed. + */ + if (owner_ex.pid == 0) { + p->fown.pid = 0; + return 0; + } + + ret = sys_fcntl(fd, F_GETOWNER_UIDS, (long)&v); + if (ret) { + pr_err("fcntl(%d, F_GETOWNER_UIDS) -> %d\n", fd, ret); + return -1; + } + + p->fown.uid = v[0]; + p->fown.euid = v[1]; + p->fown.pid_type = owner_ex.type; + p->fown.pid = owner_ex.pid; + + return 0; +} + static int fill_fds_opts(struct parasite_drain_fd *fds, struct fd_opts *opts) { int i; for (i = 0; i < fds->nr_fds; i++) { - int flags, fd = fds->fds[i], ret; + int flags, fd = fds->fds[i]; struct fd_opts *p = opts + i; - struct f_owner_ex owner_ex; - uint32_t v[2]; flags = sys_fcntl(fd, F_GETFD, 0); if (flags < 0) { @@ -335,30 +380,8 @@ static int fill_fds_opts(struct parasite_drain_fd *fds, struct fd_opts *opts) p->flags = (char)flags; - ret = sys_fcntl(fd, F_GETOWN_EX, (long)&owner_ex); - if (ret) { - pr_err("fcntl(%d, F_GETOWN_EX) -> %d\n", fd, ret); + if (fill_fds_fown(fd, p)) return -1; - } - - /* - * Simple case -- nothing is changed. - */ - if (owner_ex.pid == 0) { - p->fown.pid = 0; - continue; - } - - ret = sys_fcntl(fd, F_GETOWNER_UIDS, (long)&v); - if (ret) { - pr_err("fcntl(%d, F_GETOWNER_UIDS) -> %d\n", fd, ret); - return -1; - } - - p->fown.uid = v[0]; - p->fown.euid = v[1]; - p->fown.pid_type = owner_ex.type; - p->fown.pid = owner_ex.pid; } return 0; From 8b9c1f4c5bebd501f544cfe81534e4386f85246f Mon Sep 17 00:00:00 2001 From: Pavel Tikhomirov Date: Fri, 7 Feb 2020 15:59:55 +0300 Subject: [PATCH 24/48] zdtm: add a test for files opened with O_PATH On these test without the patch ("fown: Don't fail on dumping files opened wit O_PATH") we trigger these errors: Error (criu/pie/parasite.c:340): fcntl(4, F_GETOWN_EX) -> -9 Error (criu/files.c:403): Can't get owner signum on 18: Bad file descriptor Error (criu/files-reg.c:1887): Can't restore file pos: Bad file descriptor Signed-off-by: Pavel Tikhomirov Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) --- test/zdtm/static/Makefile | 1 + test/zdtm/static/opath_file.c | 95 +++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 test/zdtm/static/opath_file.c diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile index 5afd18cd6..035b8fa9c 100644 --- a/test/zdtm/static/Makefile +++ b/test/zdtm/static/Makefile @@ -363,6 +363,7 @@ TST_DIR = \ private_bind_propagation \ ghost_on_rofs \ overmounted_file \ + opath_file \ TST_DIR_FILE = \ chroot \ diff --git a/test/zdtm/static/opath_file.c b/test/zdtm/static/opath_file.c new file mode 100644 index 000000000..602a5af27 --- /dev/null +++ b/test/zdtm/static/opath_file.c @@ -0,0 +1,95 @@ +#include +#include +#include +#include + +#include "zdtmtst.h" + +#define TEST_FILE "test_file" +#define BUF_SIZE 4096 +#define fdinfo_field(str, field) !strncmp(str, field":", sizeof(field)) +#define pr_debug(format, arg...) test_msg("DBG: %s:%d: " format, __FILE__, __LINE__, ## arg) + +const char *test_doc = "Check open file with O_PATH preserved"; +const char *test_author = "Pavel Tikhomirov "; + +char *dirname; +TEST_OPTION(dirname, string, "directory name", 1); + +struct fdinfo { + int flags; +}; + +static int parse_self_fdinfo(int fd, struct fdinfo *fi) +{ + char path[PATH_MAX], line[BUF_SIZE]; + FILE *file; + int ret = -1; + unsigned long long val; + + snprintf(path, sizeof(path), "/proc/self/fdinfo/%d", fd); + file = fopen(path, "r"); + if (!file) { + pr_perror("fopen"); + return -1; + } + + while (fgets(line, sizeof(line), file)) { + if (fdinfo_field(line, "flags")) { + if (sscanf(line, "%*s %llu", &val) != 1) { + pr_err("failed to read flags: %s", line); + goto fail; + } + pr_debug("Open flags = %llu\n", val); + fi->flags = val; + ret = 0; + break; + } + } +fail: + fclose(file); + return ret; +} + +int main(int argc, char **argv) +{ + char test_file[PATH_MAX]; + struct fdinfo fi; + int fd; + + test_init(argc, argv); + + if (mkdir(dirname, 0700)) { + pr_perror("can't make directory %s", dirname); + exit(1); + } + + snprintf(test_file, sizeof(test_file), "%s/%s", dirname, TEST_FILE); + fd = creat(test_file, 0644); + if (fd == -1) { + pr_perror("cat't create %s", test_file); + return 1; + } + close(fd); + + fd = open(test_file, O_PATH); + if (fd == -1) { + pr_perror("cat't open file %s with O_PATH", test_file); + return 1; + } + + test_daemon(); + test_waitsig(); + + if (parse_self_fdinfo(fd, &fi)) + return 1; + + if (!(fi.flags & O_PATH)) { + fail("File lost O_PATH open flag"); + return 1; + } + + close(fd); + pass(); + return 0; +} From 1936608ce42283b6c5aa007c883092bb2776af4b Mon Sep 17 00:00:00 2001 From: Alexander Mikhalitsyn Date: Fri, 7 Feb 2020 16:00:01 +0300 Subject: [PATCH 25/48] files: allow dumping opened symlinks To really open symlink file and not the regular file below it, one needs to do open with O_PATH|O_NOFOLLOW flags. Looks like systemd started to open /etc/localtime symlink this way sometimes, and before that nobody actually used this and thus we never supported this in CRIU. Error (criu/files-ext.c:96): Can't dump file 11 of that type [120777] (unknown /etc/localtime) Looks like it is quiet easy to support, as c/r of symlink file is almost the same as c/r of regular one. We need to only make fstatat not following links in check_path_remap. Also we need to take into account support of ghost symlinks. Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) Co-developed-by: Pavel Tikhomirov Signed-off-by: Pavel Tikhomirov --- criu/files-reg.c | 88 +++++++++++++++++++++++++++++++++++++---- criu/files.c | 3 +- images/ghost-file.proto | 2 + 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/criu/files-reg.c b/criu/files-reg.c index d1d1ee5af..b53e9b080 100644 --- a/criu/files-reg.c +++ b/criu/files-reg.c @@ -282,19 +282,53 @@ static int mkreg_ghost(char *path, GhostFileEntry *gfe, struct cr_img *img) return ret; } +static int mklnk_ghost(char *path, GhostFileEntry *gfe) +{ + if (!gfe->symlnk_target) { + pr_err("Ghost symlink target is NULL for %s. Image from old CRIU?\n", path); + return -1; + } + + if (symlink(gfe->symlnk_target, path) < 0) { + /* + * ENOENT case is OK + * Take a look closer on create_ghost() function + */ + if (errno != ENOENT) + pr_perror("symlink(%s, %s) failed", gfe->symlnk_target, path); + return -1; + } + + return 0; +} + static int ghost_apply_metadata(const char *path, GhostFileEntry *gfe) { struct timeval tv[2]; int ret = -1; - if (chown(path, gfe->uid, gfe->gid) < 0) { - pr_perror("Can't reset user/group on ghost %s", path); - goto err; - } + if (S_ISLNK(gfe->mode)) { + if (lchown(path, gfe->uid, gfe->gid) < 0) { + pr_perror("Can't reset user/group on ghost %s", path); + goto err; + } - if (chmod(path, gfe->mode)) { - pr_perror("Can't set perms %o on ghost %s", gfe->mode, path); - goto err; + /* + * We have no lchmod() function, and fchmod() will fail on + * O_PATH | O_NOFOLLOW fd. Yes, we have fchmodat() + * function and flag AT_SYMLINK_NOFOLLOW described in + * man 2 fchmodat, but it is not currently implemented. %) + */ + } else { + if (chown(path, gfe->uid, gfe->gid) < 0) { + pr_perror("Can't reset user/group on ghost %s", path); + goto err; + } + + if (chmod(path, gfe->mode)) { + pr_perror("Can't set perms %o on ghost %s", gfe->mode, path); + goto err; + } } if (gfe->atim) { @@ -353,6 +387,9 @@ again: } else if (S_ISDIR(gfe->mode)) { if ((ret = mkdirpat(AT_FDCWD, path, gfe->mode)) < 0) msg = "Can't make ghost dir"; + } else if (S_ISLNK(gfe->mode)) { + if ((ret = mklnk_ghost(path, gfe)) < 0) + msg = "Can't create ghost symlink"; } else { if ((ret = mkreg_ghost(path, gfe, img)) < 0) msg = "Can't create ghost regfile"; @@ -740,6 +777,7 @@ static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_de int exit_code = -1; GhostFileEntry gfe = GHOST_FILE_ENTRY__INIT; Timeval atim = TIMEVAL__INIT, mtim = TIMEVAL__INIT; + char pathbuf[PATH_MAX]; pr_info("Dumping ghost file contents (id %#x)\n", id); @@ -773,6 +811,36 @@ static int dump_ghost_file(int _fd, u32 id, const struct stat *st, dev_t phys_de gfe.size = st->st_size; } + /* + * We set gfe.symlnk_target only if we need to dump + * symlink content, otherwise we leave it NULL. + * It will be taken into account on restore in mklnk_ghost function. + */ + if (S_ISLNK(st->st_mode)) { + ssize_t ret; + + /* + * We assume that _fd opened with O_PATH | O_NOFOLLOW + * flags because S_ISLNK(st->st_mode). With current kernel version, + * it's looks like correct assumption in any case. + */ + ret = readlinkat(_fd, "", pathbuf, sizeof(pathbuf) - 1); + if (ret < 0) { + pr_perror("Can't readlinkat"); + goto err_out; + } + + pathbuf[ret] = 0; + + if (ret != st->st_size) { + pr_err("Buffer for readlinkat is too small: ret %zd, st_size %"PRId64", buf %u %s\n", + ret, st->st_size, PATH_MAX, pathbuf); + goto err_out; + } + + gfe.symlnk_target = pathbuf; + } + if (pb_write_one(img, &gfe, PB_GHOST_FILE)) goto err_out; @@ -1116,6 +1184,7 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms, int ret, mntns_root; struct stat pst; const struct stat *ost = &parms->stat; + int flags = 0; if (parms->fs_type == PROC_SUPER_MAGIC) { /* The file points to /proc/pid/ where pid is a dead @@ -1212,7 +1281,10 @@ static int check_path_remap(struct fd_link *link, const struct fd_parms *parms, if (mntns_root < 0) return -1; - ret = fstatat(mntns_root, rpath, &pst, 0); + if (S_ISLNK(parms->stat.st_mode)) + flags = AT_SYMLINK_NOFOLLOW; + + ret = fstatat(mntns_root, rpath, &pst, flags); if (ret < 0) { /* * Linked file, but path is not accessible (unless any diff --git a/criu/files.c b/criu/files.c index 3f1d77931..f6ba39a30 100644 --- a/criu/files.c +++ b/criu/files.c @@ -545,7 +545,8 @@ static int dump_one_file(struct pid *pid, int fd, int lfd, struct fd_opts *opts, return do_dump_gen_file(&p, lfd, ops, e); } - if (S_ISREG(p.stat.st_mode) || S_ISDIR(p.stat.st_mode)) { + if (S_ISREG(p.stat.st_mode) || S_ISDIR(p.stat.st_mode) || + S_ISLNK(p.stat.st_mode)) { if (fill_fdlink(lfd, &p, &link)) return -1; diff --git a/images/ghost-file.proto b/images/ghost-file.proto index eda466451..0576089fd 100644 --- a/images/ghost-file.proto +++ b/images/ghost-file.proto @@ -15,6 +15,8 @@ message ghost_file_entry { optional timeval mtim = 8; optional bool chunks = 9; optional uint64 size = 10; + /* this field makes sense only when S_ISLNK(mode) */ + optional string symlnk_target = 11; } message ghost_chunk_entry { From 73e0ed3b8ae2a87d4232dd4a39ce3dab4edb1f24 Mon Sep 17 00:00:00 2001 From: Pavel Tikhomirov Date: Fri, 7 Feb 2020 16:00:05 +0300 Subject: [PATCH 26/48] zdtm: add a test on open symlink migration Signed-off-by: Pavel Tikhomirov Co-Developed-by: Vitaly Ostrosablin Signed-off-by: Vitaly Ostrosablin Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) --- test/zdtm/static/Makefile | 3 + test/zdtm/static/opath_file.c | 2 +- test/zdtm/static/symlink.c | 102 ++++++++++++++++++++++++++++++++++ test/zdtm/static/symlink01.c | 1 + 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 test/zdtm/static/symlink.c create mode 120000 test/zdtm/static/symlink01.c diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile index 035b8fa9c..ee69612c7 100644 --- a/test/zdtm/static/Makefile +++ b/test/zdtm/static/Makefile @@ -364,6 +364,8 @@ TST_DIR = \ ghost_on_rofs \ overmounted_file \ opath_file \ + symlink \ + symlink01 \ TST_DIR_FILE = \ chroot \ @@ -539,6 +541,7 @@ clone_fs: LDLIBS += -pthread # we have to explicitly specify both .o and .d for this case: netns_sub_veth.o netns_sub_veth.d: CPPFLAGS += $(call pkg-cflags, libnl-3.0) netns_sub_veth: LDLIBS += $(call pkg-libs, libnl-route-3.0 libnl-3.0) +symlink01: CFLAGS += -DZDTM_UNLINK_SYMLINK socket-tcp-fin-wait1: CFLAGS += -D ZDTM_TCP_FIN_WAIT1 socket-tcp-fin-wait2: CFLAGS += -D ZDTM_TCP_FIN_WAIT2 diff --git a/test/zdtm/static/opath_file.c b/test/zdtm/static/opath_file.c index 602a5af27..943f4eddb 100644 --- a/test/zdtm/static/opath_file.c +++ b/test/zdtm/static/opath_file.c @@ -36,7 +36,7 @@ static int parse_self_fdinfo(int fd, struct fdinfo *fi) while (fgets(line, sizeof(line), file)) { if (fdinfo_field(line, "flags")) { - if (sscanf(line, "%*s %llu", &val) != 1) { + if (sscanf(line, "%*s %llo", &val) != 1) { pr_err("failed to read flags: %s", line); goto fail; } diff --git a/test/zdtm/static/symlink.c b/test/zdtm/static/symlink.c new file mode 100644 index 000000000..074c80052 --- /dev/null +++ b/test/zdtm/static/symlink.c @@ -0,0 +1,102 @@ +#include +#include +#include +#include +#include +#include + +#include "zdtmtst.h" + +#define TEST_FILE "test_file" +#define TEST_SYMLINK "test_symlink" + +const char *test_doc = "Check open symlink preserved"; +const char *test_author = "Pavel Tikhomirov "; + +char *dirname; +TEST_OPTION(dirname, string, "directory name", 1); + +int main(int argc, char **argv) +{ + char test_symlink[PATH_MAX]; + char test_file[PATH_MAX]; + char pathbuf[PATH_MAX]; + struct stat stb, sta; + int ret, fd; + + test_init(argc, argv); + + if (mkdir(dirname, 0700)) { + pr_perror("can't make directory %s", dirname); + exit(1); + } + + snprintf(test_file, sizeof(test_file), "%s/%s", dirname, TEST_FILE); + ret = creat(test_file, 0644); + if (ret == -1) { + pr_perror("cat't create %s", test_file); + return 1; + } + close(ret); + + snprintf(test_symlink, sizeof(test_symlink), "%s/%s", dirname, TEST_SYMLINK); + ret = symlink(test_file, test_symlink); + if (ret == -1) { + pr_perror("cat't symlink to %s", test_symlink); + return 1; + } + + fd = open(test_symlink, O_PATH | O_NOFOLLOW); + if (fd == -1) { + pr_perror("cat't open symlink %s", test_symlink); + return 1; + } + + ret = fstat(fd, &sta); + if (ret == -1) { + pr_perror("cat't fstat %s", test_symlink); + return 1; + } + + if (!S_ISLNK(sta.st_mode)) { + pr_perror("file is not symlink %s", test_symlink); + return 1; + } + +#ifdef ZDTM_UNLINK_SYMLINK + if (unlink(test_symlink)) { + pr_perror("can't unlink symlink %s", test_symlink); + return 1; + } +#endif + + test_daemon(); + test_waitsig(); + + ret = fstat(fd, &stb); + if (ret == -1) { + fail("cat't fstat %s", test_symlink); + return 1; + } + + if (!S_ISLNK(stb.st_mode)) { + fail("file is not symlink %s", test_symlink); + return 1; + } + + ret = readlinkat(fd, "", pathbuf, sizeof(pathbuf) - 1); + if (ret < 0) { + fail("Can't readlinkat"); + return 1; + } + pathbuf[ret] = 0; + + if (strcmp(test_file, pathbuf)) { + fail("symlink points to %s but %s expected", pathbuf, test_file); + return 1; + } + + close(fd); + pass(); + return 0; +} diff --git a/test/zdtm/static/symlink01.c b/test/zdtm/static/symlink01.c new file mode 120000 index 000000000..e2d071ea4 --- /dev/null +++ b/test/zdtm/static/symlink01.c @@ -0,0 +1 @@ +symlink.c \ No newline at end of file From 065ff6f4151805fe50cc881ef506b6ae6407ec57 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 5 Mar 2020 08:30:23 +0300 Subject: [PATCH 27/48] zdtm/fifo_loop: don't try to write more than pipe size ... otherwise write() can block. Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin --- test/zdtm/transition/fifo_loop.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/zdtm/transition/fifo_loop.c b/test/zdtm/transition/fifo_loop.c index 2e28320ba..b028c2fd5 100644 --- a/test/zdtm/transition/fifo_loop.c +++ b/test/zdtm/transition/fifo_loop.c @@ -39,6 +39,7 @@ int main(int argc, char **argv) int i; uint8_t buf[0x100000]; char *file_path; + int pipe_size; test_init(argc, argv); @@ -104,6 +105,13 @@ int main(int argc, char **argv) exit(1); } + pipe_size = fcntl(writefd, F_SETPIPE_SZ, sizeof(buf)); + if (pipe_size != sizeof(buf)) { + pr_perror("fcntl(writefd, F_GETPIPE_SZ) -> %d", pipe_size); + kill(0, SIGKILL); + exit(1); + } + file_path = path[i - 1]; readfd = open(file_path, O_RDONLY); if (readfd < 0) { @@ -138,13 +146,14 @@ int main(int argc, char **argv) for (p = rbuf, len = wlen; len > 0; p += rlen, len -= rlen) { rlen = read(readfd, p, len); + if (rlen < 0 && errno == EINTR) { + continue; + } + if (rlen <= 0) break; } - if (rlen < 0 && errno == EINTR) - continue; - if (len > 0) { fail("read failed: %m\n"); ret = 1; From 62ad2f6095b466bfade1af6fea60bcb0fa1505ec Mon Sep 17 00:00:00 2001 From: Dmitry Safonov Date: Thu, 5 Mar 2020 14:45:34 +0000 Subject: [PATCH 28/48] criu: Remove compel.h includes The plan is to remove "compel.h". That file only includes other headers (which may be not needed). If we aim for one-include-for-compel, we could instead paste all subheaders into "compel.h". Rather, I think it's worth to migrate to more fine-grained compel headers than follow the strategy 'one header to rule them all'. Further, the header creates problems for cross-compilation: it's included in files, those are used by host-compel. Which rightfully confuses compiler/linker as host's definitions for fpu regs/other platform details get drained into host's compel. As a first step - stop including "compel.h" in criu. Signed-off-by: Dmitry Safonov --- criu/aio.c | 2 +- criu/arch/aarch64/crtools.c | 2 +- criu/arch/arm/crtools.c | 3 +-- criu/arch/ppc64/crtools.c | 2 +- criu/arch/s390/crtools.c | 2 +- criu/arch/x86/crtools.c | 2 +- criu/arch/x86/sys-exec-tbl.c | 1 - criu/cr-restore.c | 1 - criu/include/proc_parse.h | 2 +- criu/kerndat.c | 1 - criu/mem.c | 2 +- criu/parasite-syscall.c | 2 -- criu/pie/pie-relocs.h | 2 -- criu/seize.c | 1 - criu/vdso.c | 1 - 15 files changed, 8 insertions(+), 18 deletions(-) diff --git a/criu/aio.c b/criu/aio.c index 45651f2d3..6ee65d5f4 100644 --- a/criu/aio.c +++ b/criu/aio.c @@ -11,7 +11,7 @@ #include "parasite.h" #include "parasite-syscall.h" #include "images/mm.pb-c.h" -#include +#include "compel/infect.h" #define NR_IOEVENTS_IN_NPAGES(npages) ((PAGE_SIZE * (npages) - sizeof(struct aio_ring)) / sizeof(struct io_event)) diff --git a/criu/arch/aarch64/crtools.c b/criu/arch/aarch64/crtools.c index f98743a23..76bd1fea7 100644 --- a/criu/arch/aarch64/crtools.c +++ b/criu/arch/aarch64/crtools.c @@ -19,7 +19,7 @@ #include "util.h" #include "cpu.h" #include "restorer.h" -#include +#include "compel/infect.h" #define assign_reg(dst, src, e) dst->e = (__typeof__(dst->e))(src)->e diff --git a/criu/arch/arm/crtools.c b/criu/arch/arm/crtools.c index c216cdc5c..840d489a6 100644 --- a/criu/arch/arm/crtools.c +++ b/criu/arch/arm/crtools.c @@ -18,8 +18,7 @@ #include "elf.h" #include "parasite-syscall.h" #include "restorer.h" - -#include +#include "compel/infect.h" #define assign_reg(dst, src, e) dst->e = (__typeof__(dst->e))((src)->ARM_##e) diff --git a/criu/arch/ppc64/crtools.c b/criu/arch/ppc64/crtools.c index 5a5966ad4..0d9f49c3f 100644 --- a/criu/arch/ppc64/crtools.c +++ b/criu/arch/ppc64/crtools.c @@ -17,7 +17,7 @@ #include "log.h" #include "util.h" #include "cpu.h" -#include +#include "compel/infect.h" #include "protobuf.h" #include "images/core.pb-c.h" diff --git a/criu/arch/s390/crtools.c b/criu/arch/s390/crtools.c index 238035b76..000b7779f 100644 --- a/criu/arch/s390/crtools.c +++ b/criu/arch/s390/crtools.c @@ -17,7 +17,7 @@ #include "log.h" #include "util.h" #include "cpu.h" -#include +#include "compel/infect.h" #include "protobuf.h" #include "images/core.pb-c.h" diff --git a/criu/arch/x86/crtools.c b/criu/arch/x86/crtools.c index e4073c27b..9c8beeedd 100644 --- a/criu/arch/x86/crtools.c +++ b/criu/arch/x86/crtools.c @@ -1,5 +1,5 @@ #include "compel/asm/fpu.h" -#include "compel/compel.h" +#include "compel/infect.h" #include "compel/plugins/std/syscall-codes.h" #include "cpu.h" #include "cr_options.h" diff --git a/criu/arch/x86/sys-exec-tbl.c b/criu/arch/x86/sys-exec-tbl.c index 608dc2510..225b8a153 100644 --- a/criu/arch/x86/sys-exec-tbl.c +++ b/criu/arch/x86/sys-exec-tbl.c @@ -1,4 +1,3 @@ -#include static struct syscall_exec_desc sc_exec_table_64[] = { #include "sys-exec-tbl-64.c" diff --git a/criu/cr-restore.c b/criu/cr-restore.c index 85105a18e..41146d4ad 100644 --- a/criu/cr-restore.c +++ b/criu/cr-restore.c @@ -67,7 +67,6 @@ #include "timerfd.h" #include "action-scripts.h" #include "shmem.h" -#include #include "aio.h" #include "lsm.h" #include "seccomp.h" diff --git a/criu/include/proc_parse.h b/criu/include/proc_parse.h index 96a097b3d..fd50ff47e 100644 --- a/criu/include/proc_parse.h +++ b/criu/include/proc_parse.h @@ -3,7 +3,7 @@ #include -#include +#include "compel/infect.h" #define PROC_TASK_COMM_LEN 32 #define PROC_TASK_COMM_LEN_FMT "(%31s" diff --git a/criu/kerndat.c b/criu/kerndat.c index 8ac83820b..2ad72c350 100644 --- a/criu/kerndat.c +++ b/criu/kerndat.c @@ -33,7 +33,6 @@ #include "net.h" #include "tun.h" #include -#include #include "netfilter.h" #include "fsnotify.h" #include "linux/userfaultfd.h" diff --git a/criu/mem.c b/criu/mem.c index 4e110c9e9..55022d94a 100644 --- a/criu/mem.c +++ b/criu/mem.c @@ -29,7 +29,7 @@ #include "pagemap-cache.h" #include "fault-injection.h" #include "prctl.h" -#include +#include "compel/infect-util.h" #include "protobuf.h" #include "images/pagemap.pb-c.h" diff --git a/criu/parasite-syscall.c b/criu/parasite-syscall.c index e5a8194e5..b649d1b51 100644 --- a/criu/parasite-syscall.c +++ b/criu/parasite-syscall.c @@ -45,8 +45,6 @@ #include "infect-rpc.h" #include "pie/parasite-blob.h" -#include - unsigned long get_exec_start(struct vm_area_list *vmas) { struct vma_area *vma_area; diff --git a/criu/pie/pie-relocs.h b/criu/pie/pie-relocs.h index 6797486c2..e36126be6 100644 --- a/criu/pie/pie-relocs.h +++ b/criu/pie/pie-relocs.h @@ -1,8 +1,6 @@ #ifndef __PIE_RELOCS_H__ #define __PIE_RELOCS_H__ -#include - #include "common/config.h" #include "common/compiler.h" diff --git a/criu/seize.c b/criu/seize.c index 0ba2d9b1d..f973806d9 100644 --- a/criu/seize.c +++ b/criu/seize.c @@ -23,7 +23,6 @@ #include "string.h" #include "xmalloc.h" #include "util.h" -#include #define NR_ATTEMPTS 5 diff --git a/criu/vdso.c b/criu/vdso.c index 19ba4765d..433a54728 100644 --- a/criu/vdso.c +++ b/criu/vdso.c @@ -20,7 +20,6 @@ #include "criu-log.h" #include "mem.h" #include "vma.h" -#include #include #ifdef LOG_PREFIX From 327554ee646ac8c7728981d0607dc42420c3a85a Mon Sep 17 00:00:00 2001 From: Dmitry Safonov Date: Thu, 5 Mar 2020 15:00:08 +0000 Subject: [PATCH 29/48] compel: Remove compel.h The file only includes other headers (which may be not needed). If we aim for one-include-for-compel, we could instead paste all subheaders into "compel.h". Rather, I think it's worth to migrate to more fine-grained compel headers than follow the strategy 'one header to rule them all'. Further, the header creates problems for cross-compilation: it's included in files, those are used by host-compel. Which rightfully confuses compiler/linker as host's definitions for fpu regs/other platform details get drained into host's compel. Signed-off-by: Dmitry Safonov --- Documentation/compel.txt | 2 +- compel/arch/aarch64/src/lib/handle-elf.c | 3 +-- compel/arch/arm/src/lib/handle-elf.c | 3 +-- compel/arch/arm/src/lib/infect.c | 1 + compel/arch/ppc64/src/lib/cpu.c | 1 + compel/arch/ppc64/src/lib/handle-elf.c | 3 +-- compel/arch/s390/src/lib/handle-elf.c | 3 +-- compel/arch/s390/src/lib/infect.c | 1 + compel/arch/x86/src/lib/handle-elf.c | 3 +-- compel/arch/x86/src/lib/infect.c | 1 + compel/include/log.h | 3 +-- compel/include/uapi/compel.h | 14 -------------- compel/src/lib/handle-elf.c | 6 ++---- compel/src/lib/log.c | 3 --- compel/src/main.c | 2 -- compel/test/fdspy/spy.c | 1 - compel/test/infect/spy.c | 1 - compel/test/rsys/spy.c | 2 -- include/common/scm.h | 2 ++ 19 files changed, 15 insertions(+), 40 deletions(-) delete mode 100644 compel/include/uapi/compel.h diff --git a/Documentation/compel.txt b/Documentation/compel.txt index 744a3b35d..6ccd20861 100644 --- a/Documentation/compel.txt +++ b/Documentation/compel.txt @@ -86,7 +86,7 @@ Infecting code ~~~~~~~~~~~~~~ The parasitic code is compiled and converted to a header using *compel*, and included here. -*#include * +*#include * *#include "parasite.h"* diff --git a/compel/arch/aarch64/src/lib/handle-elf.c b/compel/arch/aarch64/src/lib/handle-elf.c index 1c3686c48..1ee65ee2c 100644 --- a/compel/arch/aarch64/src/lib/handle-elf.c +++ b/compel/arch/aarch64/src/lib/handle-elf.c @@ -1,6 +1,5 @@ #include - -#include "uapi/compel.h" +#include #include "handle-elf.h" #include "piegen.h" diff --git a/compel/arch/arm/src/lib/handle-elf.c b/compel/arch/arm/src/lib/handle-elf.c index 8abf8dad1..5b8d00a6f 100644 --- a/compel/arch/arm/src/lib/handle-elf.c +++ b/compel/arch/arm/src/lib/handle-elf.c @@ -1,6 +1,5 @@ #include - -#include "uapi/compel.h" +#include #include "handle-elf.h" #include "piegen.h" diff --git a/compel/arch/arm/src/lib/infect.c b/compel/arch/arm/src/lib/infect.c index c17cb9c9b..0053bef58 100644 --- a/compel/arch/arm/src/lib/infect.c +++ b/compel/arch/arm/src/lib/infect.c @@ -1,6 +1,7 @@ #include #include #include +#include #include #include #include "common/page.h" diff --git a/compel/arch/ppc64/src/lib/cpu.c b/compel/arch/ppc64/src/lib/cpu.c index 338ab4891..7a3972790 100644 --- a/compel/arch/ppc64/src/lib/cpu.c +++ b/compel/arch/ppc64/src/lib/cpu.c @@ -2,6 +2,7 @@ #include #include #include +#include #include "compel-cpu.h" diff --git a/compel/arch/ppc64/src/lib/handle-elf.c b/compel/arch/ppc64/src/lib/handle-elf.c index 3d4020f59..f29fdc8a3 100644 --- a/compel/arch/ppc64/src/lib/handle-elf.c +++ b/compel/arch/ppc64/src/lib/handle-elf.c @@ -1,6 +1,5 @@ #include - -#include "uapi/compel.h" +#include #include "handle-elf.h" #include "piegen.h" diff --git a/compel/arch/s390/src/lib/handle-elf.c b/compel/arch/s390/src/lib/handle-elf.c index 01a8bf4c8..6ed382c92 100644 --- a/compel/arch/s390/src/lib/handle-elf.c +++ b/compel/arch/s390/src/lib/handle-elf.c @@ -1,6 +1,5 @@ #include - -#include "uapi/compel.h" +#include #include "handle-elf.h" #include "piegen.h" diff --git a/compel/arch/s390/src/lib/infect.c b/compel/arch/s390/src/lib/infect.c index 7e7d24ce2..5a4675449 100644 --- a/compel/arch/s390/src/lib/infect.c +++ b/compel/arch/s390/src/lib/infect.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include diff --git a/compel/arch/x86/src/lib/handle-elf.c b/compel/arch/x86/src/lib/handle-elf.c index 62fb28f49..938999b2e 100644 --- a/compel/arch/x86/src/lib/handle-elf.c +++ b/compel/arch/x86/src/lib/handle-elf.c @@ -1,6 +1,5 @@ #include - -#include "uapi/compel.h" +#include #include "handle-elf.h" #include "piegen.h" diff --git a/compel/arch/x86/src/lib/infect.c b/compel/arch/x86/src/lib/infect.c index 11e7f4c91..9c4abb60c 100644 --- a/compel/arch/x86/src/lib/infect.c +++ b/compel/arch/x86/src/lib/infect.c @@ -3,6 +3,7 @@ #include #include #include +#include #include diff --git a/compel/include/log.h b/compel/include/log.h index 559f909ce..49e65bb50 100644 --- a/compel/include/log.h +++ b/compel/include/log.h @@ -1,8 +1,7 @@ #ifndef COMPEL_LOG_H__ #define COMPEL_LOG_H__ -#include "uapi/compel/compel.h" -#include "uapi/compel/loglevels.h" +#include "uapi/compel/log.h" #ifndef LOG_PREFIX # define LOG_PREFIX diff --git a/compel/include/uapi/compel.h b/compel/include/uapi/compel.h deleted file mode 100644 index 318a472da..000000000 --- a/compel/include/uapi/compel.h +++ /dev/null @@ -1,14 +0,0 @@ -#ifndef UAPI_COMPEL_H__ -#define UAPI_COMPEL_H__ - -#include -#include - -#include -#include -#include -#include -#include -#include - -#endif /* UAPI_COMPEL_H__ */ diff --git a/compel/src/lib/handle-elf.c b/compel/src/lib/handle-elf.c index ca7c53b71..69d5104b6 100644 --- a/compel/src/lib/handle-elf.c +++ b/compel/src/lib/handle-elf.c @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include @@ -12,8 +12,6 @@ #include #include -#include "uapi/compel.h" - #include "handle-elf.h" #include "piegen.h" #include "log.h" @@ -228,7 +226,7 @@ int __handle_elf(void *mem, size_t size) } pr_out("/* Autogenerated from %s */\n", opts.input_filename); - pr_out("#include \n"); + pr_out("#include \n"); for (i = 0; i < symtab_hdr->sh_size / symtab_hdr->sh_entsize; i++) { Elf_Sym *sym = &symbols[i]; diff --git a/compel/src/lib/log.c b/compel/src/lib/log.c index d195343e4..c86be02c5 100644 --- a/compel/src/lib/log.c +++ b/compel/src/lib/log.c @@ -4,11 +4,8 @@ #include #include #include - #include -#include - #include "log.h" static unsigned int current_loglevel = COMPEL_DEFAULT_LOGLEVEL; diff --git a/compel/src/main.c b/compel/src/main.c index 8b2c8bc8d..36127c357 100644 --- a/compel/src/main.c +++ b/compel/src/main.c @@ -13,8 +13,6 @@ #include #include -#include "uapi/compel/compel.h" - #include "version.h" #include "piegen.h" #include "log.h" diff --git a/compel/test/fdspy/spy.c b/compel/test/fdspy/spy.c index 258e3ab75..1a373b6bb 100644 --- a/compel/test/fdspy/spy.c +++ b/compel/test/fdspy/spy.c @@ -5,7 +5,6 @@ #include #include -#include #include "parasite.h" #define PARASITE_CMD_GETFD PARASITE_USER_CMDS diff --git a/compel/test/infect/spy.c b/compel/test/infect/spy.c index a5aba7308..b5f8b2559 100644 --- a/compel/test/infect/spy.c +++ b/compel/test/infect/spy.c @@ -3,7 +3,6 @@ #include #include -#include #include "parasite.h" #define PARASITE_CMD_INC PARASITE_USER_CMDS diff --git a/compel/test/rsys/spy.c b/compel/test/rsys/spy.c index f5c999d5a..98654efcf 100644 --- a/compel/test/rsys/spy.c +++ b/compel/test/rsys/spy.c @@ -4,8 +4,6 @@ #include #include -#include - static void print_vmsg(unsigned int lvl, const char *fmt, va_list parms) { printf("\tLC%u: ", lvl); diff --git a/include/common/scm.h b/include/common/scm.h index ab27137b8..a8eb9ec4c 100644 --- a/include/common/scm.h +++ b/include/common/scm.h @@ -3,7 +3,9 @@ #include #include +#include #include +#include /* * Because of kernel doing kmalloc for user data passed From 18ac1540c4b64108b53fcd8fa3b3df256075e3f6 Mon Sep 17 00:00:00 2001 From: Dmitry Safonov Date: Thu, 5 Mar 2020 15:04:15 +0000 Subject: [PATCH 30/48] travis: Add aarch64-cross test on amd64 Fixes: #924 Signed-off-by: Dmitry Safonov --- .travis.yml | 4 +++ scripts/build/Dockerfile.aarch64-cross | 45 ++++++++++++++++++++++++++ scripts/build/Makefile | 2 +- 3 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 scripts/build/Dockerfile.aarch64-cross diff --git a/.travis.yml b/.travis.yml index 7c36af006..ffa82f15f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -86,6 +86,10 @@ jobs: arch: amd64 env: TR_ARCH=armv7-cross dist: bionic + - os: linux + arch: amd64 + env: TR_ARCH=aarch64-cross + dist: bionic allow_failures: - env: TR_ARCH=docker-test - env: TR_ARCH=docker-test DIST=xenial diff --git a/scripts/build/Dockerfile.aarch64-cross b/scripts/build/Dockerfile.aarch64-cross new file mode 100644 index 000000000..38229497a --- /dev/null +++ b/scripts/build/Dockerfile.aarch64-cross @@ -0,0 +1,45 @@ +FROM dockcross/base:latest + +# Add the cross compiler sources +RUN echo "deb http://ftp.us.debian.org/debian/ jessie main" >> /etc/apt/sources.list && \ + dpkg --add-architecture arm64 && \ + apt-get install emdebian-archive-keyring + +RUN apt-get update && apt-get install -y \ + crossbuild-essential-arm64 \ + libc6-dev-arm64-cross \ + libc6-arm64-cross \ + libbz2-dev:arm64 \ + libexpat1-dev:arm64 \ + ncurses-dev:arm64 \ + libssl-dev:arm64 \ + protobuf-c-compiler \ + protobuf-compiler \ + python-protobuf \ + libnl-3-dev:arm64 \ + libprotobuf-dev:arm64 \ + libnet-dev:arm64 \ + libprotobuf-c-dev:arm64 \ + libcap-dev:arm64 \ + libaio-dev:arm64 \ + libnl-route-3-dev:arm64 + +ENV CROSS_TRIPLE=aarch64-linux-gnu +ENV CROSS_COMPILE=${CROSS_TRIPLE}- \ + CROSS_ROOT=/usr/${CROSS_TRIPLE} \ + AS=/usr/bin/${CROSS_TRIPLE}-as \ + AR=/usr/bin/${CROSS_TRIPLE}-ar \ + CC=/usr/bin/${CROSS_TRIPLE}-gcc \ + CPP=/usr/bin/${CROSS_TRIPLE}-cpp \ + CXX=/usr/bin/${CROSS_TRIPLE}-g++ \ + LD=/usr/bin/${CROSS_TRIPLE}-ld \ + FC=/usr/bin/${CROSS_TRIPLE}-gfortran + +ENV PATH="${PATH}:${CROSS_ROOT}/bin" \ + PKG_CONFIG_PATH=/usr/lib/${CROSS_TRIPLE}/pkgconfig \ + ARCH=aarch64 + +COPY . /criu +WORKDIR /criu + +RUN make mrproper && date && make -j $(nproc) zdtm && date diff --git a/scripts/build/Makefile b/scripts/build/Makefile index d093ce76c..913a86d6c 100644 --- a/scripts/build/Makefile +++ b/scripts/build/Makefile @@ -2,7 +2,7 @@ ARCHES := x86_64 fedora-asan fedora-rawhide centos armv7hf TARGETS := $(ARCHES) alpine TARGETS_CLANG := $(addsuffix $(TARGETS),-clang) CONTAINER_RUNTIME := docker -TARGETS += armv7-cross +TARGETS += armv7-cross aarch64-cross all: $(TARGETS) $(TARGETS_CLANG) .PHONY: all From 1f74f8d77087ce06624cb54f2fa70afaf1380103 Mon Sep 17 00:00:00 2001 From: Dmitry Safonov Date: Thu, 5 Mar 2020 16:43:48 +0000 Subject: [PATCH 31/48] travis: Use debian/buster as base for cross build tests Jessie is called 'oldoldstable', migrate to Buster. Suggested-by: Adrian Reber Signed-off-by: Dmitry Safonov --- scripts/build/Dockerfile.aarch64-cross | 2 +- scripts/build/Dockerfile.armv7-cross | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/build/Dockerfile.aarch64-cross b/scripts/build/Dockerfile.aarch64-cross index 38229497a..252e0f875 100644 --- a/scripts/build/Dockerfile.aarch64-cross +++ b/scripts/build/Dockerfile.aarch64-cross @@ -1,7 +1,7 @@ FROM dockcross/base:latest # Add the cross compiler sources -RUN echo "deb http://ftp.us.debian.org/debian/ jessie main" >> /etc/apt/sources.list && \ +RUN echo "deb http://ftp.us.debian.org/debian/ buster main" >> /etc/apt/sources.list && \ dpkg --add-architecture arm64 && \ apt-get install emdebian-archive-keyring diff --git a/scripts/build/Dockerfile.armv7-cross b/scripts/build/Dockerfile.armv7-cross index 434934aad..17a55561e 100644 --- a/scripts/build/Dockerfile.armv7-cross +++ b/scripts/build/Dockerfile.armv7-cross @@ -1,7 +1,7 @@ FROM dockcross/base:latest # Add the cross compiler sources -RUN echo "deb http://ftp.us.debian.org/debian/ jessie main" >> /etc/apt/sources.list && \ +RUN echo "deb http://ftp.us.debian.org/debian/ buster main" >> /etc/apt/sources.list && \ dpkg --add-architecture armhf && \ apt-get install emdebian-archive-keyring From b9c8e957d8f198fb47ed9e73a5d5c3727ba4d4cc Mon Sep 17 00:00:00 2001 From: Alexander Mikhalitsyn Date: Tue, 10 Mar 2020 17:40:57 +0300 Subject: [PATCH 32/48] crit-recode: skip (not try to parse) nftables raw image We should ignore (not parse) images that has non-crtool format, that images has no magic number (RAW_IMAGE_MAGIC equals 0). nftables images has format compatible with `nft -f /proc/self/fd/0` input format. Reported-by: Mr Jenkins Signed-off-by: Alexander Mikhalitsyn (Virtuozzo) --- test/crit-recode.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/crit-recode.py b/test/crit-recode.py index a7dcc7272..adaf33733 100755 --- a/test/crit-recode.py +++ b/test/crit-recode.py @@ -47,6 +47,8 @@ for imgf in find.stdout.readlines(): continue if imgf_b.startswith(b'ip6tables-'): continue + if imgf_b.startswith(b'nftables-'): + continue if imgf_b.startswith(b'route-'): continue if imgf_b.startswith(b'route6-'): From c3ad4942d43524c617bb77345ce8912461b6f9aa Mon Sep 17 00:00:00 2001 From: Mike Rapoport Date: Thu, 5 Mar 2020 20:46:16 +0200 Subject: [PATCH 33/48] travis: add ppc64-cross test on amd64 Signed-off-by: Mike Rapoport --- .travis.yml | 4 +++ scripts/build/Dockerfile.ppc64-cross | 45 ++++++++++++++++++++++++++++ scripts/build/Makefile | 2 +- 3 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 scripts/build/Dockerfile.ppc64-cross diff --git a/.travis.yml b/.travis.yml index ffa82f15f..9928f16c2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -90,6 +90,10 @@ jobs: arch: amd64 env: TR_ARCH=aarch64-cross dist: bionic + - os: linux + arch: amd64 + env: TR_ARCH=ppc64-cross + dist: bionic allow_failures: - env: TR_ARCH=docker-test - env: TR_ARCH=docker-test DIST=xenial diff --git a/scripts/build/Dockerfile.ppc64-cross b/scripts/build/Dockerfile.ppc64-cross new file mode 100644 index 000000000..44061c558 --- /dev/null +++ b/scripts/build/Dockerfile.ppc64-cross @@ -0,0 +1,45 @@ +FROM dockcross/base:latest + +# Add the cross compiler sources +RUN echo "deb http://ftp.us.debian.org/debian/ buster main" >> /etc/apt/sources.list && \ + dpkg --add-architecture ppc64el && \ + apt-get install emdebian-archive-keyring + +RUN apt-get update && apt-get install -y \ + crossbuild-essential-ppc64el \ + libc6-dev-ppc64el-cross \ + libc6-ppc64el-cross \ + libbz2-dev:ppc64el \ + libexpat1-dev:ppc64el \ + ncurses-dev:ppc64el \ + libssl-dev:ppc64el \ + protobuf-c-compiler \ + protobuf-compiler \ + python-protobuf \ + libnl-3-dev:ppc64el \ + libprotobuf-dev:ppc64el \ + libnet-dev:ppc64el \ + libprotobuf-c-dev:ppc64el \ + libcap-dev:ppc64el \ + libaio-dev:ppc64el \ + libnl-route-3-dev:ppc64el + +ENV CROSS_TRIPLE=powerpc64le-linux-gnu +ENV CROSS_COMPILE=${CROSS_TRIPLE}- \ + CROSS_ROOT=/usr/${CROSS_TRIPLE} \ + AS=/usr/bin/${CROSS_TRIPLE}-as \ + AR=/usr/bin/${CROSS_TRIPLE}-ar \ + CC=/usr/bin/${CROSS_TRIPLE}-gcc \ + CPP=/usr/bin/${CROSS_TRIPLE}-cpp \ + CXX=/usr/bin/${CROSS_TRIPLE}-g++ \ + LD=/usr/bin/${CROSS_TRIPLE}-ld \ + FC=/usr/bin/${CROSS_TRIPLE}-gfortran + +ENV PATH="${PATH}:${CROSS_ROOT}/bin" \ + PKG_CONFIG_PATH=/usr/lib/${CROSS_TRIPLE}/pkgconfig \ + ARCH=ppc64 + +COPY . /criu +WORKDIR /criu + +RUN make mrproper && date && make -j $(nproc) zdtm && date diff --git a/scripts/build/Makefile b/scripts/build/Makefile index 913a86d6c..855539152 100644 --- a/scripts/build/Makefile +++ b/scripts/build/Makefile @@ -2,7 +2,7 @@ ARCHES := x86_64 fedora-asan fedora-rawhide centos armv7hf TARGETS := $(ARCHES) alpine TARGETS_CLANG := $(addsuffix $(TARGETS),-clang) CONTAINER_RUNTIME := docker -TARGETS += armv7-cross aarch64-cross +TARGETS += armv7-cross aarch64-cross ppc64-cross all: $(TARGETS) $(TARGETS_CLANG) .PHONY: all From d0d6f1ad108a6bd7eb0e2019aaca7689bff45275 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sat, 21 Mar 2020 10:35:42 -0700 Subject: [PATCH 34/48] mailmap: update my email Signed-off-by: Andrei Vagin --- .mailmap | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.mailmap b/.mailmap index d8c3f594d..6f046b972 100644 --- a/.mailmap +++ b/.mailmap @@ -1,6 +1,8 @@ Stanislav Kinsbursky Pavel Emelyanov -Andrey Vagin -Andrey Vagin -Andrey Vagin Andrew Vagin +Andrei Vagin +Andrei Vagin +Andrei Vagin +Andrei Vagin +Andrei Vagin Cyrill Gorcunov From f42ae70c75802787e980715a7ca895eb2b390d06 Mon Sep 17 00:00:00 2001 From: Nicolas Viennot Date: Fri, 20 Mar 2020 23:12:59 +0000 Subject: [PATCH 35/48] make: use cflags/ldflags for config.h detection mechanism The config.h detection scripts should use the provided CFLAGS/LDFLAGS as it tries to link libnl, libnet, and others. Signed-off-by: Nicolas Viennot --- scripts/nmk/scripts/utils.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/nmk/scripts/utils.mk b/scripts/nmk/scripts/utils.mk index 0cf216bc0..b9790615c 100644 --- a/scripts/nmk/scripts/utils.mk +++ b/scripts/nmk/scripts/utils.mk @@ -3,7 +3,7 @@ ifndef ____nmk_defined__utils # # Usage: option := $(call try-compile,language,source-to-build,cc-options,cc-defines) try-compile = $(shell sh -c 'echo "$(2)" | \ - $(CC) $(4) -x $(1) - $(3) -o /dev/null > /dev/null 2>&1 && \ + $(CC) $(CFLAGS) $(LDFLAGS) $(4) -x $(1) - $(3) -o /dev/null > /dev/null 2>&1 && \ echo true || echo false') # From fb65ab2b1a47558c2fe92a635630d53b971e5876 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sun, 15 Mar 2020 10:44:14 +0300 Subject: [PATCH 36/48] mem: dump shared memory file descriptors Any shared memroy mapping can be opened via /proc/self/maps_files/. Such file descriptors look like memfd file descriptors, so they can be dumped by the same way. Signed-off-by: Andrei Vagin --- criu/files.c | 2 +- criu/include/memfd.h | 2 +- criu/memfd.c | 11 +++++++---- criu/proc_parse.c | 38 ++++++++++---------------------------- 4 files changed, 19 insertions(+), 34 deletions(-) diff --git a/criu/files.c b/criu/files.c index f6ba39a30..a1fd26764 100644 --- a/criu/files.c +++ b/criu/files.c @@ -552,7 +552,7 @@ static int dump_one_file(struct pid *pid, int fd, int lfd, struct fd_opts *opts, p.link = &link; - if (is_memfd(p.stat.st_dev, &link.name[1])) + if (is_memfd(p.stat.st_dev)) ops = &memfd_dump_ops; else if (link.name[1] == '/') ops = ®file_dump_ops; diff --git a/criu/include/memfd.h b/criu/include/memfd.h index 2d8eda545..4189766fd 100644 --- a/criu/include/memfd.h +++ b/criu/include/memfd.h @@ -8,7 +8,7 @@ struct fd_parms; struct file_desc; -extern int is_memfd(dev_t dev, const char *path); +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; diff --git a/criu/memfd.c b/criu/memfd.c index 983e01b38..bca6900cb 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -58,15 +58,14 @@ static LIST_HEAD(memfd_inodes); static u32 memfd_inode_ids = 1; -int is_memfd(dev_t dev, const char *path) +int is_memfd(dev_t dev) { /* * TODO When MAP_HUGETLB is used, the file device is not shmem_dev, * Note that other parts of CRIU have similar issues, see * is_anon_shmem_map(). */ - return dev == kdat.shmem_dev && - !strncmp(path, MEMFD_PREFIX, MEMFD_PREFIX_LEN); + return dev == kdat.shmem_dev; } static int dump_memfd_inode(int fd, struct memfd_inode *inode, @@ -167,7 +166,11 @@ static int dump_one_memfd(int lfd, u32 id, const struct fd_parms *p) link = p->link; strip_deleted(link); - name = &link->name[1+MEMFD_PREFIX_LEN]; + /* link->name is always started with "." which has to be skipped. */ + if (strncmp(link->name + 1, MEMFD_PREFIX, MEMFD_PREFIX_LEN) == 0) + name = &link->name[1 + MEMFD_PREFIX_LEN]; + else + name = link->name + 1; inode = dump_unique_memfd_inode(lfd, name, &p->stat); if (!inode) diff --git a/criu/proc_parse.c b/criu/proc_parse.c index 468afcdf3..980342870 100644 --- a/criu/proc_parse.c +++ b/criu/proc_parse.c @@ -305,7 +305,7 @@ static int vma_get_mapfile_user(const char *fname, struct vma_area *vma, vfi_dev = makedev(vfi->dev_maj, vfi->dev_min); - if (is_memfd(vfi_dev, fname)) { + if (is_memfd(vfi_dev)) { struct fd_link link; link.len = strlen(fname); strlcpy(link.name, fname, sizeof(link.name)); @@ -596,39 +596,21 @@ static int handle_vma(pid_t pid, struct vma_area *vma_area, goto err; } - /* - * /dev/zero stands for anon-shared mapping - * otherwise it's some file mapping. - * - * We treat memfd mappings as regular file mappings because - * their backing can be seen as files, which is easy to - * support. So even though memfd is an anonymous shmem, we - * treat it differently. - * Note: maybe we should revisit this as /proc/map_files/ - * may not always be accessible. - */ - - if (is_memfd(st_buf->st_dev, file_path)) { - vma_area->e->status |= VMA_AREA_MEMFD; - goto normal_file; - } - - if (is_anon_shmem_map(st_buf->st_dev)) { - if (!(vma_area->e->flags & MAP_SHARED)) - goto err_bogus_mapping; + if (is_anon_shmem_map(st_buf->st_dev) && !strncmp(file_path, "/SYSV", 5)) { vma_area->e->flags |= MAP_ANONYMOUS; vma_area->e->status |= VMA_ANON_SHARED; vma_area->e->shmid = st_buf->st_ino; - - if (!strncmp(file_path, "/SYSV", 5)) { - pr_info("path: %s\n", file_path); - vma_area->e->status |= VMA_AREA_SYSVIPC; - } else { + if (!(vma_area->e->flags & MAP_SHARED)) + goto err_bogus_mapping; + pr_info("path: %s\n", file_path); + vma_area->e->status |= VMA_AREA_SYSVIPC; + } else { + if (is_anon_shmem_map(st_buf->st_dev)) { + vma_area->e->status |= VMA_AREA_MEMFD; if (fault_injected(FI_HUGE_ANON_SHMEM_ID)) vma_area->e->shmid += FI_HUGE_ANON_SHMEM_ID_BASE; } - } else { -normal_file: + if (vma_area->e->flags & MAP_PRIVATE) vma_area->e->status |= VMA_FILE_PRIVATE; else From 10b1d46f674ec458cd1a006eb1b0546bf5a7135c Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 19 Mar 2020 09:37:18 +0300 Subject: [PATCH 37/48] mem/vma: set VMA_FILE_{PRIVATE,SHARED} if a vma file is borrowed Here is a fast path when two consequent vma-s share the same file. But one of these vma-s can map a file with MAP_SHARED, but another one can map it with MAP_PRIVATE and we need to take this into account. --- criu/proc_parse.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/criu/proc_parse.c b/criu/proc_parse.c index 980342870..60aba8788 100644 --- a/criu/proc_parse.c +++ b/criu/proc_parse.c @@ -584,6 +584,14 @@ static int handle_vma(pid_t pid, struct vma_area *vma_area, vma_area->e->shmid = prev->e->shmid; vma_area->vmst = prev->vmst; vma_area->mnt_id = prev->mnt_id; + + if (!(vma_area->e->status & VMA_AREA_SYSVIPC)) { + vma_area->e->status &= ~(VMA_FILE_PRIVATE | VMA_FILE_SHARED); + if (vma_area->e->flags & MAP_PRIVATE) + vma_area->e->status |= VMA_FILE_PRIVATE; + else + vma_area->e->status |= VMA_FILE_SHARED; + } } else if (*vm_file_fd >= 0) { struct stat *st_buf = vma_area->vmst; From c40c09cbbf03afc058a761314fcdb14a3f69cb53 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sun, 15 Mar 2020 10:53:51 +0300 Subject: [PATCH 38/48] test/zdtmp: add a test to C/R shared memory file descriptors Any shared memory region can be openned via /proc/self/map_files. Signed-off-by: Andrei Vagin --- test/zdtm/static/Makefile | 2 + test/zdtm/static/shmemfd-priv.c | 84 ++++++++++++++++++++++ test/zdtm/static/shmemfd-priv.desc | 1 + test/zdtm/static/shmemfd.c | 107 +++++++++++++++++++++++++++++ test/zdtm/static/shmemfd.desc | 1 + 5 files changed, 195 insertions(+) create mode 100644 test/zdtm/static/shmemfd-priv.c create mode 100644 test/zdtm/static/shmemfd-priv.desc create mode 100644 test/zdtm/static/shmemfd.c create mode 100644 test/zdtm/static/shmemfd.desc diff --git a/test/zdtm/static/Makefile b/test/zdtm/static/Makefile index ee69612c7..a8e4107d3 100644 --- a/test/zdtm/static/Makefile +++ b/test/zdtm/static/Makefile @@ -224,6 +224,8 @@ TST_NOFILE := \ memfd01 \ memfd02 \ memfd03 \ + shmemfd \ + shmemfd-priv \ # jobctl00 \ ifneq ($(ARCH),arm) diff --git a/test/zdtm/static/shmemfd-priv.c b/test/zdtm/static/shmemfd-priv.c new file mode 100644 index 000000000..bbdb46905 --- /dev/null +++ b/test/zdtm/static/shmemfd-priv.c @@ -0,0 +1,84 @@ +#include +#include + +#include +#include +#include +#include +#include + +#include "zdtmtst.h" + +const char *test_doc = "Test C/R of shared memory file descriptors"; +const char *test_author = "Andrei Vagin "; + +#define err(exitcode, msg, ...) ({ pr_perror(msg, ##__VA_ARGS__); exit(exitcode); }) + +int main(int argc, char *argv[]) +{ + void *addr, *priv_addr, *addr2; + char path[4096]; + int fd; + + test_init(argc, argv); + + addr = mmap(NULL, 5 * PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, -1, 0); + if (addr == MAP_FAILED) { + pr_perror("mmap"); + return 1; + } + + *(int *) addr = 1; + *(int *) (addr + PAGE_SIZE) = 11; + *(int *) (addr + 2 * PAGE_SIZE) = 111; + + snprintf(path, sizeof(path), "/proc/self/map_files/%lx-%lx", + (long)addr, (long)addr + 5 * PAGE_SIZE); + fd = open(path, O_RDWR | O_LARGEFILE); + if (fd < 0) + err(1, "Can't open %s", path); + + priv_addr = mmap(NULL, 5 * PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_FILE | MAP_PRIVATE, fd, PAGE_SIZE); + if (priv_addr == MAP_FAILED) { + pr_perror("mmap"); + return 1; + } + + addr2 = mmap(NULL, 5 * PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_FILE | MAP_SHARED, fd, 2 * PAGE_SIZE); + if (addr2 == MAP_FAILED) { + pr_perror("mmap"); + return 1; + } + + *(int *) (priv_addr + PAGE_SIZE) = 22; + + test_daemon(); + test_waitsig(); + + if (*(int *) (priv_addr + PAGE_SIZE) != 22) { + fail("the second page of the private mapping is corrupted"); + return 1; + } + if (*(int *) (priv_addr) != 11) { + fail("the first page of the private mapping is corrupted"); + return 1; + } + if (*(int *) (addr2) != 111) { + fail("the first page of the second shared mapping is corrupted"); + return 1; + } + *(int *) (addr2) = 333; + if (*(int *) (addr + 2 * PAGE_SIZE) != 333) { + fail("the first page of the second shared mapping isn't shared"); + return 1; + } + *(int *) (addr + 3 * PAGE_SIZE) = 444; + if (*(int *) (priv_addr + 2 * PAGE_SIZE) != 444) { + fail("the third page of the private mapping is corrupted"); + return 1; + } + + pass(); + + return 0; +} diff --git a/test/zdtm/static/shmemfd-priv.desc b/test/zdtm/static/shmemfd-priv.desc new file mode 100644 index 000000000..d969725f6 --- /dev/null +++ b/test/zdtm/static/shmemfd-priv.desc @@ -0,0 +1 @@ +{'flavor': 'h ns', 'flags': 'suid'} diff --git a/test/zdtm/static/shmemfd.c b/test/zdtm/static/shmemfd.c new file mode 100644 index 000000000..b65faa2e1 --- /dev/null +++ b/test/zdtm/static/shmemfd.c @@ -0,0 +1,107 @@ +#include +#include + +#include +#include +#include +#include +#include + +#include "zdtmtst.h" + +const char *test_doc = "Test C/R of shared memory file descriptors"; +const char *test_author = "Andrei Vagin "; + +#define err(exitcode, msg, ...) ({ pr_perror(msg, ##__VA_ARGS__); exit(exitcode); }) + +int main(int argc, char *argv[]) +{ + int fd, fl_flags1, fl_flags2, fd_flags1, fd_flags2; + struct statfs statfs1, statfs2; + off_t pos1, pos2; + char path[4096]; + char buf[5]; + void *addr; + + test_init(argc, argv); + + addr = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_SHARED, -1, 0); + if (addr == MAP_FAILED) { + pr_perror("mmap"); + return 1; + } + + snprintf(path, sizeof(path), "/proc/self/map_files/%lx-%lx", + (long)addr, (long)addr + PAGE_SIZE); + fd = open(path, O_RDWR | O_LARGEFILE); + if (fd < 0) + err(1, "Can't open %s", path); + ftruncate(fd, 0); + munmap(addr, PAGE_SIZE); + + if (fcntl(fd, F_SETFL, O_APPEND) < 0) + err(1, "Can't get fl flags"); + + if ((fl_flags1 = fcntl(fd, F_GETFL)) == -1) + err(1, "Can't get fl flags"); + + if ((fd_flags1 = fcntl(fd, F_GETFD)) == -1) + err(1, "Can't get fd flags"); + + if (fstatfs(fd, &statfs1) < 0) + err(1, "statfs issue"); + + if (write(fd, "hello", 5) != 5) + err(1, "write error"); + + pos1 = 3; + if (lseek(fd, pos1, SEEK_SET) < 0) + err(1, "seek error"); + + test_daemon(); + test_waitsig(); + + if ((fl_flags2 = fcntl(fd, F_GETFL)) == -1) + err(1, "Can't get fl flags"); + + if (fl_flags1 != fl_flags2) { + fail("fl flags differs %x %x", fl_flags1, fl_flags2); + return 1; + } + + if ((fd_flags2 = fcntl(fd, F_GETFD)) == -1) + err(1, "Can't get fd flags"); + + if (fd_flags1 != fd_flags2) { + fail("fd flags differs"); + return 1; + } + + if (fstatfs(fd, &statfs2) < 0) + err(1, "statfs issue"); + + if (statfs1.f_type != statfs2.f_type) { + fail("statfs.f_type differs"); + return 1; + } + + pos2 = lseek(fd, 0, SEEK_CUR); + if (pos1 != pos2) { + fail("position differs"); + return 1; + } + + if (pread(fd, buf, sizeof(buf), 0) != sizeof(buf)) { + fail("read problem"); + return 1; + } + + if (memcmp(buf, "hello", sizeof(buf))) { + fail("content mismatch"); + return 1; + } + + pass(); + + return 0; +} diff --git a/test/zdtm/static/shmemfd.desc b/test/zdtm/static/shmemfd.desc new file mode 100644 index 000000000..d969725f6 --- /dev/null +++ b/test/zdtm/static/shmemfd.desc @@ -0,0 +1 @@ +{'flavor': 'h ns', 'flags': 'suid'} From 691b4a4e7ee980778d8f13eaebddf9b04063942a Mon Sep 17 00:00:00 2001 From: Valeriy Vdovin Date: Mon, 3 Feb 2020 15:08:26 +0300 Subject: [PATCH 39/48] zdtm: Implemented get_current_dir_name wrapper that checks for 'x' permissions Any filesystem syscall, that needs to navigate to inode by it's absolute path performs successive lookup operations for each part of the path. Lookup operation includes access rights check. Usually but not always zdtm tests processes fall under 'other' access category. Also, usually directories don't have 'x' bit set for other. In case when bit 'x' is not set and user-ID and group-ID of a process relate it to 'other', test's will not succeed in performing these syscalls which are most of filesystem api, that has const char *path as part of it arguments (open, openat, mkdir, bind, etc). The observable behavior of that is that zdtm tests fail at file creation ops on one system and pass on the other. The above is not immediately clear to the developer by just looking at failed test's logs. Investigation of that is also not quick for a developer due to the complex structure of zdtm runtime where nested clones with NAMESPACE flags take place alongside with bind-mounts. As an additional note: 'get_current_dir_name' is documented as returning EACCESS in case when some part of the path lacks read/list permissions. But in fact it's not always so. Practice shows, that test processes can get false success on this operation only to fail on later call to something like mkdir/mknod/bind with a given path in arguments. 'get_cwd_check_perm' is a wrapper around 'get_current_dir_name'. It also checks for permissions on the given filepath and logs the error. This directs the developer towards the right investigation path or even eliminates the need for investigation completely. Signed-off-by: Valeriy Vdovin --- test/zdtm/lib/fs.c | 24 ++++++++++++++++++++++++ test/zdtm/lib/fs.h | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/test/zdtm/lib/fs.c b/test/zdtm/lib/fs.c index 0decfc37b..e82011ec8 100644 --- a/test/zdtm/lib/fs.c +++ b/test/zdtm/lib/fs.c @@ -94,3 +94,27 @@ err: mnt_info_free(&m); goto out; } + +int get_cwd_check_perm(char **result) +{ + char *cwd; + *result = 0; + cwd = get_current_dir_name(); + if (!cwd) { + pr_perror("failed to get current directory"); + return -1; + } + + if (access(cwd, X_OK)) { + pr_err("access check for bit X for current dir path '%s' " + "failed for uid:%d,gid:%d, error: %d(%s). " + "Bit 'x' should be set in all path components of " + "this directory\n", + cwd, getuid(), getgid(), errno, strerror(errno) + ); + return -1; + } + + *result = cwd; + return 0; +} diff --git a/test/zdtm/lib/fs.h b/test/zdtm/lib/fs.h index 972b15aba..af7a665fb 100644 --- a/test/zdtm/lib/fs.h +++ b/test/zdtm/lib/fs.h @@ -50,4 +50,28 @@ extern mnt_info_t *mnt_info_alloc(void); extern void mnt_info_free(mnt_info_t **m); extern mnt_info_t *get_cwd_mnt_info(void); +/* + * get_cwd_check_perm is called to check that cwd is actually usable for a calling + * process. + * + * Example output of a stat command on a '/root' path shows file access bits: + * > stat /root + * File: ‘/root’ + * ... + * Access: (0550/dr-xr-x---) Uid: ( 0/root) Gid: ( 0/root) + * ^- no 'x' bit for other + * + * Here we can see that '/root' dir (that often can be part of cwd path) does not + * allow non-root user and non-root group to list contents of this directory. + * Calling process matching 'other' access category may succeed getting cwd path, but will + * fail performing further filesystem operations based on this path with confusing errors. + * + * This function calls get_current_dir_name and explicitly checks that bit 'x' is enabled for + * a calling process and logs the error. + * + * If check passes, stores get_current_dir's result in *result and returns 0 + * If check fails, stores 0 in *result and returns -1 + */ +extern int get_cwd_check_perm(char **result); + #endif /* ZDTM_FS_H_ */ From fa705e418b4e7c2bce0925ad9f8689cd40b0c00d Mon Sep 17 00:00:00 2001 From: Valeriy Vdovin Date: Mon, 3 Feb 2020 15:27:40 +0300 Subject: [PATCH 40/48] zdtm: Use safe helper function to initialize unix socket sockaddr structure The helper function removes code duplication from tests that want to initialize unix socket address to an absolute file path, derived from current working directory of the test + relative filename of a resulting socket. Because the former code used cwd = get_current_dir_name() as part of absolute filename generation, the resulting filepath could later cause failure of bind systcall due to unchecked permissions and introduce confusing permission errors. Signed-off-by: Valeriy Vdovin --- test/zdtm/lib/Makefile | 2 +- test/zdtm/lib/unix.c | 19 ++++++++++++++++ test/zdtm/lib/zdtmtst.h | 3 +++ test/zdtm/static/del_standalone_un.c | 17 +------------- test/zdtm/static/deleted_unix_sock.c | 19 ++-------------- test/zdtm/static/sk-unix01.c | 33 ++++++---------------------- 6 files changed, 33 insertions(+), 60 deletions(-) create mode 100644 test/zdtm/lib/unix.c diff --git a/test/zdtm/lib/Makefile b/test/zdtm/lib/Makefile index b87f36e8f..89ca90933 100644 --- a/test/zdtm/lib/Makefile +++ b/test/zdtm/lib/Makefile @@ -4,7 +4,7 @@ CFLAGS += $(USERCFLAGS) LIB := libzdtmtst.a -LIBSRC := datagen.c msg.c parseargs.c test.c streamutil.c lock.c ns.c tcp.c fs.c sysctl.c +LIBSRC := datagen.c msg.c parseargs.c test.c streamutil.c lock.c ns.c tcp.c unix.c fs.c sysctl.c LIBOBJ := $(LIBSRC:%.c=%.o) BIN := groups diff --git a/test/zdtm/lib/unix.c b/test/zdtm/lib/unix.c new file mode 100644 index 000000000..c36846cad --- /dev/null +++ b/test/zdtm/lib/unix.c @@ -0,0 +1,19 @@ +#include +#include +#include "zdtmtst.h" +#include "fs.h" + +int unix_fill_sock_name(struct sockaddr_un *name, char *relFilename) +{ + char *cwd; + + if (get_cwd_check_perm(&cwd)) { + pr_err("failed to get current working directory with valid permissions.\n"); + return -1; + } + + name->sun_family = AF_LOCAL; + ssprintf(name->sun_path, "%s/%s", cwd, relFilename); + return 0; +} + diff --git a/test/zdtm/lib/zdtmtst.h b/test/zdtm/lib/zdtmtst.h index 2cd4bdd1d..6eec26647 100644 --- a/test/zdtm/lib/zdtmtst.h +++ b/test/zdtm/lib/zdtmtst.h @@ -149,6 +149,9 @@ extern int tcp_init_server(int family, int *port); extern int tcp_accept_server(int sock); extern int tcp_init_client(int family, char *servIP, unsigned short servPort); +struct sockaddr_un; +extern int unix_fill_sock_name(struct sockaddr_un *name, char *relFilename); + struct zdtm_tcp_opts { bool reuseaddr; bool reuseport; diff --git a/test/zdtm/static/del_standalone_un.c b/test/zdtm/static/del_standalone_un.c index d8200068b..5426fc786 100644 --- a/test/zdtm/static/del_standalone_un.c +++ b/test/zdtm/static/del_standalone_un.c @@ -16,19 +16,6 @@ const char *test_author = "Tycho Andersen "; char *dirname; TEST_OPTION(dirname, string, "directory name", 1); -static int fill_sock_name(struct sockaddr_un *name, const char *filename) -{ - char *cwd; - - cwd = get_current_dir_name(); - if (strlen(filename) + strlen(cwd) + 1 >= sizeof(name->sun_path)) - return -1; - - name->sun_family = AF_LOCAL; - ssprintf(name->sun_path, "%s/%s", cwd, filename); - return 0; -} - static int bind_and_listen(struct sockaddr_un *addr) { int sk; @@ -71,10 +58,8 @@ int main(int argc, char **argv) goto out; } - if (fill_sock_name(&addr, filename) < 0) { - pr_err("filename \"%s\" is too long\n", filename); + if (unix_fill_sock_name(&addr, filename)) goto out; - } sk1 = bind_and_listen(&addr); if (sk1 < 0) diff --git a/test/zdtm/static/deleted_unix_sock.c b/test/zdtm/static/deleted_unix_sock.c index bcc33f3de..4d328e996 100644 --- a/test/zdtm/static/deleted_unix_sock.c +++ b/test/zdtm/static/deleted_unix_sock.c @@ -17,28 +17,13 @@ const char *test_author = "Roman Kagan "; char *filename; TEST_OPTION(filename, string, "file name", 1); -static int fill_sock_name(struct sockaddr_un *name, const char *filename) -{ - char *cwd; - - cwd = get_current_dir_name(); - if (strlen(filename) + strlen(cwd) + 1 >= sizeof(name->sun_path)) - return -1; - - name->sun_family = AF_LOCAL; - sprintf(name->sun_path, "%s/%s", cwd, filename); - return 0; -} - static int setup_srv_sock(void) { struct sockaddr_un name; int sock; - if (fill_sock_name(&name, filename) < 0) { - pr_perror("filename \"%s\" is too long", filename); + if (unix_fill_sock_name(&name, filename)) return -1; - } sock = socket(PF_LOCAL, SOCK_STREAM, 0); if (sock < 0) { @@ -67,7 +52,7 @@ static int setup_clnt_sock(void) struct sockaddr_un name; int sock; - if (fill_sock_name(&name, filename) < 0) + if (unix_fill_sock_name(&name, filename)) return -1; sock = socket(PF_LOCAL, SOCK_STREAM, 0); diff --git a/test/zdtm/static/sk-unix01.c b/test/zdtm/static/sk-unix01.c index 2bceef79a..0e9006a15 100644 --- a/test/zdtm/static/sk-unix01.c +++ b/test/zdtm/static/sk-unix01.c @@ -24,22 +24,6 @@ const char *test_author = "Cyrill Gorcunov "; char *dirname; TEST_OPTION(dirname, string, "directory name", 1); -static int fill_sock_name(struct sockaddr_un *name, const char *filename) -{ - char *cwd; - - cwd = get_current_dir_name(); - if (strlen(filename) + strlen(cwd) + 1 >= sizeof(name->sun_path)) { - pr_err("Name %s/%s is too long for socket\n", - cwd, filename); - return -1; - } - - name->sun_family = AF_LOCAL; - ssprintf(name->sun_path, "%s/%s", cwd, filename); - return 0; -} - static int sk_alloc_bind(int type, struct sockaddr_un *addr) { int sk; @@ -155,10 +139,9 @@ int main(int argc, char **argv) */ ssprintf(filename, "%s/%s", subdir_dg, "sk-dt"); - if (fill_sock_name(&addr, filename) < 0) { - pr_err("%s is too long for socket\n", filename); + if (unix_fill_sock_name(&addr, filename)) return 1; - } + unlink(addr.sun_path); sk_dgram[0] = sk_alloc_bind(SOCK_DGRAM, &addr); @@ -184,10 +167,9 @@ int main(int argc, char **argv) test_msg("sk-dt: alloc/connect/unlink %d %s\n", sk_dgram[3], addr.sun_path); ssprintf(filename, "%s/%s", dirname, "sole"); - if (fill_sock_name(&addr, filename) < 0) { - pr_err("%s is too long for socket\n", filename); + if (unix_fill_sock_name(&addr, filename)) return 1; - } + unlink(addr.sun_path); sk_dgram[4] = sk_alloc_bind(SOCK_DGRAM, &addr); @@ -237,7 +219,7 @@ int main(int argc, char **argv) sk_dgram_pair[0], sk_dgram_pair[1]); ssprintf(filename, "%s/%s", subdir_dg, "sk-dtp"); - if (fill_sock_name(&addr, filename) < 0) { + if (unix_fill_sock_name(&addr, filename)) { pr_err("%s is too long for socket\n", filename); return 1; } @@ -270,10 +252,9 @@ int main(int argc, char **argv) * - delete socket on fs */ ssprintf(filename, "%s/%s", subdir_st, "sk-st"); - if (fill_sock_name(&addr, filename) < 0) { - pr_err("%s is too long for socket\n", filename); + if (unix_fill_sock_name(&addr, filename)) return 1; - } + unlink(addr.sun_path); sk_st[0] = sk_alloc_bind(SOCK_STREAM, &addr); From 2b376168efd751856ebef192e764ebd0037e7174 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sat, 21 Mar 2020 21:58:55 +0300 Subject: [PATCH 41/48] pipe: restore pipe size even if a pipe is empty Without this patch, pipe size is restored only if a pipe has queued data. Reported-by: Mr Jenkins Signed-off-by: Andrei Vagin --- criu/pipes.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/criu/pipes.c b/criu/pipes.c index cb5da71de..d74329161 100644 --- a/criu/pipes.c +++ b/criu/pipes.c @@ -160,24 +160,24 @@ int restore_pipe_data(int img_type, int pfd, u32 id, struct pipe_data_rst **hash return 0; } - if (!pd->pde->bytes) - goto out; - - if (!pd->data) { - pr_err("Double data restore occurred on %#x\n", id); - return -1; - } - if (pd->pde->has_size) { pr_info("Restoring size %#x for %#x\n", pd->pde->size, pd->pde->pipe_id); ret = fcntl(pfd, F_SETPIPE_SZ, pd->pde->size); if (ret < 0) { pr_perror("Can't restore pipe size"); - goto err; + return -1; } } + if (!pd->pde->bytes) + return 0; + + if (!pd->data) { + pr_err("Double data restore occurred on %#x\n", id); + return -1; + } + iov.iov_base = pd->data; iov.iov_len = pd->pde->bytes; @@ -185,14 +185,13 @@ int restore_pipe_data(int img_type, int pfd, u32 id, struct pipe_data_rst **hash ret = vmsplice(pfd, &iov, 1, SPLICE_F_GIFT | SPLICE_F_NONBLOCK); if (ret < 0) { pr_perror("%#x: Error splicing data", id); - goto err; + return -1; } if (ret == 0 || ret > iov.iov_len /* sanity */) { pr_err("%#x: Wanted to restore %zu bytes, but got %d\n", id, iov.iov_len, ret); - ret = -1; - goto err; + return -1; } iov.iov_base += ret; @@ -211,10 +210,7 @@ int restore_pipe_data(int img_type, int pfd, u32 id, struct pipe_data_rst **hash munmap(pd->data, pd->pde->bytes); pd->data = NULL; -out: - ret = 0; -err: - return ret; + return 0; } static int userns_reopen(void *_arg, int fd, pid_t pid) From 1ad209b9c2b780fe2d5b043c3ffe29634629252c Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sat, 21 Mar 2020 22:08:38 +0300 Subject: [PATCH 42/48] test/pipe03: check that pipe size is restored Create two pipes with and without queued data. Signed-off-by: Andrei Vagin --- test/zdtm/static/pipe03.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/test/zdtm/static/pipe03.c b/test/zdtm/static/pipe03.c index a8721e934..d649007b7 100644 --- a/test/zdtm/static/pipe03.c +++ b/test/zdtm/static/pipe03.c @@ -13,27 +13,28 @@ const char *test_author = "Andrei Vagin "; int main(int argc, char **argv) { - int p[2], i; + int p[2][2], i; uint8_t buf[BUF_SIZE]; uint32_t crc; test_init(argc, argv); - if (pipe2(p, O_NONBLOCK)) { - pr_perror("pipe"); - return 1; - } - - if (fcntl(p[1], F_SETPIPE_SZ, DATA_SIZE) == -1) { - pr_perror("Unable to change a pipe size"); - return 1; + for (i = 0; i < 2; i++) { + if (pipe2(p[i], O_NONBLOCK)) { + pr_perror("pipe"); + return 1; + } + if (fcntl(p[i][1], F_SETPIPE_SZ, DATA_SIZE) == -1) { + pr_perror("Unable to change a pipe size"); + return 1; + } } crc = ~0; datagen(buf, BUF_SIZE, &crc); for (i = 0; i < DATA_SIZE / BUF_SIZE; i++) { - if (write(p[1], buf, BUF_SIZE) != BUF_SIZE) { + if (write(p[0][1], buf, BUF_SIZE) != BUF_SIZE) { pr_perror("write"); return 1; } @@ -43,12 +44,26 @@ int main(int argc, char **argv) test_waitsig(); for (i = 0; i < DATA_SIZE / BUF_SIZE; i++) { - if (read(p[0], buf, BUF_SIZE) != BUF_SIZE) { + if (read(p[0][0], buf, BUF_SIZE) != BUF_SIZE) { pr_perror("read"); return 1; } } + for (i = 0; i < 2; i++) { + int size; + + size = fcntl(p[i][1], F_GETPIPE_SZ); + if (size < 0) { + pr_perror("Unable to get a pipe size"); + return 1; + } + if (size != DATA_SIZE) { + fail("%d: size %d expected %d", i, size, DATA_SIZE); + return 1; + } + } + pass(); return 0; } From 5f28b692a0c972ddefb1ca4d1d0ef003dec4f617 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Sat, 21 Mar 2020 22:11:28 +0300 Subject: [PATCH 43/48] test/fifo_loop: change sizes of all fifo-s to fit a test buffer This test doesn't expect that the write operation will block. Signed-off-by: Andrei Vagin --- test/zdtm/transition/fifo_loop.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/zdtm/transition/fifo_loop.c b/test/zdtm/transition/fifo_loop.c index b028c2fd5..b06592586 100644 --- a/test/zdtm/transition/fifo_loop.c +++ b/test/zdtm/transition/fifo_loop.c @@ -84,6 +84,14 @@ int main(int argc, char **argv) ret = errno; return ret; } + + pipe_size = fcntl(writefd, F_SETPIPE_SZ, sizeof(buf)); + if (pipe_size != sizeof(buf)) { + pr_perror("fcntl(writefd, F_SETPIPE_SZ) -> %d", pipe_size); + kill(0, SIGKILL); + exit(1); + } + signal(SIGPIPE, SIG_IGN); if (pipe_in2out(readfd, writefd, buf, sizeof(buf)) < 0) /* pass errno as exit code to the parent */ @@ -107,7 +115,7 @@ int main(int argc, char **argv) pipe_size = fcntl(writefd, F_SETPIPE_SZ, sizeof(buf)); if (pipe_size != sizeof(buf)) { - pr_perror("fcntl(writefd, F_GETPIPE_SZ) -> %d", pipe_size); + pr_perror("fcntl(writefd, F_SETPIPE_SZ) -> %d", pipe_size); kill(0, SIGKILL); exit(1); } From 1ad8657ddb3d383874cc07cd3cf456cac7977db6 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 24 Mar 2020 19:31:42 +0300 Subject: [PATCH 44/48] config/nftables: include string.h for strlen Fixes: 9433b7b9db3e ("make: use cflags/ldflags for config.h detection mechanism") Signed-off-by: Andrei Vagin --- scripts/feature-tests.mak | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/feature-tests.mak b/scripts/feature-tests.mak index 21b390092..8df20afb7 100644 --- a/scripts/feature-tests.mak +++ b/scripts/feature-tests.mak @@ -152,6 +152,8 @@ endef define FEATURE_TEST_NFTABLES_LIB_API_0 +#include + #include int main(int argc, char **argv) From cc362b432e2d2e3ec68628fb33b117fe3e89f9c2 Mon Sep 17 00:00:00 2001 From: Pavel Tikhomirov Date: Fri, 20 Mar 2020 17:34:57 +0300 Subject: [PATCH 45/48] namespaces: fix error handling in dump_user_ns Fix n_xid_map leaks on error path and remove useless exit_code. Fixes: 6e1726f8 ("userns: set uid and gid before entering into userns") Signed-off-by: Pavel Tikhomirov --- criu/namespaces.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/criu/namespaces.c b/criu/namespaces.c index 21266df7c..2db805b2f 100644 --- a/criu/namespaces.c +++ b/criu/namespaces.c @@ -938,9 +938,9 @@ static int check_user_ns(int pid) int dump_user_ns(pid_t pid, int ns_id) { - int ret, exit_code = -1; UsernsEntry *e = &userns_entry; struct cr_img *img; + int ret; ret = parse_id_map(pid, "uid_map", &e->uid_map); if (ret < 0) @@ -953,7 +953,7 @@ int dump_user_ns(pid_t pid, int ns_id) e->n_gid_map = ret; if (check_user_ns(pid)) - return -1; + goto err; img = open_image(CR_FD_USERNS, O_DUMP, ns_id); if (!img) @@ -973,7 +973,7 @@ err: xfree(e->gid_map[0]); xfree(e->gid_map); } - return exit_code; + return -1; } void free_userns_maps(void) From 967797a8676c8b3b7cd8954892b113c6765af25a Mon Sep 17 00:00:00 2001 From: Byeonggon Lee Date: Sun, 15 Mar 2020 16:32:15 +0900 Subject: [PATCH 46/48] Add build directory to gitignore After running make install, build directory is generated but not ignored in gitignore. So this commit add build directory to gitignore. Signed-off-by: Byeonggon Lee --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index c231104af..23cd703be 100644 --- a/.gitignore +++ b/.gitignore @@ -42,3 +42,4 @@ lib/.crit-setup.files compel/include/asm include/common/asm include/common/config.h +build/ From e3a5d0975240f9e9b6b6d7a096af6b4bbad36737 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Mon, 23 Mar 2020 07:37:00 +0300 Subject: [PATCH 47/48] memfd: save all memfd inodes in one image Per-object image is acceptable if we expect to have 1-3 objects per-container. If we expect to have more objects, it is better to save them all into one image. There are a number of reasons for this: * We need fewer system calls to read all objects from one image. * It is faster to save or move one image. Signed-off-by: Andrei Vagin --- criu/cr-restore.c | 3 ++ criu/image-desc.c | 2 +- criu/include/image-desc.h | 2 +- criu/include/memfd.h | 2 + criu/memfd.c | 90 +++++++++++++++++---------------------- images/memfd.proto | 1 + 6 files changed, 46 insertions(+), 54 deletions(-) diff --git a/criu/cr-restore.c b/criu/cr-restore.c index 41146d4ad..1d3092f2f 100644 --- a/criu/cr-restore.c +++ b/criu/cr-restore.c @@ -233,6 +233,9 @@ static int restore_finish_ns_stage(int from, int to) static int crtools_prepare_shared(void) { + if (prepare_memfd_inodes()) + return -1; + if (prepare_files()) return -1; diff --git a/criu/image-desc.c b/criu/image-desc.c index b538a76ea..ac627a829 100644 --- a/criu/image-desc.c +++ b/criu/image-desc.c @@ -66,7 +66,7 @@ struct cr_fd_desc_tmpl imgset_template[CR_FD_MAX] = { FD_ENTRY(FS, "fs-%u"), FD_ENTRY(REMAP_FPATH, "remap-fpath"), FD_ENTRY_F(GHOST_FILE, "ghost-file-%x", O_NOBUF), - FD_ENTRY_F(MEMFD_INODE, "memfd-%u", O_NOBUF), + FD_ENTRY_F(MEMFD_INODE, "memfd", O_NOBUF), FD_ENTRY(TCP_STREAM, "tcp-stream-%x"), FD_ENTRY(MNTS, "mountpoints-%u"), FD_ENTRY(NETDEV, "netdev-%u"), diff --git a/criu/include/image-desc.h b/criu/include/image-desc.h index 9ca9643a1..ce6ef1529 100644 --- a/criu/include/image-desc.h +++ b/criu/include/image-desc.h @@ -65,6 +65,7 @@ enum { CR_FD_CGROUP, CR_FD_FILE_LOCKS, CR_FD_SECCOMP, + CR_FD_MEMFD_INODE, _CR_FD_GLOB_TO, CR_FD_TMPFS_IMG, @@ -107,7 +108,6 @@ enum { CR_FD_PIPES, CR_FD_TTY_FILES, CR_FD_MEMFD_FILE, - CR_FD_MEMFD_INODE, CR_FD_AUTOFS, diff --git a/criu/include/memfd.h b/criu/include/memfd.h index 4189766fd..3074a5c0f 100644 --- a/criu/include/memfd.h +++ b/criu/include/memfd.h @@ -17,6 +17,8 @@ extern struct collect_image_info memfd_cinfo; extern struct file_desc *collect_memfd(u32 id); extern int apply_memfd_seals(void); +extern int prepare_memfd_inodes(void); + #ifdef CONFIG_HAS_MEMFD_CREATE # include #else diff --git a/criu/memfd.c b/criu/memfd.c index bca6900cb..2158e925b 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -43,9 +43,10 @@ struct memfd_inode { }; /* Only for restore */ struct { - mutex_t lock; - int fdstore_id; - unsigned int pending_seals; + mutex_t lock; + int fdstore_id; + unsigned int pending_seals; + MemfdInodeEntry *mie; }; }; }; @@ -71,9 +72,8 @@ int is_memfd(dev_t dev) static int dump_memfd_inode(int fd, struct memfd_inode *inode, const char *name, const struct stat *st) { - int ret = -1; - struct cr_img *img = NULL; MemfdInodeEntry mie = MEMFD_INODE_ENTRY__INIT; + int ret = -1; u32 shmid; /* @@ -90,10 +90,7 @@ static int dump_memfd_inode(int fd, struct memfd_inode *inode, if (dump_one_memfd_shmem(fd, shmid, st->st_size) < 0) goto out; - img = open_image(CR_FD_MEMFD_INODE, O_DUMP, inode->id); - if (!img) - goto out; - + mie.inode_id = inode->id; mie.uid = userns_uid(st->st_uid); mie.gid = userns_gid(st->st_gid); mie.name = (char *)name; @@ -104,14 +101,12 @@ static int dump_memfd_inode(int fd, struct memfd_inode *inode, if (mie.seals == -1) goto out; - if (pb_write_one(img, &mie, PB_MEMFD_INODE)) + if (pb_write_one(img_from_set(glob_imgset, CR_FD_MEMFD_INODE), &mie, PB_MEMFD_INODE)) goto out; ret = 0; out: - if (img) - close_image(img); return ret; } @@ -212,8 +207,6 @@ struct memfd_info { struct memfd_inode *inode; }; -static int memfd_open_inode(struct memfd_inode *inode); - static struct memfd_inode *memfd_alloc_inode(int id) { struct memfd_inode *inode; @@ -222,35 +215,47 @@ static struct memfd_inode *memfd_alloc_inode(int id) if (inode->id == id) return inode; - inode = shmalloc(sizeof(*inode)); - if (!inode) - return NULL; + pr_err("Unable to find the %d memfd inode\n", id); + return NULL; +} - inode->id = id; +static int collect_one_memfd_inode(void *o, ProtobufCMessage *base, struct cr_img *i) +{ + MemfdInodeEntry *mie = pb_msg(base, MemfdInodeEntry); + struct memfd_inode *inode = o; + + inode->mie = mie; + inode->id = mie->inode_id; mutex_init(&inode->lock); inode->fdstore_id = -1; inode->pending_seals = 0; list_add_tail(&inode->list, &memfd_inodes); - return inode; + + return 0; +} + +static struct collect_image_info memfd_inode_cinfo = { + .fd_type = CR_FD_MEMFD_INODE, + .pb_type = PB_MEMFD_INODE, + .priv_size = sizeof(struct memfd_inode), + .collect = collect_one_memfd_inode, + .flags = COLLECT_SHARED | COLLECT_NOFREE, +}; + +int prepare_memfd_inodes(void) +{ + return collect_image(&memfd_inode_cinfo); } -extern int restore_memfd_shm(int fd, u64 id, u64 size); static int memfd_open_inode_nocache(struct memfd_inode *inode) { MemfdInodeEntry *mie = NULL; - struct cr_img *img = NULL; int fd = -1; int ret = -1; int flags; - img = open_image(CR_FD_MEMFD_INODE, O_RSTR, inode->id); - if (!img) - goto out; - - if (pb_read_one(img, &mie, PB_MEMFD_INODE) < 0) - goto out; - + mie = inode->mie; if (mie->seals == F_SEAL_SEAL) { inode->pending_seals = 0; flags = 0; @@ -285,10 +290,6 @@ static int memfd_open_inode_nocache(struct memfd_inode *inode) out: if (fd != -1) close(fd); - if (img) - close_image(img); - if (mie) - memfd_inode_entry__free_unpacked(mie, NULL); return ret; } @@ -373,33 +374,17 @@ static int memfd_open_fe_fd(struct file_desc *fd, int *new_fd) static char *memfd_d_name(struct file_desc *d, char *buf, size_t s) { MemfdInodeEntry *mie = NULL; - struct cr_img *img = NULL; struct memfd_info *mfi; - char *ret = NULL; mfi = container_of(d, struct memfd_info, d); - img = open_image(CR_FD_MEMFD_INODE, O_RSTR, mfi->inode->id); - if (!img) - goto out; - - if (pb_read_one(img, &mie, PB_MEMFD_INODE) < 0) - goto out; - + mie = mfi->inode->mie; if (snprintf(buf, s, "%s%s", MEMFD_PREFIX, mie->name) >= s) { pr_err("Buffer too small for memfd name %s\n", mie->name); - goto out; + return NULL; } - ret = buf; - -out: - if (img) - close_image(img); - if (mie) - memfd_inode_entry__free_unpacked(mie, NULL); - - return ret; + return buf; } static struct file_desc_ops memfd_desc_ops = { @@ -427,7 +412,8 @@ struct collect_image_info memfd_cinfo = { .collect = collect_one_memfd, }; -struct file_desc *collect_memfd(u32 id) { +struct file_desc *collect_memfd(u32 id) +{ struct file_desc *fdesc; fdesc = find_file_desc_raw(FD_TYPES__MEMFD, id); diff --git a/images/memfd.proto b/images/memfd.proto index 546ffc2ab..ad5373d10 100644 --- a/images/memfd.proto +++ b/images/memfd.proto @@ -18,4 +18,5 @@ message memfd_inode_entry { required uint64 size = 4; required uint32 shmid = 5; required uint32 seals = 6 [(criu).flags = "seals.flags"]; + required uint64 inode_id = 7; }; From 8c36865c84666c73424b7a0fdb9f460557465ff2 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Wed, 25 Mar 2020 20:14:24 +0300 Subject: [PATCH 48/48] memfd: split the struct memfd_inode The struct memfd_inode has a union for dump and restore parts. The only common parts are the list_head node, and the inode id. Suggested-by: Nicolas Viennot Signed-off-by: Andrei Vagin --- criu/memfd.c | 58 ++++++++++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/criu/memfd.c b/criu/memfd.c index 2158e925b..4419b4bf5 100644 --- a/criu/memfd.c +++ b/criu/memfd.c @@ -32,23 +32,19 @@ /* Linux 5.1+ */ #define F_SEAL_FUTURE_WRITE 0x0010 /* prevent future writes while mapped */ -struct memfd_inode { +struct memfd_dump_inode { struct list_head list; u32 id; - union { - /* Only for dump */ - struct { - u32 dev; - u32 ino; - }; - /* Only for restore */ - struct { - mutex_t lock; - int fdstore_id; - unsigned int pending_seals; - MemfdInodeEntry *mie; - }; - }; + u32 dev; + u32 ino; +}; + +struct memfd_restore_inode { + struct list_head list; + mutex_t lock; + int fdstore_id; + unsigned int pending_seals; + MemfdInodeEntry *mie; }; static LIST_HEAD(memfd_inodes); @@ -69,7 +65,7 @@ int is_memfd(dev_t dev) return dev == kdat.shmem_dev; } -static int dump_memfd_inode(int fd, struct memfd_inode *inode, +static int dump_memfd_inode(int fd, struct memfd_dump_inode *inode, const char *name, const struct stat *st) { MemfdInodeEntry mie = MEMFD_INODE_ENTRY__INIT; @@ -110,9 +106,10 @@ out: return ret; } -static struct memfd_inode *dump_unique_memfd_inode(int lfd, const char *name, const struct stat *st) +static struct memfd_dump_inode * +dump_unique_memfd_inode(int lfd, const char *name, const struct stat *st) { - struct memfd_inode *inode; + struct memfd_dump_inode *inode; int fd; list_for_each_entry(inode, &memfd_inodes, list) @@ -149,7 +146,7 @@ static int dump_one_memfd(int lfd, u32 id, const struct fd_parms *p) { MemfdFileEntry mfe = MEMFD_FILE_ENTRY__INIT; FileEntry fe = FILE_ENTRY__INIT; - struct memfd_inode *inode; + struct memfd_dump_inode *inode; struct fd_link _link, *link; const char *name; @@ -202,17 +199,17 @@ const struct fdtype_ops memfd_dump_ops = { */ struct memfd_info { - MemfdFileEntry *mfe; - struct file_desc d; - struct memfd_inode *inode; + MemfdFileEntry *mfe; + struct file_desc d; + struct memfd_restore_inode *inode; }; -static struct memfd_inode *memfd_alloc_inode(int id) +static struct memfd_restore_inode *memfd_alloc_inode(int id) { - struct memfd_inode *inode; + struct memfd_restore_inode *inode; list_for_each_entry(inode, &memfd_inodes, list) - if (inode->id == id) + if (inode->mie->inode_id == id) return inode; pr_err("Unable to find the %d memfd inode\n", id); @@ -222,10 +219,9 @@ static struct memfd_inode *memfd_alloc_inode(int id) static int collect_one_memfd_inode(void *o, ProtobufCMessage *base, struct cr_img *i) { MemfdInodeEntry *mie = pb_msg(base, MemfdInodeEntry); - struct memfd_inode *inode = o; + struct memfd_restore_inode *inode = o; inode->mie = mie; - inode->id = mie->inode_id; mutex_init(&inode->lock); inode->fdstore_id = -1; inode->pending_seals = 0; @@ -238,7 +234,7 @@ static int collect_one_memfd_inode(void *o, ProtobufCMessage *base, struct cr_im static struct collect_image_info memfd_inode_cinfo = { .fd_type = CR_FD_MEMFD_INODE, .pb_type = PB_MEMFD_INODE, - .priv_size = sizeof(struct memfd_inode), + .priv_size = sizeof(struct memfd_restore_inode), .collect = collect_one_memfd_inode, .flags = COLLECT_SHARED | COLLECT_NOFREE, }; @@ -248,7 +244,7 @@ int prepare_memfd_inodes(void) return collect_image(&memfd_inode_cinfo); } -static int memfd_open_inode_nocache(struct memfd_inode *inode) +static int memfd_open_inode_nocache(struct memfd_restore_inode *inode) { MemfdInodeEntry *mie = NULL; int fd = -1; @@ -293,7 +289,7 @@ out: return ret; } -static int memfd_open_inode(struct memfd_inode *inode) +static int memfd_open_inode(struct memfd_restore_inode *inode) { int fd; @@ -433,7 +429,7 @@ int apply_memfd_seals(void) */ int ret, fd; - struct memfd_inode *inode; + struct memfd_restore_inode *inode; list_for_each_entry(inode, &memfd_inodes, list) { if (!inode->pending_seals)