From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (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 AFE89445320 for ; Wed, 22 Jul 2020 11:59:57 +0300 (MSK) Date: Wed, 22 Jul 2020 08:59:56 +0000 From: Nikita Pettik Message-ID: <20200722085956.GA25532@tarantool.org> References: <20200721223242.24467-1-i.kosarev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200721223242.24467-1-i.kosarev@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v3] lua: assert in lua_gettop() in case of negative stack size 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 22 Jul 01:32, Ilya Kosarev wrote: > (gdb) p port->vtab->dump_msgpack > count = port_dump_msgpack_16(&port, out); > else > count = port_dump_msgpack(&port, out); > port_destroy(&port); > if (count < 0) { > obuf_rollback_to_svp(out, &svp); > goto error; > } > > iproto_reply_select(out, &svp, msg->header.sync, > ::schema_version, count); > iproto_wpos_create(&msg->wpos, out); > return; > error: > tx_reply_error(msg); > } > > As we can see, we fail to the error path through count == -1. It comes > from port_lua_dump() calling port_lua_do_dump() with encode_lua_call(). > > static int > port_lua_dump(struct port *base, struct obuf *out) > { > return port_lua_do_dump(base, out, encode_lua_call); > } > > encode_lua_call() actually sets port->size equal to lua_gettop(), being > returned by port_lua_do_dump(). As a first assumption it is OK. But there might be other reasons for negative value in size var. For instance, out-of-bound memory access in luamp_encode() which overwrites value of stack var.. > src/lj_api.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/lj_api.c b/src/lj_api.c > index c9b5d22..5ca2c57 100644 > --- a/src/lj_api.c > +++ b/src/lj_api.c > @@ -135,7 +135,12 @@ LUA_API const lua_Number *lua_version(lua_State *L) > > LUA_API int lua_gettop(lua_State *L) > { > - return (int)(L->top - L->base); > + int stack_size = (int)(L->top - L->base); > + /* > + * Stack size has to be non-negative according to Lua reference manual. > + */ > + assert(stack_size >= 0); > + return stack_size; > } > > LUA_API void lua_settop(lua_State *L, int idx) > -- > 2.17.1 >