Re: [Tarantool-patches] [PATCH v2 2/2] fiber: exit with panic if we unable to revert guard page

[Alexander Turenko <alexander.turenko@tarantool.org>]

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 <gorcunov@gmail.com>
> ---
>  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
>