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

[Jamie Liu]

Flawed, fix to follow very shortly.

On Thu, Apr 3, 2014 at 3:04 PM, Jamie Liu <jamieliu at google.com> 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        | 5 +++++
>  proc_parse.c       | 7 +++++++
>  protobuf/vma.proto | 3 +++
>  4 files changed, 16 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..fd9a68c 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,9 @@ 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;
>         return open_path(vma->fd, do_open_reg_noseek, NULL);
>  }
>
> diff --git a/proc_parse.c b/proc_parse.c
> index d628e81..32f25e7 100644
> --- a/proc_parse.c
> +++ b/proc_parse.c
> @@ -100,9 +100,16 @@ static int parse_vmflags(char *buf, struct vma_area *vma_area)
>         if (!tok)
>                 return 0;
>
> +       vma_area->e->fdflags = O_RDONLY;
> +       vma_area->e->has_fdflags = true;
> +
>  #define _vmflag_match(_t, _s) (_t[0] == _s[0] && _t[1] == _s[1])
>
>         do {
> +               /* open() block */
> +               if (_vmflag_match(tok, "mw"))
> +                       vma_area->e->fdflags = O_RDWR;
> +
>                 /* mmap() block */
>                 if (_vmflag_match(tok, "gd"))
>                         vma_area->e->flags |= MAP_GROWSDOWN;
> 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
>