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

[Andrew Vagin]

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?

> +		return -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
>