From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (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 92F30445320 for ; Mon, 20 Jul 2020 13:50:57 +0300 (MSK) Date: Mon, 20 Jul 2020 10:50:56 +0000 From: Nikita Pettik Message-ID: <20200720105056.GA17289@tarantool.org> References: <20200717172529.12608-1-i.kosarev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200717172529.12608-1-i.kosarev@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] 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 17 Jul 20:25, 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 > > @ChangeLog: > * Raise assert on lua_gettop() negative return value (gh-4649). I guess changelog is likely to be redundant in this case (patch doesnt introduce user-visible changes). > src/box/lua/call.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/box/lua/call.c b/src/box/lua/call.c > index ca871e077..75634de5b 100644 > --- a/src/box/lua/call.c > +++ b/src/box/lua/call.c > @@ -361,6 +361,7 @@ encode_lua_call(lua_State *L) > > struct luaL_serializer *cfg = luaL_msgpack_default; > int size = lua_gettop(port->L); > + assert(size >= 0); I don't understand much in Lua internals, so it would be nice to see rationale for this check. Even if you don't have complete insight concerning it, any thoughts which bring you to this check will be helpful. > for (int i = 1; i <= size; ++i) > luamp_encode(port->L, cfg, &stream, i); > port->size = size; > -- > 2.17.1 >