From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp18.mail.ru (smtp18.mail.ru [94.100.176.155]) (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 22B99445320 for ; Wed, 22 Jul 2020 01:32:46 +0300 (MSK) From: Ilya Kosarev Date: Wed, 22 Jul 2020 01:32:42 +0300 Message-Id: <20200721223242.24467-1-i.kosarev@tarantool.org> Subject: [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: imun@tarantool.org Cc: tarantool-patches@dev.tarantool.org According to user report (details provided in tarantool/tarantool#4649) lua_gettop() returns negative value in encode_lua_call(). Such behavior is not expected and violates Lua reference manual. Thus corresponding sane check with assert is introduced. Relates to tarantool/tarantool#4649 --- Branch: https://github.com/tarantool/luajit/tree/i.kosarev/gh-4649-assert-in-lua_gettop Issue: https://github.com/tarantool/tarantool/issues/4649 Changes in v2: - added reasoning in comment Changes in v3: - moved assert to LuaJIT itself (gdb) bt #0 0x00007f0012bbf387 in raise () from .../lib64/libc.so.6 #1 0x00007f0012bc0a78 in abort () from .../lib64/libc.so.6 #2 0x000000000041108e in sig_fatal_cb (signo=11, siginfo=0x7ef54107f8f0, context=0x7ef54107f7c0) at .../src/main.cc:258 #3 #4 0x00007f0012cf8691 in __strlen_sse2_pminub () from .../lib64/libc.so.6 #5 0x000000000061b18c in iproto_reply_error (out=0x7f00098692e8, e=0x0, sync=54211898, schema_version=495) at .../src/box/xrow.c:434 #6 0x00000000004168c2 in tx_reply_error (msg=0x7f0009816d80) at .../src/box/iproto.cc:1465 #7 0x0000000000416ec6 in tx_process_call (m=0x7f0009816d80) at .../src/box/iproto.cc:1649 #8 0x0000000000520e5a in cmsg_deliver (msg=0x7f0009816d80) at .../src/cbus.c:375 #9 0x0000000000521e34 in fiber_pool_f (ap=0x7ef5448c1728) at .../src/fiber_pool.c:64 #10 0x0000000000410a7a in fiber_cxx_invoke(fiber_func, typedef __va_list_tag __va_list_tag *) (f=0x521ca1 , ap=0x7ef5448c1728) at .../src/fiber.h:678 #11 0x000000000051af4e in fiber_loop (data=0x0) at .../src/fiber.c:713 #12 0x00000000006abae3 in coro_init () at .../third_party/coro/coro.c:110 (gdb) fr 7 #7 0x0000000000416ec6 in tx_process_call (m=0x7f0009816d80) at .../src/box/iproto.cc:1649 1649 in .../src/box/iproto.cc (gdb) p rc $29 = 0 (gdb) p count $30 = -1 (gdb) p msg->header.type $31 = 10 (gdb) p port->vtab->dump_msgpack $32 = (int (*)(port *, obuf *)) 0x4e759f static void tx_process_call(struct cmsg *m) { struct iproto_msg *msg = tx_accept_msg(m); if (tx_check_schema(msg->header.schema_version)) goto error; /* * CALL/EVAL should copy its arguments so we can discard * input on yield to avoid stalling other connections by * a long polling request. */ struct trigger fiber_on_yield; trigger_create(&fiber_on_yield, tx_process_call_on_yield, msg, NULL); trigger_add(&fiber()->on_yield, &fiber_on_yield); int rc; struct port port; switch (msg->header.type) { case IPROTO_CALL: case IPROTO_CALL_16: rc = box_process_call(&msg->call, &port); break; case IPROTO_EVAL: rc = box_process_eval(&msg->call, &port); break; default: unreachable(); } trigger_clear(&fiber_on_yield); if (rc != 0) goto error; /* * Add all elements returned by the function to iproto. * * To allow clients to understand a complex return from * a procedure, we are compatible with SELECT protocol, * and return the number of return values first, and * then each return value as a tuple. * * (!) Please note that a save point for output buffer * must be taken only after finishing executing of Lua * function because Lua can yield and leave the * buffer in inconsistent state (a parallel request * from the same connection will break the protocol). */ int count; struct obuf *out; struct obuf_svp svp; out = msg->connection->tx.p_obuf; if (iproto_prepare_select(out, &svp) != 0) { port_destroy(&port); goto error; } if (msg->header.type == IPROTO_CALL_16) 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(). 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