From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (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 DE25B445320 for ; Tue, 21 Jul 2020 14:54:02 +0300 (MSK) Date: Tue, 21 Jul 2020 11:54:01 +0000 From: Nikita Pettik Message-ID: <20200721115401.GA21662@tarantool.org> References: <20200721110514.10344-1-i.kosarev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200721110514.10344-1-i.kosarev@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2] lua: assert on lua_gettop() negative return value List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ilya Kosarev Cc: tarantool-patches@dev.tarantool.org On 21 Jul 14:05, Ilya Kosarev wrote: > In case lua_gettop() called from encode_lua_call() returns negative > value, we will segfault in iproto_reply_error() with empty diag, as far > as it is unexpected error path not covered with diagnostics. Thus > corresponding sane check with assert is introduced. > > Closes #4649 > --- > Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4649-sane-check-on-lua_gettop > Issue: https://github.com/tarantool/tarantool/issues/4649 > > Changes in v2: > - added reasoning in comment > > src/box/lua/call.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/box/lua/call.c b/src/box/lua/call.c > index ca871e077..516276a9f 100644 > --- a/src/box/lua/call.c > +++ b/src/box/lua/call.c > @@ -361,6 +361,12 @@ encode_lua_call(lua_State *L) > > struct luaL_serializer *cfg = luaL_msgpack_default; > int size = lua_gettop(port->L); > + /* > + * lua_gettop() might return negative value in case the internal state > + * of the given Lua coroutine is seriously broken. In case of such > + * behavior execution has to be aborted immediately. > + */ > + assert(size >= 0); As I said, assuming lua_gettop() does return negative index under no circumstances and your assumption is false, what other asserts or checks can you suggest? I mean this problem appears extremely rare, so we'd better provide extra checks just in case (so that we locate problem with ease next time). Could you elaborate on this? > for (int i = 1; i <= size; ++i) > luamp_encode(port->L, cfg, &stream, i); > port->size = size; > -- > 2.17.1 >