[tarantool-patches] [PATCH v2 1/2] sql: proper check for index in sqlite3Insert()
imeevma at tarantool.org
imeevma at tarantool.org
Thu Nov 15 15:09:22 MSK 2018
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
More information about the Tarantool-patches
mailing list