From e7a9fd0b255ebf4485feec2841876cb4c667bd28 Mon Sep 17 00:00:00 2001
From: mechanik20051988 <mechanik20051988@tarantool.org>
Date: Fri, 1 Oct 2021 13:09:02 +0300
Subject: [PATCH] iproto: fix crash if box.cfg listen is woken up.
There was access to previously freed memory in case when `cbus_call`
is interrupted: `cbus_call_msg` in iproto allocates on stack, and if
`cbus_call` failed due to fiber cancelation or wake up, `cbus_call_msg`
memory is released. But function called through cbus is still work in
iproto thread and there will be an attempt to access this memory when
this function in iproto thread finished it's work. This patch rework
this behaviour, now before `cbus_call` we reset FIBER_IS_CANCELLABLE
flag, to prevent fiber cancellation or it's wake up.
Closes #6480
---
...480-crash-if-box-cfg-listen-is-woken-up.md | 3 ++
src/box/iproto.cc | 32 +++++++++++--------
...crash-if-box-cfg-listen-is-woken-up.result | 25 +++++++++++++++
...ash-if-box-cfg-listen-is-woken-up.test.lua | 10 ++++++
4 files changed, 57 insertions(+), 13 deletions(-)
create mode 100644 changelogs/unreleased/gh-6480-crash-if-box-cfg-listen-is-woken-up.md
create mode 100644 test/box/gh-6480-crash-if-box-cfg-listen-is-woken-up.result
create mode 100644 test/box/gh-6480-crash-if-box-cfg-listen-is-woken-up.test.lua
diff --git a/changelogs/unreleased/gh-6480-crash-if-box-cfg-listen-is-woken-up.md b/changelogs/unreleased/gh-6480-crash-if-box-cfg-listen-is-woken-up.md
new file mode 100644
index 0000000000..cb5cb5bedd
--- /dev/null
+++ b/changelogs/unreleased/gh-6480-crash-if-box-cfg-listen-is-woken-up.md
@@ -0,0 +1,3 @@
+## bugfix/core
+
+* Fixed crash in case a fiber changing box.cfg.listen is woken up (gh-6480).
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 436d743e2f..f68f0ea680 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -2955,21 +2955,29 @@ static inline int
iproto_do_cfg(struct iproto_thread *iproto_thread, struct iproto_cfg_msg *msg)
{
msg->iproto_thread = iproto_thread;
- if (cbus_call(&iproto_thread->net_pipe, &iproto_thread->tx_pipe, msg,
- iproto_do_cfg_f, NULL, TIMEOUT_INFINITY) != 0)
- return -1;
- return 0;
+ bool prev = fiber_set_cancellable(false);
+ int rc = cbus_call(&iproto_thread->net_pipe, &iproto_thread->tx_pipe,
+ msg, iproto_do_cfg_f, NULL, TIMEOUT_INFINITY);
+ fiber_set_cancellable(prev);
+ return rc;
}
-static inline int
+static inline void
+iproto_do_cfg_crit(struct iproto_thread *iproto_thread,
+ struct iproto_cfg_msg *cfg_msg)
+{
+ int rc = iproto_do_cfg(iproto_thread, cfg_msg);
+ (void)rc;
+ assert(rc == 0);
+}
+
+static inline void
iproto_send_stop_msg(void)
{
struct iproto_cfg_msg cfg_msg;
iproto_cfg_msg_create(&cfg_msg, IPROTO_CFG_STOP);
for (int i = 0; i < iproto_threads_count; i++)
- if (iproto_do_cfg(&iproto_threads[i], &cfg_msg) != 0)
- return -1;
- return 0;
+ iproto_do_cfg_crit(&iproto_threads[i], &cfg_msg);
}
static inline int
@@ -2987,8 +2995,7 @@ iproto_send_listen_msg(struct evio_service *binary)
int
iproto_listen(const char *uri)
{
- if (iproto_send_stop_msg() != 0)
- return -1;
+ iproto_send_stop_msg();
evio_service_stop(&tx_binary);
if (uri == NULL) {
tx_binary.addr_len = 0;
@@ -3040,7 +3047,7 @@ iproto_thread_stats_get(struct iproto_stats *stats, int thread_id)
iproto_cfg_msg_create(&cfg_msg, IPROTO_CFG_STAT);
assert(thread_id >= 0 && thread_id < iproto_threads_count);
cfg_msg.stats = stats;
- iproto_do_cfg(&iproto_threads[thread_id], &cfg_msg);
+ iproto_do_cfg_crit(&iproto_threads[thread_id], &cfg_msg);
stats->requests_in_progress =
iproto_threads[thread_id].tx.requests_in_progress;
}
@@ -3066,8 +3073,7 @@ iproto_set_msg_max(int new_iproto_msg_max)
iproto_cfg_msg_create(&cfg_msg, IPROTO_CFG_MSG_MAX);
cfg_msg.iproto_msg_max = new_iproto_msg_max;
for (int i = 0; i < iproto_threads_count; i++) {
- if (iproto_do_cfg(&iproto_threads[i], &cfg_msg) != 0)
- diag_raise();
+ iproto_do_cfg_crit(&iproto_threads[i], &cfg_msg);
cpipe_set_max_input(&iproto_threads[i].net_pipe,
new_iproto_msg_max / 2);
}
diff --git a/test/box/gh-6480-crash-if-box-cfg-listen-is-woken-up.result b/test/box/gh-6480-crash-if-box-cfg-listen-is-woken-up.result
new file mode 100644
index 0000000000..c942a5b5bb
--- /dev/null
+++ b/test/box/gh-6480-crash-if-box-cfg-listen-is-woken-up.result
@@ -0,0 +1,25 @@
+test_run = require('test_run').new()
+---
+...
+fiber = require('fiber')
+---
+...
+f = fiber.create(function() box.stat.net() end) f:wakeup()
+---
+...
+old_listen = box.cfg.listen
+---
+...
+new_listen = old_listen .. "A"
+---
+...
+f = fiber.create(function() box.cfg{listen = new_listen} end) f:wakeup()
+---
+...
+test_run:wait_cond(function() return box.cfg.listen == new_listen end)
+---
+- true
+...
+box.cfg{listen = old_listen}
+---
+...
diff --git a/test/box/gh-6480-crash-if-box-cfg-listen-is-woken-up.test.lua b/test/box/gh-6480-crash-if-box-cfg-listen-is-woken-up.test.lua
new file mode 100644
index 0000000000..4d52147880
--- /dev/null
+++ b/test/box/gh-6480-crash-if-box-cfg-listen-is-woken-up.test.lua
@@ -0,0 +1,10 @@
+test_run = require('test_run').new()
+fiber = require('fiber')
+
+f = fiber.create(function() box.stat.net() end) f:wakeup()
+
+old_listen = box.cfg.listen
+new_listen = old_listen .. "A"
+f = fiber.create(function() box.cfg{listen = new_listen} end) f:wakeup()
+test_run:wait_cond(function() return box.cfg.listen == new_listen end)
+box.cfg{listen = old_listen}
--
GitLab