[Tarantool-patches] [PATCH] relay: fix use after free in subscribe_f
Serge Petrenko
sergepetrenko at tarantool.org
Thu May 13 13:34:55 MSK 2021
12.05.2021 23:25, Vladislav Shpilevoy пишет:
> Hi! Thanks for the patch!
>
>> diff --git a/src/box/relay.cc b/src/box/relay.cc
>> index ff43c2fc7..32d3a58dd 100644
>> --- a/src/box/relay.cc
>> +++ b/src/box/relay.cc
>> @@ -756,6 +755,8 @@ relay_subscribe_f(va_list ap)
>> if (!relay->replica->anon)
>> relay_send_is_raft_enabled(relay, &raft_enabler, true);
>>
>> + struct recovery *r = relay->r;
>> +
> There is another cbus_process() on line 808. Won't it lead to the same issue
> if recovery would be restarted? I see it is for version < 1.7.4 so probably
> not. Another option would be to simply inline relay->r in its usage places
> and not remember it into a variable.
>
> Anyway LGTM. Up to you if want to inline.
Thanks for the review!
Yep, let's inline this.
I've also added a changelog entry.
The patch's changed a lot so here's the whole new version.
===========================================
Author: Serge Petrenko <sergepetrenko at tarantool.org>
Date: Wed May 12 12:58:34 2021 +0300
relay: fix use after free in subscribe_f
relay_subscribe_f() remembered old recovery pointer, which might be
replaced by relay_restart_recovery() if a raft message is delivered
during
cbus_process() loop in relay_send_is_raft_enabled().
Fix the issue by removing the alias altogether and referencing relay->r
directly to prevent any further mistakes.
Closes #6031
diff --git a/changelogs/unreleased/gh-6031-relay-use-after-free.md
b/changelogs/unreleased/gh-6031-relay-use-after-free.md
new file mode 100644
index 000000000..910b29614
--- /dev/null
+++ b/changelogs/unreleased/gh-6031-relay-use-after-free.md
@@ -0,0 +1,3 @@
+## bugfix/replication
+
+* Fix use after free in relay thread when using elections (gh-6031).
diff --git a/src/box/relay.cc b/src/box/relay.cc
index ff43c2fc7..5a21a4499 100644
--- a/src/box/relay.cc
+++ b/src/box/relay.cc
@@ -741,7 +741,6 @@ static int
relay_subscribe_f(va_list ap)
{
struct relay *relay = va_arg(ap, struct relay *);
- struct recovery *r = relay->r;
coio_enable();
relay_set_cord_name(relay->io.fd);
@@ -764,7 +763,7 @@ relay_subscribe_f(va_list ap)
struct trigger on_close_log;
trigger_create(&on_close_log, relay_on_close_log_f, relay, NULL);
if (!relay->replica->anon)
- trigger_add(&r->on_close_log, &on_close_log);
+ trigger_add(&relay->r->on_close_log, &on_close_log);
/* Setup WAL watcher for sending new rows to the replica. */
wal_set_watcher(&relay->wal_watcher, relay->endpoint.name,
@@ -816,7 +815,7 @@ relay_subscribe_f(va_list ap)
continue;
struct vclock *send_vclock;
if (relay->version_id < version_id(1, 7, 4))
- send_vclock = &r->vclock;
+ send_vclock = &relay->r->vclock;
else
send_vclock = &relay->recv_vclock;
===========================================
--
Serge Petrenko
More information about the Tarantool-patches
mailing list