[CRIU] [PATCH v4] criu: fix filemap open permissions
Jamie Liu
jamieliu at google.com
Fri Apr 4 10:27:42 PDT 2014
No problem, I only didn't respond because I was asleep. :) I'll send a
compatibility fix today.
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