Re: [Tarantool-patches] [PATCH v3] lua: assert in lua_gettop() in case of negative stack size

["Ilya Kosarev" <i.kosarev@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]


Hi!

Thanks to @igormunkin we discovered more details of this problem.
Turns out we can see lua_State stored in port and it seems that
something is totally wrong here: it points to some function address
(port_lua_dump_16()). Thus I will make some more research into it and
this patch should be discarded for now.
(gdb) p ((struct port_lua *)port)->vtab
$19 = (const port_vtab *) 0x4e759f <port_lua_dump>
(gdb) p ((struct port_lua *)port)->L
$20 = (lua_State *) 0x4e75c9 <port_lua_dump_16>
  
>Среда, 22 июля 2020, 11:59 +03:00 от Nikita Pettik <korablev@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
>> 
 
 
--
Ilya Kosarev
 

[-- Attachment #2: Type: text/html, Size: 3126 bytes --]