Tarantool development patches archive
 help / color / mirror / Atom feed
From: Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tml <tarantool-patches@dev.tarantool.org>
Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [Tarantool-patches] [PATCH 2/2] fiber: fiber_join -- don't crash on misuse
Date: Mon, 26 Apr 2021 13:10:00 +0300	[thread overview]
Message-ID: <20210426101000.278874-3-gorcunov@gmail.com> (raw)
In-Reply-To: <20210426101000.278874-1-gorcunov@gmail.com>

In case if we call jiber_join() over the nonjoinable fiber
we trigger an assert and crash execution (on debug build).

On release build the asserts will be zapped and won't cause
problems but there is an another one -- the target fiber will
cause double fiber_reset() calls which in result cause to
unregister_fid with id = 0 (not causing crash but definitely
out of intention) and we will drop stack protection which
might be not ours anymore.

Thus lets return error just like Lua interface does. Since
nobody complained about this bug yet I think such combination
is not use commonly in external C modules and we can change
the API behaviour.

Fixes #6046

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/lib/core/fiber.c | 5 ++++-
 test/unit/fiber.cc   | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index baf78a130..dd7498dd7 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -615,7 +615,10 @@ fiber_join(struct fiber *fiber)
 int
 fiber_join_timeout(struct fiber *fiber, double timeout)
 {
-	assert(fiber->flags & FIBER_IS_JOINABLE);
+	if (!(fiber->flags & FIBER_IS_JOINABLE)) {
+		diag_set(IllegalParams, "the fiber is not joinable");
+		return -1;
+	}
 
 	if (! fiber_is_dead(fiber)) {
 		bool exceeded = false;
diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
index 9c1a23bdd..fbdd82772 100644
--- a/test/unit/fiber.cc
+++ b/test/unit/fiber.cc
@@ -96,6 +96,9 @@ fiber_join_test()
 	header();
 
 	struct fiber *fiber = fiber_new_xc("join", noop_f);
+	/* gh-6046: crash on attempt to join non joinable */
+	fiber_set_joinable(fiber, false);
+	fiber_join(fiber);
 	fiber_set_joinable(fiber, true);
 	fiber_wakeup(fiber);
 	fiber_join(fiber);
-- 
2.30.2


  parent reply	other threads:[~2021-04-26 10:11 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-26 10:09 [Tarantool-patches] [PATCH 0/2] fiber: prevent fiber_join from misuse Cyrill Gorcunov via Tarantool-patches
2021-04-26 10:09 ` [Tarantool-patches] [PATCH 1/2] fiber: fiber_join -- drop redundat variable Cyrill Gorcunov via Tarantool-patches
2021-04-26 10:10 ` Cyrill Gorcunov via Tarantool-patches [this message]
2021-04-26 20:58   ` [Tarantool-patches] [PATCH 2/2] fiber: fiber_join -- don't crash on misuse Vladislav Shpilevoy via Tarantool-patches
2021-04-26 21:36     ` Cyrill Gorcunov via Tarantool-patches
2021-04-26 20:59   ` Vladislav Shpilevoy via Tarantool-patches

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=20210426101000.278874-3-gorcunov@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2/2] fiber: fiber_join -- don'\''t crash on misuse' \
    /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