From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 3F2192F09A for ; Thu, 15 Nov 2018 07:09:24 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xbdWcP07_qIO for ; Thu, 15 Nov 2018 07:09:24 -0500 (EST) Received: from smtp33.i.mail.ru (smtp33.i.mail.ru [94.100.177.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id F38932D7B5 for ; Thu, 15 Nov 2018 07:09:23 -0500 (EST) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v2 1/2] sql: proper check for index in sqlite3Insert() Date: Thu, 15 Nov 2018 15:09:22 +0300 Message-Id: <05acfbeed0d0a02a349aa1ceb5a44624ff333060.1542282499.git.imeevma@gmail.com> In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, v.shpilevoy@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