[CRIU] [PATCH v4] criu: fix filemap open permissions

Andrew Vagin avagin at parallels.com
Fri Apr 4 12:24:46 PDT 2014


On Fri, Apr 04, 2014 at 10:27:42AM -0700, Jamie Liu wrote:
> No problem, I only didn't respond because I was asleep. :) I'll send a
> compatibility fix today.

Looks like here are not only compatibility problems:
$ bash test/zdtm.sh static/maps00
...
23:19:39.723: 29890: map: ptr 0x7fa5e426d000 flag        2 prot        0
23:19:39.723: 29890: map: ptr 0x7fa5e426b000 flag        1 prot        0
23:19:39.723: 29890: map: ptr 0x7fa5e4269000 flag       22 prot        0
23:19:39.724: 29890: map: ptr 0x7fa5e4267000 flag       21 prot        0
23:19:39.724: 29890: map: ptr 0x7fa5e4265000 flag        2 prot        1
23:19:39.724: 29890: map: ptr 0x7fa5e4263000 flag        1 prot        1
23:19:39.725: 29890: map: ptr 0x7fa5e425d000 flag       22 prot        1
23:19:39.725: 29890: map: ptr 0x7fa5e425b000 flag       21 prot        1
23:19:39.725: 29890: map: ptr 0x7fa5e4259000 flag        2 prot        3
23:19:39.725: 29890: map: ptr 0x7fa5e4257000 flag        1 prot        3
23:19:39.726: 29890: map: ptr 0x7fa5e4255000 flag       22 prot        3
23:19:39.726: 29890: map: ptr 0x7fa5e4253000 flag       21 prot        3
23:19:39.726: 29890: map: ptr 0x7fa5e4251000 flag        2 prot        7
23:19:39.727: 29890: map: ptr 0x7fa5e424f000 flag        1 prot        7
23:19:39.727: 29890: map: ptr 0x7fa5e424d000 flag       22 prot        7
23:19:39.727: 29890: map: ptr 0x7fa5e424b000 flag       21 prot        7
23:19:40.013: 29890: ERR: maps00.c:181: failed to write maps00.test-00: Bad file descriptor
 (errno = 9 (Bad file descriptor))
------------------------------------- END -------------------------------------
================================= ERROR OVER =================================

It's because you rewrite flags of a file, which is associated with a
file descriptor, so this fd is restored with wrong flags.

        if (vma->e->has_fdflags) {
                rfi->rfe->flags = vma->e->fdflags;
        } else {
                pr_err("vma %#"PRIx64" missing fdflags", vma->e->start);
                return -1;
        }

> 
> On Fri, Apr 4, 2014 at 10:25 AM, Andrew Vagin <avagin at parallels.com> wrote:
> > On Fri, Apr 04, 2014 at 12:36:39PM +0400, Andrew Vagin wrote:
> >> On Fri, Apr 04, 2014 at 01:04:08AM -0700, Jamie Liu wrote:
> >> > An mmaped file is opened O_RDONLY or O_RDWR depending on the permissions
> >> > on the first vma dump_task_mm() encounters mapping that file. This
> >> > causes two problems:
> >> >
> >> > 1. If a file has multiple MAP_SHARED mappings, some of which are
> >> >    read-only and some of which are read-write, and the first encountered
> >> >    mapping happens to be read-only, the file will be opened O_RDONLY
> >> >    during restore, and mmap(PROT_WRITE) will fail with EACCES, causing
> >> >    the restore to fail.
> >> >
> >> > 2. If a file is opened read-write and mapped read-only, it will be
> >> >    opened O_RDONLY during restore, so restore will succeed, but
> >> >    mprotect(PROT_WRITE) on the read-only mapping after restore will
> >> >    fail.
> >> >
> >> > To fix both of these, record open flags per-vma based on the presence of
> >> > VM_MAYWRITE in smaps.
> >> >
> >> > Signed-off-by: Jamie Liu <jamieliu at google.com>
> >> > ---
> >> >  cr-dump.c          |  6 +-----
> >> >  files-reg.c        |  9 +++++++++
> >> >  proc_parse.c       | 14 ++++++++++++++
> >> >  protobuf/vma.proto |  3 +++
> >> >  4 files changed, 27 insertions(+), 5 deletions(-)
> >> >
> >> > diff --git a/cr-dump.c b/cr-dump.c
> >> > index 0111115..258ba4b 100644
> >> > --- a/cr-dump.c
> >> > +++ b/cr-dump.c
> >> > @@ -15,7 +15,6 @@
> >> >  #include <sys/vfs.h>
> >> >
> >> >  #include <sys/sendfile.h>
> >> > -#include <sys/mman.h>
> >> >
> >> >  #include <sched.h>
> >> >  #include <sys/resource.h>
> >> > @@ -347,10 +346,7 @@ static int dump_filemap(pid_t pid, struct vma_area *vma_area,
> >> >     BUG_ON(!vma_area->st);
> >> >     p.stat = *vma_area->st;
> >> >
> >> > -   if ((vma->prot & PROT_WRITE) && vma_entry_is(vma, VMA_FILE_SHARED))
> >> > -           p.flags = O_RDWR;
> >> > -   else
> >> > -           p.flags = O_RDONLY;
> >> > +   /* Flags will be set during restore in get_filemap_fd() */
> >> >
> >> >     if (fd_id_generate_special(&p.stat, &id))
> >> >             ret = dump_one_reg_file(vma_area->vm_file_fd, id, &p);
> >> > diff --git a/files-reg.c b/files-reg.c
> >> > index eccd04e..57312f9 100644
> >> > --- a/files-reg.c
> >> > +++ b/files-reg.c
> >> > @@ -708,6 +708,8 @@ int open_reg_by_id(u32 id)
> >> >
> >> >  int get_filemap_fd(struct vma_area *vma)
> >> >  {
> >> > +   struct reg_file_info *rfi;
> >> > +
> >> >     /*
> >> >      * Thevma->fd should have been assigned in collect_filemap
> >> >      *
> >> > @@ -715,6 +717,13 @@ int get_filemap_fd(struct vma_area *vma)
> >> >      */
> >> >
> >> >     BUG_ON(vma->fd == NULL);
> >> > +   rfi = container_of(vma->fd, struct reg_file_info, d);
> >> > +   if (vma->e->has_fdflags)
> >> > +           rfi->rfe->flags = vma->e->fdflags;
> >> > +   else {
> >> > +           pr_err("vma %#lx missing fdflags", vma->e->start);
> >>
> >> Are you going to break backward compatibility?
> >
> > Sorry for this short and rude comment.
> > I wanted to say that criu should able to restore processes from old images.
> >
> > You can use zdtm to check compatibility:
> >
> > [root at avagin-fc19-cr crtools]# bash  test/zdtm.sh -b v1.2 static/maps00
> > make[1]: `built-in.o' is up to date.
> > make[1]: `lib/libcriu.so' is up to date.
> > make[2]: `arch/x86/syscalls.built-in.o' is up to date.
> > make[2]: `arch/x86/crtools.built-in.o' is up to date.
> > make[2]: `pie/util-fd.o' is up to date.
> > make[2]: `pie/util.o' is up to date.
> > make[2]: `arch/x86/vdso-pie.o' is up to date.
> > make[2]: `built-in.o' is up to date.
> > ================================= CRIU CHECK =================================
> > Looks good.
> > Execute zdtm/live/static/maps00
> > ./maps00 --pidfile=maps00.pid --outfile=maps00.out --filename=maps00.test
> > Dump 26910
> > Restore
> > Test: zdtm/live/static/maps00, Result: FAIL
> > ==================================== ERROR ====================================
> > Test: zdtm/live/static/maps00, Namespace:
> > Dump log   : /root/git/crtools/test/dump/maps00/26910/1/dump.log
> > --------------------------------- grep Error ---------------------------------
> > (00.001400) Error (image.c:197): Unable to open irmap-cache: No such file or directory
> > ------------------------------------- END -------------------------------------
> > Restore log: /root/git/crtools/test/dump/maps00/26910/1/restore.log
> > --------------------------------- grep Error ---------------------------------
> > (00.021803)  26910: Error (files-reg.c:724): vma 0x400000 missing fdflags(00.021837)  26910: Error (cr-restore.c:231): Can't fixup VMA's fd
> > (00.022950) Error (cr-restore.c:1036): 26910 exited, status=1
> > (00.023051) Error (cr-restore.c:1579): Restoring FAILED.
> > ------------------------------------- END -------------------------------------
> > ================================= ERROR OVER =================================
> >
> > If you have time, could you fix this issue?
> >
> > Thanks.
> >
> >> > +           rturn -1;
> >> > +   }
> >> >     return open_path(vma->fd, do_open_reg_noseek, NULL);
> >> >  }
> >> >
> >> > diff --git a/proc_parse.c b/proc_parse.c
> >> > index d628e81..b8dce14 100644
> >> > --- a/proc_parse.c
> >> > +++ b/proc_parse.c
> >> > @@ -92,6 +92,8 @@ static bool is_vma_range_fmt(char *line)
> >> >  static int parse_vmflags(char *buf, struct vma_area *vma_area)
> >> >  {
> >> >     char *tok;
> >> > +   bool shared = false;
> >> > +   bool maywrite = false;
> >> >
> >> >     if (!buf[0])
> >> >             return 0;
> >> > @@ -103,6 +105,12 @@ static int parse_vmflags(char *buf, struct vma_area *vma_area)
> >> >  #define _vmflag_match(_t, _s) (_t[0] == _s[0] && _t[1] == _s[1])
> >> >
> >> >     do {
> >> > +           /* open() block */
> >> > +           if (_vmflag_match(tok, "sh"))
> >> > +                   shared = true;
> >> > +           else if (_vmflag_match(tok, "mw"))
> >> > +                   maywrite = true;
> >> > +
> >> >             /* mmap() block */
> >> >             if (_vmflag_match(tok, "gd"))
> >> >                     vma_area->e->flags |= MAP_GROWSDOWN;
> >> > @@ -136,6 +144,12 @@ static int parse_vmflags(char *buf, struct vma_area *vma_area)
> >> >
> >> >  #undef _vmflag_match
> >> >
> >> > +   if (shared && maywrite)
> >> > +           vma_area->e->fdflags = O_RDWR;
> >> > +   else
> >> > +           vma_area->e->fdflags = O_RDONLY;
> >> > +   vma_area->e->has_fdflags = true;
> >> > +
> >> >     if (vma_area->e->madv)
> >> >             vma_area->e->has_madv = true;
> >> >
> >> > diff --git a/protobuf/vma.proto b/protobuf/vma.proto
> >> > index 8100965..af88807 100644
> >> > --- a/protobuf/vma.proto
> >> > +++ b/protobuf/vma.proto
> >> > @@ -15,4 +15,7 @@ message vma_entry {
> >> >
> >> >     /* madvise flags bitmap */
> >> >     optional uint64         madv    = 9;
> >> > +
> >> > +   /* file status flags */
> >> > +   optional uint32         fdflags = 10;
> >> >  }
> >> > --
> >> > 1.9.1.423.g4596e3a
> >> >
> >> _______________________________________________
> >> CRIU mailing list
> >> CRIU at openvz.org
> >> https://lists.openvz.org/mailman/listinfo/criu


More information about the CRIU mailing list