[CRIU] [PATCH v2] criu: correctly handle mixed-permission mapped files

[Andrew Vagin]

On Wed, Apr 02, 2014 at 05:52:56PM -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 means
> that 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 with O_RDONLY
> during restore, and mmap(PROT_WRITE) will fail with EACCES.
> 
> To fix this, defer filemap open flags selection until restore, since
> mapped files are opened per mapping.

Hi Jamie,

I have modified your test and it fails even with this patch.

The idea of my changes is simple, I call mprotect after suspend/resume:

open(xxx, O_RDWR)
mmap(0, size, PROT_READ, MAP_SHARED, fd, 0);
/* suspend/resume */
mprotect(addr, size, PROT_READ | PROT_WRITE))


Execute zdtm/live/static/maps_file_mixedprot
./maps_file_mixedprot --pidfile=maps_file_mixedprot.pid --outfile=maps_file_mixedprot.out --filename=maps_file_mixedprot.test
Dump 29705
Restore
Check results 29705
12:41:42.588: 29705: ERR: maps_file_mixedprot.c:39: mprotect failed (errno = 13 (Permission denied))
Test: zdtm/live/static/maps_file_mixedprot, Result: FAIL
==================================== ERROR ====================================
Test: zdtm/live/static/maps_file_mixedprot, Namespace: 
Dump log   : /root/git/crtools/test/dump/maps_file_mixedprot/29705/1/dump.log
--------------------------------- grep Error ---------------------------------
------------------------------------- END -------------------------------------
Restore log: /root/git/crtools/test/dump/maps_file_mixedprot/29705/1/restore.log
--------------------------------- grep Error ---------------------------------
------------------------------------- END -------------------------------------
Output file: /root/git/crtools/test/zdtm/live/static/maps_file_mixedprot.out
------------------------------------------------------------------------------
12:41:42.588: 29705: ERR: maps_file_mixedprot.c:39: mprotect failed (errno = 13 (Permission denied))
------------------------------------- END -------------------------------------
================================= ERROR OVER =================================

> 
> Signed-off-by: Jamie Liu <jamieliu at google.com>
> ---
>  cr-dump.c   | 6 +-----
>  files-reg.c | 8 ++++++++
>  2 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/cr-dump.c b/cr-dump.c
> index 0111115..f7945d9 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 detected 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..743b035 100644
> --- a/files-reg.c
> +++ b/files-reg.c
> @@ -3,6 +3,7 @@
>  #include <errno.h>
>  #include <string.h>
>  #include <sys/types.h>
> +#include <sys/mman.h>
>  #include <sys/stat.h>
>  #include <sys/vfs.h>
>  #include <ctype.h>
> @@ -708,6 +709,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 +718,11 @@ 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->prot & PROT_WRITE) && vma_area_is(vma, VMA_FILE_SHARED))
> +		rfi->rfe->flags = O_RDWR;
> +	else
> +		rfi->rfe->flags = O_RDONLY;
>  	return open_path(vma->fd, do_open_reg_noseek, NULL);
>  }
>  
> -- 
> 1.9.1.423.g4596e3a
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
-------------- next part --------------
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <linux/limits.h>
#include "zdtmtst.h"

const char *test_doc	= "Test mappings of same file with different prot";
const char *test_author	= "Jamie Liu <jamieliu at google.com>";

char *filename;
TEST_OPTION(filename, string, "file name", 1);

#define die(fmt, arg...) do { err(fmt, ## arg); return 1; } while (0)

int main(int argc, char ** argv)
{
	void *ro_map, *rw_map;
	int fd;

	test_init(argc, argv);

	fd = open(filename, O_RDWR | O_CREAT, 0644);
	if (fd < 0)
		die("open failed");
	ftruncate(fd, PAGE_SIZE * 2);

	ro_map = mmap(NULL, 2 * PAGE_SIZE, PROT_READ, MAP_SHARED, fd, 0);
	if (ro_map == MAP_FAILED)
		die("ro_map mmap failed");

	test_daemon();
	test_waitsig();

	rw_map = ro_map + PAGE_SIZE;
	if (mprotect(rw_map, PAGE_SIZE, PROT_READ | PROT_WRITE))
		die("mprotect failed");

	close(fd);
	/* Check that rw_map is still writeable */
	*(volatile char *)rw_map = 1;

	pass();
	return 0;
}