[CRIU] [PATCH] zdtm: add maps_file_mixedprot

[Andrew Vagin]

It looks Ok, just a few nit comments. Thanks.

On Wed, Apr 02, 2014 at 05:53:29PM -0700, Jamie Liu wrote:
> Tests for the bug fixed by "criu: correctly handle mixed-permission
> mapped files".
> 
> Signed-off-by: Jamie Liu <jamieliu at google.com>
> ---
>  test/zdtm.sh                                |  1 +
>  test/zdtm/live/static/Makefile              |  1 +
>  test/zdtm/live/static/maps_file_mixedprot.c | 58 +++++++++++++++++++++++++++++
>  3 files changed, 60 insertions(+)
>  create mode 100644 test/zdtm/live/static/maps_file_mixedprot.c
> 
> diff --git a/test/zdtm.sh b/test/zdtm.sh
> index d7f8fca..082aca0 100755
> --- a/test/zdtm.sh
> +++ b/test/zdtm.sh
> @@ -12,6 +12,7 @@ static/maps00
>  static/maps01
>  static/maps02
>  static/maps04
> +static/maps_file_mixedprot
>  static/mprotect00
>  static/mtime_mmap
>  static/sleeping00
> diff --git a/test/zdtm/live/static/Makefile b/test/zdtm/live/static/Makefile
> index 77e8788..ed6a341 100644
> --- a/test/zdtm/live/static/Makefile
> +++ b/test/zdtm/live/static/Makefile
> @@ -140,6 +140,7 @@ TST_FILE	=				\
>  		file_locks00			\
>  		file_locks01			\
>  		netns-nf			\
> +		maps_file_mixedprot		\
>  
>  TST_DIR		=				\
>  		cwd00				\
> diff --git a/test/zdtm/live/static/maps_file_mixedprot.c b/test/zdtm/live/static/maps_file_mixedprot.c
> new file mode 100644
> index 0000000..4c5b8a4
> --- /dev/null
> +++ b/test/zdtm/live/static/maps_file_mixedprot.c
> @@ -0,0 +1,58 @@
> +#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);
> +
> +static char buf[2*PAGE_SIZE];
> +
> +#define die(fmt, arg...) do { err(fmt, ## arg); return 1; } while (0)
> +
> +int main(int argc, char ** argv)
> +{
> +	void *base_addr, *ro_map, *rw_map;
> +	int fd;
> +
> +	test_init(argc, argv);
> +
> +	fd = open(filename, O_RDWR | O_CREAT, 0644);
> +	if (fd < 0)
> +		die("open failed");
> +	if (write(fd, buf, sizeof(buf)) != sizeof(buf))
> +		die("write failed");

We can use ftruncate(fd, 2 * PAGE_SIZE) here, can't we?

> +
> +	/* Use MAP_FIXED so we can ensure ro_map < rw_map */
> +	base_addr = mmap(NULL, 2 * PAGE_SIZE, PROT_NONE,
> +			MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	if (base_addr == MAP_FAILED)
> +		die("base_addr mmap failed");
> +	ro_map = mmap(base_addr, PAGE_SIZE, PROT_READ, MAP_SHARED | MAP_FIXED,
> +			fd, 0);
> +	if (ro_map == MAP_FAILED)
> +		die("ro_map mmap failed");
> +	rw_map = mmap(base_addr + PAGE_SIZE, PAGE_SIZE,
> +			PROT_READ | PROT_WRITE, MAP_SHARED | MAP_FIXED, fd,
> +			PAGE_SIZE);
> +	if (rw_map == MAP_FAILED)
> +		die("rw_map mmap failed");

The following code does the same and it's a bit more clear (imho).

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

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


> +	close(fd);
> +
> +	test_daemon();
> +	test_waitsig();
> +
> +	/* Check that rw_map is still writeable */
> +	*(volatile char *)rw_map = 1;
> +
> +	pass();
> +	return 0;
> +}
> -- 
> 1.9.1.423.g4596e3a
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu