[tarantool-patches] [PATCH v2 1/2] sql: proper check for index in sqlite3Insert()

[imeevma@tarantool.org]

Index received in function sqlite3Insert() wasn't checked
properly. It lead to segmentation fault when INSERT and
DROP TABLE executed simultaneously for the same table.

Closes #3780
---
 src/box/sql/insert.c     | 49 ++++++++++++++++++++++++------------------------
 test/sql/errinj.result   | 33 ++++++++++++++++++++++++++++++++
 test/sql/errinj.test.lua | 12 ++++++++++++
 3 files changed, 69 insertions(+), 25 deletions(-)

diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index fd05c02..01bffa4 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -117,14 +117,13 @@ sql_space_autoinc_fieldno(struct space *space)
  * SELECT.
  *
  * @param parser Parse context.
- * @param table Table AST object.
- * @retval  true if the table table in database or any of its
- *          indices have been opened at any point in the VDBE
- *          program.
- * @retval  false else.
+ * @param space Space to check read of.
+ * @retval true If the space or any of its indices have been
+ *         opened at any point in the VDBE program.
+ * @retval false Else.
  */
 static bool
-vdbe_has_table_read(struct Parse *parser, const struct Table *table)
+vdbe_has_table_read(struct Parse *parser, const struct space *space)
 {
 	struct Vdbe *v = sqlite3GetVdbe(parser);
 	int last_instr = sqlite3VdbeCurrentAddr(v);
@@ -136,12 +135,12 @@ vdbe_has_table_read(struct Parse *parser, const struct Table *table)
 		 * and Write cursors.
 		 */
 		if (op->opcode == OP_IteratorOpen) {
-			struct space *space = NULL;
+			struct space *space_p4 = NULL;
 			if (op->p4type == P4_SPACEPTR)
-				space = op->p4.space;
+				space_p4 = op->p4.space;
 			else
 				continue;
-			if (space->def->id == table->def->id)
+			if (space_p4->def->id == space->def->id)
 				return true;
 		}
 	}
@@ -315,24 +314,21 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 	if (pTab == NULL)
 		goto insert_cleanup;
 
-	space_id = pTab->def->id;
+	struct space *space = pTab->space;
+	struct space_def *def = space->def;
+	space_id = def->id;
 
 	/* Figure out if we have any triggers and if the table being
 	 * inserted into is a view
 	 */
 	trigger = sql_triggers_exist(pTab, TK_INSERT, NULL, &tmask);
-	bool is_view = pTab->def->opts.is_view;
+	bool is_view = def->opts.is_view;
 	assert((trigger != NULL && tmask != 0) ||
 	       (trigger == NULL && tmask == 0));
 
-	/* If pTab is really a view, make sure it has been initialized.
-	 * ViewGetColumnNames() is a no-op if pTab is not a view.
-	 */
-	if (is_view &&
-	    sql_view_assign_cursors(pParse, pTab->def->opts.sql) != 0)
+	if (is_view && sql_view_assign_cursors(pParse, def->opts.sql) != 0)
 		goto insert_cleanup;
 
-	struct space_def *def = pTab->def;
 	/* Cannot insert into a read-only table. */
 	if (is_view && tmask == 0) {
 		sqlite3ErrorMsg(pParse, "cannot modify %s because it is a view",
@@ -340,6 +336,12 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 		goto insert_cleanup;
 	}
 
+	if (! is_view && index_find(space, 0) == NULL) {
+		pParse->nErr++;
+		pParse->rc = SQL_TARANTOOL_ERROR;
+		goto insert_cleanup;
+	}
+
 	/* Allocate a VDBE. */
 	v = sqlite3GetVdbe(pParse);
 	if (v == NULL)
@@ -358,7 +360,7 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 	 * This is the 2nd template.
 	 */
 	if (pColumn == 0 &&
-	    xferOptimization(pParse, pTab->space, pSelect, on_error)) {
+	    xferOptimization(pParse, space, pSelect, on_error)) {
 		assert(trigger == NULL);
 		assert(pList == 0);
 		goto insert_end;
@@ -459,7 +461,7 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 		 * the SELECT statement. Also use a temp table in
 		 * the case of row triggers.
 		 */
-		if (trigger != NULL || vdbe_has_table_read(pParse, pTab))
+		if (trigger != NULL || vdbe_has_table_read(pParse, space))
 			useTempTable = 1;
 
 		if (useTempTable) {
@@ -576,8 +578,6 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 		    sqlite3VdbeAddOp1(v, OP_Yield, dest.iSDParm);
 		VdbeCoverage(v);
 	}
-	struct space *space = space_by_id(pTab->def->id);
-	assert(space != NULL);
 	uint32_t autoinc_fieldno = sql_space_autoinc_fieldno(space);
 	/* Run the BEFORE and INSTEAD OF triggers, if there are any
 	 */
@@ -627,7 +627,7 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 		 * table column affinities.
 		 */
 		if (!is_view)
-			sql_emit_table_affinity(v, pTab->def, regCols + 1);
+			sql_emit_table_affinity(v, def, regCols + 1);
 
 		/* Fire BEFORE or INSTEAD OF triggers */
 		vdbe_code_row_trigger(pParse, trigger, TK_INSERT, 0,
@@ -662,8 +662,7 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 				if (i == (int) autoinc_fieldno) {
 					sqlite3VdbeAddOp2(v,
 							  OP_NextAutoincValue,
-							  pTab->def->id,
-							  iRegStore);
+							  space_id, iRegStore);
 					continue;
 				}
 				struct Expr *dflt = NULL;
@@ -773,7 +772,7 @@ sqlite3Insert(Parse * pParse,	/* Parser context */
 					    on_error, endOfLoop, 0);
 		fkey_emit_check(pParse, pTab, 0, regIns, 0);
 		vdbe_emit_insertion_completion(v, space, regIns + 1,
-					       pTab->def->field_count,
+					       def->field_count,
 					       on_error);
 	}
 
diff --git a/test/sql/errinj.result b/test/sql/errinj.result
index cb993f8..beceafb 100644
--- a/test/sql/errinj.result
+++ b/test/sql/errinj.result
@@ -280,3 +280,36 @@ errinj.set("ERRINJ_WAL_IO", false)
 box.sql.execute("DROP TABLE t3;")
 ---
 ...
+-- gh-3780: Segmentation fault with two users changing the same
+-- SQL table
+box.sql.execute('create table test (id int primary key)')
+---
+...
+errinj.set("ERRINJ_WAL_DELAY", true)
+---
+- ok
+...
+function execute_yield_drop_table() box.sql.execute("drop table test") end
+---
+...
+f1 = fiber.create(execute_yield_drop_table)
+---
+...
+while f1:status() ~= 'suspended' do fiber.sleep(0) end
+---
+...
+box.sql.execute("insert into test values (1)")
+---
+- error: 'No index #0 is defined in space ''TEST'''
+...
+errinj.set("ERRINJ_WAL_DELAY", false)
+---
+- ok
+...
+while f1:status() ~= 'dead' do fiber.sleep(0) end
+---
+...
+box.sql.execute("drop table test")
+---
+- error: 'no such table: TEST'
+...
diff --git a/test/sql/errinj.test.lua b/test/sql/errinj.test.lua
index fa7f9f2..a66a812 100644
--- a/test/sql/errinj.test.lua
+++ b/test/sql/errinj.test.lua
@@ -97,3 +97,15 @@ box.sql.execute("ALTER TABLE t3 DROP CONSTRAINT fk1;")
 box.sql.execute("INSERT INTO t3 VALUES(1, 1, 3);")
 errinj.set("ERRINJ_WAL_IO", false)
 box.sql.execute("DROP TABLE t3;")
+
+-- gh-3780: Segmentation fault with two users changing the same
+-- SQL table
+box.sql.execute('create table test (id int primary key)')
+errinj.set("ERRINJ_WAL_DELAY", true)
+function execute_yield_drop_table() box.sql.execute("drop table test") end
+f1 = fiber.create(execute_yield_drop_table)
+while f1:status() ~= 'suspended' do fiber.sleep(0) end
+box.sql.execute("insert into test values (1)")
+errinj.set("ERRINJ_WAL_DELAY", false)
+while f1:status() ~= 'dead' do fiber.sleep(0) end
+box.sql.execute("drop table test")
-- 
2.7.4