[CRIU] Re: [PATCH cr 08/16] restore: don't unmap premmapped private vma-s

[Pavel Emelyanov]

> @@ -231,6 +233,23 @@ static int map_private_vma(pid_t pid, struct vma_area *vma, void *tgt_addr,
>  	return 0;
>  }
>  
> +/* Prevent merging vma-s outside and inside the premmapped region */

Why? Besides, having two guard pages doesn't prevent vmas from being merged.

> +static int unmap_priv_guard_pages()
> +{
> +	if (munmap(premmapped_addr, PAGE_SIZE)) {
> +		pr_perror("Could not unmap a guard page %p", premmapped_addr);
> +		return -1;
> +	}
> +
> +	if (munmap(premmapped_addr + premmapped_len - PAGE_SIZE, PAGE_SIZE)) {
> +		pr_perror("Could not unmap a guard page %p",
> +				premmapped_addr + premmapped_len);
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
>  static int read_vmas(int pid)
>  {
>  	int fd, ret = 0;
> @@ -280,13 +299,26 @@ static int read_vmas(int pid)
>  		priv_size += vma_area_len(vma);
>  	}
>  
> -	/* Reserve a place for mapping private vma-s one by one */
> -	addr = mmap(NULL, priv_size, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);
> +	/*
> +	 * Reserve a place for mapping private vma-s one by one.
> +	 *
> +	 * Add two guard pages from both sides. This pages will be
> +	 * unmaped before reading a current vmas for this process.
> +	 * In this case we can be sure, that vmas outside and inside
> +	 * of the premmapped region are not merged to each other.
> +	 */
> +	addr = mmap(NULL, priv_size + 2 * PAGE_SIZE,
> +			PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, 0, 0);

If we DO need guard pages then implement this in separate patch with proper description.

>  	if (addr == MAP_FAILED) {
>  		pr_perror("Unable to reserve memory");
>  		return -1;
>  	}
>  
> +	premmapped_addr = addr;
> +	premmapped_len = priv_size + 2 * PAGE_SIZE;
> +
> +	addr += PAGE_SIZE;
> +
>  	list_for_each_entry(vma, &vma_list, list) {
>  		if (!vma_priv(&vma->vma))
>  			continue;
> @@ -321,6 +321,10 @@ long __export_restore_task(struct task_restore_core_args *args)
>  		if (!vma_entry_is(vma_entry, VMA_AREA_REGULAR))
>  			continue;
>  
> +		if (args->premmapped_addr <= vma_entry->start &&
> +		    vma_entry->end <= args->premmapped_addr + args->premmapped_len)
> +			continue;

Why so complex? Isn't it easier just to remove this are from self_vma_list in cr-restore?

> +
>  		if (sys_munmap((void *)vma_entry->start, vma_entry_len(vma_entry))) {
>  			pr_err("Munmap fail for %lx\n", vma_entry->start);
>  			goto core_restore_end;
>