DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 20BBC80C1D
Date: Wed, 21 Jun 2017 17:57:30 +0200
From: Oleg Nesterov <oleg@redhat.com>
To: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Hugh Dickins <hughd@google.com>, Andrey Vagin <avagin@openvz.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Pavel Emelyanov <xemul@virtuozzo.com>,
        Dmitry Safonov <dsafonov@virtuozzo.com>,
        Andrew Morton <akpm@linuxfoundation.org>,
        Adrian Reber <areber@redhat.com>
Subject: Re: [criu] 1M guard page ruined restore
Message-ID: <20170621155730.GA32554@redhat.com>
References: <20170620075206.GB1909@uranus.lan>
 <alpine.LSU.2.11.1706200259080.11599@eggly.anvils>
 <20170621152256.GC31050@uranus>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170621152256.GC31050@uranus>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2286
Lines: 67

(add Adrian)

On 06/21, Cyrill Gorcunov wrote:
>
> The patches for criu are on the fly. Still one of the test case
> start failing with the new kernels. Basically the test does
> the following:

Cyrill, please read the last email I sent you in another (private) discussion.
Most probably you should throw out some tests which assume the kernel has the
stack-guard-page hack, it was replaced by the stack-guard-hole hack ;)

>  - allocate growsdown memory area
>  - touch first byte (which before the patch force the kernel
>    to extend the stack allocating new page)
>  - touch first-1 byte
> 
> ---
> int main(int argc, char **argv)
> {
> 	char *start_addr, *start_addr1, *fake_grow_down, *test_addr, *grow_down;
> 	volatile char *p;
> 
> 	start_addr = mmap(NULL, PAGE_SIZE * 10, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> 	if (start_addr == MAP_FAILED) {
> 		printf("Can't mal a new region");
> 		return 1;
> 	}
> 	printf("start_addr %lx\n", start_addr);
> 	munmap(start_addr, PAGE_SIZE * 10);
> 
> 	fake_grow_down = mmap(start_addr + PAGE_SIZE * 5, PAGE_SIZE,
> 			 PROT_READ | PROT_WRITE,
> 			 MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED | MAP_GROWSDOWN, -1, 0);
> 	if (fake_grow_down == MAP_FAILED) {
> 		printf("Can't mal a new region");
> 		return 1;
> 	}
> 	printf("start_addr %lx\n", fake_grow_down);
> 
> 	p = fake_grow_down;
> 	*p-- = 'c';

I guess this works? I mean, *p-- = 'c' should not fail...

> 	*p = 'b';

OK, now we need to expand the stack. This can fail or not. This depends on
whether this vma (created by mmap(MAP_GROWSDOWN) has a stack_guard_gap hole
between its ->vm_prev.

> function get dropped off. Hugh, it is done on intent and
> userspace programs have to extend stack manually?

No. a MAP_GROWSDOWN area should grow automatically. Unless the hole between
vm_prev becomes less than stack_guard_gap.

This is the whole point of guard hole, or guard page we had before. Just the
previous implementation was not accurate, that is why criu had to have some
hacks to workaround.

It no longer needs to know about guard hole/page/whatever. Just remove
(conditionalize) all the MAP_GROWSDOWN code. Except, of course, you still
need to record MAP_GROWSDOWN in vma_area->e->flags (_vmflag_match), in order
to restore this vma correctly.

Oleg.