[CRIU] [PATCH] proc_parse: fix vma file open mode recognition

[Pavel Emelyanov]

On 06/30/2016 07:24 PM, Stanislav Kinsburskiy wrote:
> The correct place to get fd open flags for file mappings is
> /proc/<pid>/map_files.
> An attempt tp speculate on "shared" and "maywrite" bits doesn't garantee,
> that file will be opened with correct permissions on restore.
> Here is an example:
> 
> Process mapping (read/write):
> 
> # cat /proc/481943/maps | grep 7f7108077000-7f7108078000
> 7f7108077000-7f7108078000 rw-p 00001000 00:35 7 <snip>
> 
> 1) Before suspend:
> 
> # ls -l /proc/481427/map_files/7f7108077000-7f7108078000
> lrw------- <snip> /proc/481427/map_files/7f7108077000-7f7108078000 -> <snip>
> 
> 2) After restore:
> 
> # ls -l /proc/481943/map_files/7f7108077000-7f7108078000
> lr-------- <snip> /proc/481943/map_files/7f7108077000-7f7108078000 -> <snip>
> 
> Write bit is lost.
> 
> This patch set vma->e->fdflags as /proc/<pid>/map_files/<vma> open mode.
> 
> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
> ---
>  criu/proc_parse.c |   51 ++++++++++++++++++++++++++++++++++-----------------
>  criu/util.c       |    3 ++-
>  2 files changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
> index 5210226..8e36916 100644
> --- a/criu/proc_parse.c
> +++ b/criu/proc_parse.c
> @@ -82,8 +82,6 @@ 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;
> @@ -95,12 +93,6 @@ 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;
> @@ -144,12 +136,6 @@ 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;
>  
> @@ -175,12 +161,46 @@ static inline int vfi_equal(struct vma_file_info *a, struct vma_file_info *b)
>  			(a->dev_min ^ b->dev_min)) == 0;
>  }
>  
> +static int vma_get_mapfile_flags(struct vma_area *vma, DIR *mfd, char *path)
> +{
> +	struct stat stat;
> +
> +	if (fstatat(dirfd(mfd), path, &stat, AT_SYMLINK_NOFOLLOW) < 0) {
> +		if (errno == ENOENT) {
> +			/* Just mapping w/o map_files link */
> +			return 0;
> +		}
> +		pr_perror("Failed fstatat on map %"PRIx64"", vma->e->start);
> +		return -1;
> +	}
> +
> +	switch(stat.st_mode & 0600) {
> +		case 0200:
> +			vma->e->fdflags = O_WRONLY;
> +			break;
> +		case 0400:
> +			vma->e->fdflags = O_RDONLY;
> +			break;
> +		case 0600:
> +			vma->e->fdflags = O_RDWR;
> +			break;
> +	}
> +	vma->e->has_fdflags = true;
> +	return 0;
> +}
> +
>  static int vma_get_mapfile(char *fname, struct vma_area *vma, DIR *mfd,
>  		struct vma_file_info *vfi, struct vma_file_info *prev_vfi)
>  {
>  	char path[32];
>  	int flags;
>  
> +	/* Figure out if it's file mapping */
> +	snprintf(path, sizeof(path), "%"PRIx64"-%"PRIx64, vma->e->start, vma->e->end);
> +
> +	if (vma_get_mapfile_flags(vma, mfd, path))
> +		return -1;

Can we re-use the vmfd borrowing logic for fdflags too?

> +
>  	if (prev_vfi->vma && vfi_equal(vfi, prev_vfi)) {
>  		struct vma_area *prev = prev_vfi->vma;
>  
> @@ -209,9 +229,6 @@ static int vma_get_mapfile(char *fname, struct vma_area *vma, DIR *mfd,
>  		return 0;
>  	}
>  
> -	/* Figure out if it's file mapping */
> -	snprintf(path, sizeof(path), "%"PRIx64"-%"PRIx64, vma->e->start, vma->e->end);
> -
>  	/*
>  	 * Note that we "open" it in dumper process space
>  	 * so later we might refer to it via /proc/self/fd/vm_file_fd
> diff --git a/criu/util.c b/criu/util.c
> index e8ebe61..07bc16e 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -161,12 +161,13 @@ void pr_vma(unsigned int loglevel, const struct vma_area *vma_area)
>  		return;
>  
>  	vma_opt_str(vma_area, opt);
> -	print_on_level(loglevel, "%#"PRIx64"-%#"PRIx64" (%"PRIi64"K) prot %#x flags %#x st %#x off %#"PRIx64" "
> +	print_on_level(loglevel, "%#"PRIx64"-%#"PRIx64" (%"PRIi64"K) prot %#x flags %#x fdflags %#o st %#x off %#"PRIx64" "
>  			"%s shmid: %#"PRIx64"\n",
>  			vma_area->e->start, vma_area->e->end,
>  			KBYTES(vma_area_len(vma_area)),
>  			vma_area->e->prot,
>  			vma_area->e->flags,
> +			vma_area->e->fdflags,
>  			vma_area->e->status,
>  			vma_area->e->pgoff,
>  			opt, vma_area->e->shmid);
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
> .
>