Tarantool development patches archive
 help / color / mirror / Atom feed
From: Ilya Kosarev <i.kosarev@tarantool.org>
To: gorcunov@gmail.com
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH 2/2] fiber: leak slab if unable to bring prots back
Date: Wed, 29 Jul 2020 19:50:42 +0300	[thread overview]
Message-ID: <26927f9720c3d70b27c0ea25b642cd230c29809e.1596040943.git.i.kosarev@tarantool.org> (raw)
In-Reply-To: <cover.1596040943.git.i.kosarev@tarantool.org>
In-Reply-To: <cover.1596040943.git.i.kosarev@tarantool.org>

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.

Relates to #5211

Co-authored-by: Cyrill Gorcunov <gorcunov@gmail.com>
(backported from commit 8d53fadc0cb15a45ea0cd461d2d5243be51b37e0)
---
 src/fiber.c                  | 11 +++++++++--
 test/unit/fiber_stack.c      | 36 +++++++++++++++++++++++++++++++++---
 test/unit/fiber_stack.result |  5 ++++-
 3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/src/fiber.c b/src/fiber.c
index 2d83bbc1a..cf6b78c43 100644
--- a/src/fiber.c
+++ b/src/fiber.c
@@ -936,15 +936,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 @a 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);
+		} else {
+			slab_put(slabc, fiber->stack_slab);
 		}
-		slab_put(slabc, fiber->stack_slab);
 	}
 }
 
diff --git a/test/unit/fiber_stack.c b/test/unit/fiber_stack.c
index 103dfaf0d..41abe3764 100644
--- a/test/unit/fiber_stack.c
+++ b/test/unit/fiber_stack.c
@@ -15,11 +15,13 @@ noop_f(va_list ap)
 static int
 main_f(va_list ap)
 {
+	struct slab_cache *slabc = &cord()->slabc;
+	size_t used_before, used_after;
 	struct errinj *inj;
 	struct fiber *fiber;
 
 	header();
-	plan(4);
+	plan(6);
 
 	/*
 	 * Set non-default stack size to prevent reusing of an
@@ -47,8 +49,7 @@ main_f(va_list ap)
 	ok(diag_get() != NULL, "mprotect: diag is armed after error");
 
 	/*
-	 * Check madvise. We can't test the fiber destroy
-	 * path since it is cleaning error.
+	 * Check madvise error on fiber creation.
 	 */
 	diag_clear(diag_get());
 	inj = errinj(ERRINJ_FIBER_MADVISE, ERRINJ_BOOL);
@@ -59,6 +60,35 @@ main_f(va_list ap)
 	ok(fiber != NULL, "madvise: non critical error on madvise hint");
 	ok(diag_get() != NULL, "madvise: diag is armed after error");
 
+	/*
+	 * Check if we leak on fiber destruction.
+	 * We will print an error and result get
+	 * compared by testing engine.
+	 */
+	fiber_attr_delete(fiber_attr);
+	fiber_attr = fiber_attr_new();
+	fiber_attr->flags |= FIBER_CUSTOM_STACK;
+	fiber_attr->stack_size = 64 << 10;
+
+	diag_clear(diag_get());
+
+	used_before = slabc->allocated.stats.used;
+
+	fiber = fiber_new_ex("test_madvise", fiber_attr, noop_f);
+	ok(fiber != NULL, "fiber with custom stack");
+	fiber_set_joinable(fiber, true);
+
+	inj = errinj(ERRINJ_FIBER_MPROTECT, ERRINJ_INT);
+	inj->iparam = PROT_READ | PROT_WRITE;
+
+	fiber_start(fiber);
+	fiber_join(fiber);
+	inj->iparam = -1;
+
+	used_after = slabc->allocated.stats.used;
+	ok(used_after > used_before, "expected leak detected");
+
+	fiber_attr_delete(fiber_attr);
 	footer();
 
 	ev_break(loop(), EVBREAK_ALL);
diff --git a/test/unit/fiber_stack.result b/test/unit/fiber_stack.result
index 43ff74b2f..7cae4e96c 100644
--- a/test/unit/fiber_stack.result
+++ b/test/unit/fiber_stack.result
@@ -1,8 +1,11 @@
 SystemError fiber mprotect failed: Cannot allocate memory
+fiber: Can't put guard page to slab. Leak 57344 bytes: Cannot allocate memory
 	*** main_f ***
-1..4
+1..6
 ok 1 - mprotect: failed to setup fiber guard page
 ok 2 - mprotect: diag is armed after error
 ok 3 - madvise: non critical error on madvise hint
 ok 4 - madvise: diag is armed after error
+ok 5 - fiber with custom stack
+ok 6 - expected leak detected
 	*** main_f: done ***
-- 
2.17.1

      parent reply	other threads:[~2020-07-29 16:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-29 16:50 [Tarantool-patches] [PATCH 0/2] fiber: backport for stack madvise/mprotect errors handling Ilya Kosarev
2020-07-29 16:50 ` [Tarantool-patches] [PATCH 1/2] fiber: set diagnostics at madvise/mprotect failure Ilya Kosarev
2020-07-29 17:04   ` Cyrill Gorcunov
2020-07-29 16:50 ` Ilya Kosarev [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=26927f9720c3d70b27c0ea25b642cd230c29809e.1596040943.git.i.kosarev@tarantool.org \
    --to=i.kosarev@tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2/2] fiber: leak slab if unable to bring prots back' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox