From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 76EE26EC5B; Thu, 13 May 2021 13:34:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 76EE26EC5B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1620902098; bh=jbWEBYDbJE9YdyceyL+C2AovBv+jYSg340w/wR+R8ko=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=Fjfz6XbKlIObmSW+cwIyOm4s9Ox8NM9mW1UtKUDg8Jvmf2F1aGwNwrvj02mfeCdLO lqIegAVHnraU3vgz3XOfET3VCHBXjtoMNikjT5YssWhXbVovUcFu8iSDWvnlm/1C+U kUmg5RZeWnJBsftVD7NvcUTmayyyDq8WNnUywcdE= Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 994936EC5B for ; Thu, 13 May 2021 13:34:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 994936EC5B Received: by smtp48.i.mail.ru with esmtpa (envelope-from ) id 1lh8gB-0006me-Oi; Thu, 13 May 2021 13:34:56 +0300 To: Vladislav Shpilevoy , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org References: <20210512113907.12968-1-sergepetrenko@tarantool.org> Message-ID: <33270514-8a1f-355c-3214-669339be9f58@tarantool.org> Date: Thu, 13 May 2021 13:34:55 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD95978C26455E69BE0B2C7F7C3B0039F139F0EF8E3717E163C182A05F5380850406B08658B2F3203D55B2C1051C02408580F7BCEDDF4CE28CA9BCED8695D8C9248 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7195F30236A8D43B4EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006371E4BC0E00C009995EA1F7E6F0F101C67CDEEF6D7F21E0D1D9295C2E9FA3191EE1B59CA4C82EFA65842798A8ED9F0644D5B67599409CDCF95F6B57BC7E64490618DEB871D839B73339E8FC8737B5C2249F459A8243F1D1D44CC7F00164DA146DAFE8445B8C89999729449624AB7ADAF37F6B57BC7E64490611E7FA7ABCAF51C92176DF2183F8FC7C05A64D9A1E9CA65708941B15DA834481F9449624AB7ADAF37BA3038C0950A5D3613377AFFFEAFD269176DF2183F8FC7C0D68B293E8C4BA4977B076A6E789B0E97A8DF7F3B2552694A1E7802607F20496D49FD398EE364050F1E561CDFBCA1751FCFA063A519E5965DB3661434B16C20AC78D18283394535A9E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8B8E48D8FAA4D20A9F75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A5FC8F197A1DC426EC36549737BA8AD86FD8FBE5A3B5C99210D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75F04B387B5D7535DE410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D346B222596F62B8FA9BADA78E0C5AB7A4CA9E5CB585DA69D01014C0A82869BC1FC86A3B4A5A5255FA61D7E09C32AA3244C6FE6B5200373A3E78450A369208C5F0739C99C45E8D137E9FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojoybArHp+PQW25OFSc1X7kQ== X-Mailru-Sender: 583F1D7ACE8F49BD95918038521BA2AA97FE0BD80A6D927763B555A90ACB2EBFE42D40BFFBC843ED424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH] relay: fix use after free in subscribe_f X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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 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