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

[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
>