Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: korablev@tarantool.org, Mergen Imeev <imeevma@gmail.com>
Subject: [tarantool-patches] [PATCH 1/3] sql: EXPLAIN through net.box leads to SEGFAULT
Date: Mon, 19 Nov 2018 16:47:28 +0300	[thread overview]
Message-ID: <a9478b925eb4ec63524c1e8449c18349339e4d45.1542635129.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1542635129.git.v.shpilevoy@tarantool.org>
In-Reply-To: <cover.1542635129.git.v.shpilevoy@tarantool.org>

From: Mergen Imeev <imeevma@gmail.com>

EXPLAIN envokes SEGMENTATION FAULT when being executed through
net.box. It happens due to column type of the result of this
function being NULL.
---
 src/box/execute.c        | 22 ++++++++++++++--------
 src/box/lua/net_box.c    | 15 ++++++++-------
 test/sql/iproto.result   | 15 +++++++++++++++
 test/sql/iproto.test.lua | 13 ++++++++++++-
 4 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index e45044488..5b747cf7f 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -529,9 +529,6 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
 		return -1;
 
 	for (int i = 0; i < column_count; ++i) {
-		size_t size = mp_sizeof_map(2) +
-			      mp_sizeof_uint(IPROTO_FIELD_NAME) +
-			      mp_sizeof_uint(IPROTO_FIELD_TYPE);
 		const char *name = sqlite3_column_name(stmt, i);
 		const char *type = sqlite3_column_datatype(stmt, i);
 		/*
@@ -540,18 +537,27 @@ sql_get_description(struct sqlite3_stmt *stmt, struct obuf *out,
 		 * column_name simply returns them.
 		 */
 		assert(name != NULL);
-		size += mp_sizeof_str(strlen(name));
-		size += mp_sizeof_str(strlen(type));
+		size_t size = mp_sizeof_uint(IPROTO_FIELD_NAME) +
+			      mp_sizeof_str(strlen(name));
+		if (type != NULL) {
+			size += mp_sizeof_map(2) +
+				mp_sizeof_uint(IPROTO_FIELD_TYPE) +
+				mp_sizeof_str(strlen(type));
+		} else {
+			size += mp_sizeof_map(1);
+		}
 		char *pos = (char *) obuf_alloc(out, size);
 		if (pos == NULL) {
 			diag_set(OutOfMemory, size, "obuf_alloc", "pos");
 			return -1;
 		}
-		pos = mp_encode_map(pos, 2);
+		pos = mp_encode_map(pos, 1 + (type != NULL));
 		pos = mp_encode_uint(pos, IPROTO_FIELD_NAME);
 		pos = mp_encode_str(pos, name, strlen(name));
-		pos = mp_encode_uint(pos, IPROTO_FIELD_TYPE);
-		pos = mp_encode_str(pos, type, strlen(type));
+		if (type != NULL) {
+			pos = mp_encode_uint(pos, IPROTO_FIELD_TYPE);
+			pos = mp_encode_str(pos, type, strlen(type));
+		}
 	}
 	return 0;
 }
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index c7063d9c8..342c6a768 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -644,8 +644,7 @@ netbox_decode_metadata(struct lua_State *L, const char **data)
 	lua_createtable(L, count, 0);
 	for (uint32_t i = 0; i < count; ++i) {
 		uint32_t map_size = mp_decode_map(data);
-		assert(map_size == 2);
-		(void) map_size;
+		assert(map_size == 1 || map_size == 2);
 		uint32_t key = mp_decode_uint(data);
 		assert(key == IPROTO_FIELD_NAME);
 		(void) key;
@@ -654,11 +653,13 @@ netbox_decode_metadata(struct lua_State *L, const char **data)
 		const char *str = mp_decode_str(data, &len);
 		lua_pushlstring(L, str, len);
 		lua_setfield(L, -2, "name");
-		key = mp_decode_uint(data);
-		assert(key == IPROTO_FIELD_TYPE);
-		const char *type = mp_decode_str(data, &len);
-		lua_pushlstring(L, type, len);
-		lua_setfield(L, -2, "type");
+		if (map_size == 2) {
+			key = mp_decode_uint(data);
+			assert(key == IPROTO_FIELD_TYPE);
+			const char *type = mp_decode_str(data, &len);
+			lua_pushlstring(L, type, len);
+			lua_setfield(L, -2, "type");
+		}
 		lua_rawseti(L, -2, i + 1);
 	}
 }
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index d077ee861..31e5d2e75 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -773,6 +773,21 @@ cn:execute('drop table test')
 cn:close()
 ---
 ...
+--
+-- EXPLAIN could lead to segfault because it does not send column
+-- data type, but looks like DQL. Netbox had been trying to decode
+-- type for any DQL.
+--
+cn = remote.connect(box.cfg.listen)
+---
+...
+-- Segmentation fault when EXPLAIN executed using net.box.
+_ = cn:execute("EXPLAIN SELECT 1;")
+---
+...
+cn:close()
+---
+...
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 ---
 ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 1470edad7..a0f0f15ee 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -247,10 +247,21 @@ cn:execute('drop table test')
 
 cn:close()
 
+--
+-- EXPLAIN could lead to segfault because it does not send column
+-- data type, but looks like DQL. Netbox had been trying to decode
+-- type for any DQL.
+--
+cn = remote.connect(box.cfg.listen)
+
+-- Segmentation fault when EXPLAIN executed using net.box.
+_ = cn:execute("EXPLAIN SELECT 1;")
+
+cn:close()
+
 box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 box.schema.user.revoke('guest', 'create', 'space')
 space = nil
 
 -- Cleanup xlog
 box.snapshot()
-
-- 
2.17.2 (Apple Git-113)

  reply	other threads:[~2018-11-19 13:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-19 13:47 [tarantool-patches] [PATCH 0/3] A set of small SQL fixes Vladislav Shpilevoy
2018-11-19 13:47 ` Vladislav Shpilevoy [this message]
2018-11-19 17:27   ` [tarantool-patches] Re: [PATCH 1/3] sql: EXPLAIN through net.box leads to SEGFAULT n.pettik
2018-11-22 18:08     ` Imeev Mergen
2018-11-22 18:31       ` Vladislav Shpilevoy
2018-11-22 20:55         ` [tarantool-patches] Re[2]: " Мерген Имеев
2018-11-23  6:04           ` [tarantool-patches] Re: Re[2]: " Kirill Yukhin
2018-11-28 12:36           ` [tarantool-patches] " n.pettik
2018-11-29 14:58             ` Konstantin Osipov
2018-11-19 13:47 ` [tarantool-patches] [PATCH 2/3] sql: SELECT from system spaces returns unpacked msgpack Vladislav Shpilevoy
2018-11-19 17:27   ` [tarantool-patches] " n.pettik
2018-11-22 18:09     ` Imeev Mergen
2018-11-27  8:51       ` n.pettik
2018-11-19 13:47 ` [tarantool-patches] [PATCH 3/3] sql: too many autogenerated ids leads to SEGFAULT Vladislav Shpilevoy
2018-11-19 17:27   ` [tarantool-patches] " n.pettik
2018-11-22 18:09     ` Imeev Mergen
2018-11-27  8:50       ` n.pettik
2018-11-27 10:09 ` [tarantool-patches] Re: [PATCH 0/3] A set of small SQL fixes Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a9478b925eb4ec63524c1e8449c18349339e4d45.1542635129.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=imeevma@gmail.com \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH 1/3] sql: EXPLAIN through net.box leads to SEGFAULT' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox