[CRIU] [PATCH 2/2] mem: Don't assume guard page is returned in procfs with new kernels

[Oleg Nesterov]

Add CC's, we will need to fix criu for rhel...


On 06/20, Cyrill Gorcunov wrote:
>
> If the guard page is not reported in show_map_vma we should
> not ajust vma address neither we should call unmap_guard_pages
> in restorer.
> 
> https://github.com/xemul/criu/issues/322
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  criu/cr-restore.c | 3 ++-
>  criu/proc_parse.c | 6 ++++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/cr-restore.c b/criu/cr-restore.c
> index 298fb693a09e..d44e723cc180 100644
> --- a/criu/cr-restore.c
> +++ b/criu/cr-restore.c
> @@ -1812,7 +1812,8 @@ static int restore_task_with_children(void *_arg)
>  
>  	timing_stop(TIME_FORK);
>  
> -	if (unmap_guard_pages(current))
> +	if (kdat.mm_guard_page_maps &&
> +	    unmap_guard_pages(current))

can't resist... I feel that unmap_guard_pages() logic is not really right and
this helper can be simply removed even with stack-guard... but I can be easily
wrong and this is off-topic right now.

>  
>  	restore_pgid();
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
> index 041d4512413d..4cb4b9dbf288 100644
> --- a/criu/proc_parse.c
> +++ b/criu/proc_parse.c
> @@ -638,8 +638,10 @@ static int vma_list_add(struct vma_area *vma_area,
>  
>  	/* Add a guard page only if here is enough space for it */
>  	if ((vma_area->e->flags & MAP_GROWSDOWN) &&
> -	    *prev_end < vma_area->e->start)
> -		vma_area->e->start -= PAGE_SIZE; /* Guard page */
> +	    *prev_end < vma_area->e->start) {
> +		if (kdat.mm_guard_page_maps)
> +			vma_area->e->start -= PAGE_SIZE; /* Guard page */
> +	}

OK, but what about MAP_GROWSDOWN checks in criu/mem.c ? At first glance it
seems that most of them (if not all) should be updated too, no?

Perhaps you can make a helper? something like

	bool vma_has_guard(vma_area *vma)
	{
		return kdat.mm_guard_page_maps && (vma->e->flags & MAP_GROWSDOWN);
	}

then it would be simpler to change the code whenever this is needed.

And. This can also help us (redhat) ;) more on this later, but most probably
we will need to disable this logic unconditionally (iow, without the 1st patch)
for the short term fix.

Oleg.