From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 C650846970F for ; Fri, 22 Nov 2019 01:58:15 +0300 (MSK) From: Vladislav Shpilevoy References: <4c563cef125a2b96a3379defc40fdae4c2a0bc6d.1574112599.git.v.shpilevoy@tarantool.org> Message-ID: Date: Fri, 22 Nov 2019 00:04:51 +0100 MIME-Version: 1.0 In-Reply-To: <4c563cef125a2b96a3379defc40fdae4c2a0bc6d.1574112599.git.v.shpilevoy@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v3 1/1] iproto: don't destroy a session during disconnect List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, kostja.osipov@gmail.com I forgot to push a test file. Here it is. ================================================================================ diff --git a/test/box/gh-4627-session-use-after-free.result b/test/box/gh-4627-session-use-after-free.result new file mode 100644 index 000000000..5e5c154b9 --- /dev/null +++ b/test/box/gh-4627-session-use-after-free.result @@ -0,0 +1,60 @@ +-- test-run result file version 2 +-- +-- gh-4627: binary session disconnect trigger yield could lead to +-- use after free of the session object. That happened because +-- iproto thread sent two requests to TX thread at disconnect: +-- +-- - Close the session and run its on disconnect triggers; +-- +-- - If all requests are handled, destroy the session. +-- +-- When a connection is idle, all requests are handled, so both +-- these requests are sent. If the first one yielded in TX thread, +-- the second one arrived and destroyed the session right under +-- the feet of the first one. +-- +net_box = require('net.box') + | --- + | ... +fiber = require('fiber') + | --- + | ... + +sid_before_yield = nil + | --- + | ... +sid_after_yield = nil + | --- + | ... +func = box.session.on_disconnect(function() \ + sid_before_yield = box.session.id() \ + fiber.yield() \ + sid_after_yield = box.session.id() \ +end) + | --- + | ... + +connection = net_box.connect(box.cfg.listen) + | --- + | ... +connection:ping() + | --- + | - true + | ... +connection:close() + | --- + | ... + +while not sid_after_yield do fiber.yield() end + | --- + | ... + +sid_after_yield == sid_before_yield and sid_after_yield ~= 0 or \ + {sid_after_yield, sid_before_yield} + | --- + | - true + | ... + +box.session.on_disconnect(nil, func) + | --- + | ... diff --git a/test/box/gh-4627-session-use-after-free.test.lua b/test/box/gh-4627-session-use-after-free.test.lua new file mode 100644 index 000000000..70624a96a --- /dev/null +++ b/test/box/gh-4627-session-use-after-free.test.lua @@ -0,0 +1,35 @@ +-- +-- gh-4627: binary session disconnect trigger yield could lead to +-- use after free of the session object. That happened because +-- iproto thread sent two requests to TX thread at disconnect: +-- +-- - Close the session and run its on disconnect triggers; +-- +-- - If all requests are handled, destroy the session. +-- +-- When a connection is idle, all requests are handled, so both +-- these requests are sent. If the first one yielded in TX thread, +-- the second one arrived and destroyed the session right under +-- the feet of the first one. +-- +net_box = require('net.box') +fiber = require('fiber') + +sid_before_yield = nil +sid_after_yield = nil +func = box.session.on_disconnect(function() \ + sid_before_yield = box.session.id() \ + fiber.yield() \ + sid_after_yield = box.session.id() \ +end) + +connection = net_box.connect(box.cfg.listen) +connection:ping() +connection:close() + +while not sid_after_yield do fiber.yield() end + +sid_after_yield == sid_before_yield and sid_after_yield ~= 0 or \ + {sid_after_yield, sid_before_yield} + +box.session.on_disconnect(nil, func)