[Tarantool-patches] [PATCH v6 2/2] fiber: leak slab if unable to bring prots back

[Vladislav Shpilevoy]

Thanks for the patch!

On 06/02/2020 13:31, Cyrill Gorcunov wrote:
> In case if we unable to revert guard page back to
> read|write we should never use such slab again.
> 
> Initially I thought of just put panic here and
> exit but it is too destructive. I think better
> print an error and continue. If node admin ignore
> this message then one moment at future there won't
> be slab left for use and creating new fibers get
> prohibited.
> 
> In future (hopefully near one) we plan to drop
> guard pages to prevent VMA fracturing and use
> stack marks instead.
> 
> Reviewed-by: Alexander Turenko <alexander.turenko at tarantool.org>
> Signed-off-by: Cyrill Gorcunov <gorcunov at gmail.com>
> ---
>  src/lib/core/fiber.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> index 57e4cb6ef..a501d846a 100644
> --- a/src/lib/core/fiber.c
> +++ b/src/lib/core/fiber.c
> @@ -1055,15 +1055,22 @@ fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc)
>  			 * to setup the original protection back in
>  			 * background.
>  			 *
> +			 * For now lets keep such slab referenced and
> +			 * leaked: if mprotect failed we must not allow
> +			 * to reuse such slab with PROT_NONE'ed page
> +			 * 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();
> +			say_syserror("fiber: Can't put guard page to slab. "
> +				     "Leak %zu bytes", (size_t)fiber->stack_size);

This behaviour actually should be testable, no? You could not test
diag, but you should be able to detect a leak. Using slab_cache_used(),
for example. Before and after a test.

> +		} else {
> +			slab_put(slabc, fiber->stack_slab);
>  		}
> -		slab_put(slabc, fiber->stack_slab);
>  	}
>  }