From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp20.mail.ru (smtp20.mail.ru [94.100.179.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id DEC2846970E for ; Tue, 4 Feb 2020 01:03:12 +0300 (MSK) Date: Tue, 4 Feb 2020 01:03:25 +0300 From: Alexander Turenko Message-ID: <20200203220325.4xrpboio4sugswm4@tkn_work_nb> References: <20200115170524.20471-1-gorcunov@gmail.com> <20200115170524.20471-3-gorcunov@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200115170524.20471-3-gorcunov@gmail.com> Subject: Re: [Tarantool-patches] [PATCH v2 2/2] fiber: exit with panic if we unable to revert guard page List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tml Your reasoning looks correct, but actually I don't have enough expertise to understand all pros and cons. So the patch LGTM, but I would ask you to acquire the second review from Vlad Sh. WBR, Alexander Turenko. On Wed, Jan 15, 2020 at 08:05:24PM +0300, Cyrill Gorcunov wrote: > At the moment we setup fiber's stack with a guard page > which is used to detect stack overrun. This page is just > a regular page taken from a slab with PROT_NONE attribute. > > Once fiber is no longer needed we try to revert this > attribute back to PROT_READ | PROT_WRITE. Still there > is a pretty small chance that this attempt get failed. > > Thus in such case we should not allow to proceed but rather > lets panic, otherwise the slab won't longer be solid r/w memory > area and attempt to write into this page will cause > an unpredictable exception. > > Signed-off-by: Cyrill Gorcunov > --- > src/lib/core/fiber.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c > index b51f46f2f..fdad7607c 100644 > --- a/src/lib/core/fiber.c > +++ b/src/lib/core/fiber.c > @@ -1041,13 +1041,17 @@ fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc) > * to setup the original protection back in > * background. > * > + * For now lets exit with panic: if mprotect > + * failed we must not allow to reuse such slab > + * with PROT_NONE'ed page somewhere inside. > + * > * Note that in case if we're called from > * fiber_stack_create() the @mprotect_flags is > * the same as the slab been created with, so > * calling mprotect for VMA with same flags > * won't fail. > */ > - diag_log(); > + panic_syserror("fiber: Can't put guard page to slab"); > } > slab_put(slabc, fiber->stack_slab); > } > -- > 2.20.1 >