1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

MDEV-16376 ASAN: heap-use-after-free in gcol.innodb_virtual_debug

Browse Source 
After a failed ADD INDEX, dict_index_remove_from_cache_low()
could iterate the index fields and dereference a freed virtual
column object when trying to remove the index from the v_indexes
of the virtual column.

This regression was caused by a merge of
MDEV-16119 InnoDB lock->index refers to a freed object.

ha_innobase_inplace_ctx::clear_added_indexes(): Detach the
indexes of uncommitted indexes from virtual columns, so that
the iteration in dict_index_remove_from_cache_low() can be avoided.

ha_innobase::prepare_inplace_alter_table(): Ignore uncommitted
corrupted indexes when rejecting ALTER TABLE. (This minor bug was
revealed by the extension of the test case.)

dict_index_t::detach_columns(): Detach an index from virtual columns.
Invoked by both dict_index_remove_from_cache_low() and
ha_innobase_inplace_ctx::clear_added_indexes().

dict_col_t::detach(const dict_index_t& index): Detach an index from
a column.

dict_col_t::is_virtual(): Replaces dict_col_is_virtual().

dict_index_t::has_virtual(): Replaces dict_index_has_virtual().
This commit is contained in:
Marko Mäkelä 2018-06-04 15:55:37 +03:00
parent 5932a4e77d
commit 8dc70c862b
8 changed files with 85 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
mysql-test/suite/gcol/r/innodb_virtual_debug.result
View File

@ -64,11 +64,19 @@ INSERT INTO t VALUES (18, 1, DEFAULT, 'mm');
INSERT INTO t VALUES (28, 1, DEFAULT, 'mm');
INSERT INTO t VALUES (null, null, DEFAULT, 'mm');
CREATE INDEX idx_1 on t(c);
SET SESSION debug_dbug="+d,create_index_fail";
ALTER TABLE t ADD COLUMN x INT GENERATED ALWAYS AS(a+b), ADD INDEX idx (x);
SET @saved_dbug = @@SESSION.debug_dbug;
SET debug_dbug = '+d,create_index_fail';
ALTER TABLE t ADD COLUMN x INT GENERATED ALWAYS AS(a+b), ADD INDEX idx (x),
ADD INDEX idcx (c,x);
ERROR 23000: Duplicate entry '' for key '*UNKNOWN*'
SET SESSION debug_dbug="";
UPDATE t SET a=a+1;
affected rows: 3
info: Rows matched: 4 Changed: 3 Warnings: 0
ALTER TABLE t ADD INDEX idc(c);
ERROR 23000: Duplicate entry '' for key '*UNKNOWN*'
SET debug_dbug = @saved_dbug;
affected rows: 0
UPDATE t SET b=b-1;
SHOW CREATE TABLE t;
Table Create Table
t CREATE TABLE `t` (

15
mysql-test/suite/gcol/t/innodb_virtual_debug.test
View File

@ -119,14 +119,23 @@ INSERT INTO t VALUES (null, null, DEFAULT, 'mm');
CREATE INDEX idx_1 on t(c);
SET SESSION debug_dbug="+d,create_index_fail";
SET @saved_dbug = @@SESSION.debug_dbug;
SET debug_dbug = '+d,create_index_fail';
--enable_info
--error ER_DUP_ENTRY
ALTER TABLE t ADD COLUMN x INT GENERATED ALWAYS AS(a+b), ADD INDEX idx (x);
SET SESSION debug_dbug="";
ALTER TABLE t ADD COLUMN x INT GENERATED ALWAYS AS(a+b), ADD INDEX idx (x),
ADD INDEX idcx (c,x);
UPDATE t SET a=a+1;
--error ER_DUP_ENTRY
ALTER TABLE t ADD INDEX idc(c);
SET debug_dbug = @saved_dbug;
--disable_info
UPDATE t SET b=b-1;
SHOW CREATE TABLE t;
SELECT c FROM t;

32
storage/innobase/dict/dict0dict.cc
View File

@ -2676,37 +2676,7 @@ dict_index_remove_from_cache_low(
UT_LIST_REMOVE(table->indexes, index);
/* Remove the index from affected virtual column index list */
if (dict_index_has_virtual(index)) {
const dict_col_t* col;
const dict_v_col_t* vcol;
for (ulint i = 0; i < dict_index_get_n_fields(index); i++) {
col = dict_index_get_nth_col(index, i);
if (dict_col_is_virtual(col)) {
vcol = reinterpret_cast<const dict_v_col_t*>(
col);
/* This could be NULL, when we do add virtual
column, add index together. We do not need to
track this virtual column's index */
if (vcol->v_indexes == NULL) {
continue;
}
dict_v_idx_list::iterator it;
for (it = vcol->v_indexes->begin();
it != vcol->v_indexes->end(); ++it) {
dict_v_idx_t v_index = *it;
if (v_index.index == index) {
vcol->v_indexes->erase(it);
break;
}
}
}
}
}
index->detach_columns();
dict_mem_index_free(index);
}

14
storage/innobase/handler/handler0alter.cc
View File

@ -245,6 +245,16 @@ struct ha_innobase_inplace_ctx : public inplace_alter_handler_ctx
@return whether the table will be rebuilt */
bool need_rebuild () const { return(old_table != new_table); }
/** Clear uncommmitted added indexes after a failed operation. */
void clear_added_indexes()
{
for (ulint i = 0; i < num_to_add_index; i++) {
if (!add_index[i]->is_committed()) {
add_index[i]->detach_columns();
}
}
}
private:
// Disable copying
ha_innobase_inplace_ctx(const ha_innobase_inplace_ctx&);
@ -5996,7 +6006,8 @@ check_if_can_drop_indexes:
for (dict_index_t* index = dict_table_get_first_index(indexed_table);
index != NULL; index = dict_table_get_next_index(index)) {
if (!index->to_be_dropped && index->is_corrupted()) {
if (!index->to_be_dropped && index->is_committed()
&& index->is_corrupted()) {
my_error(ER_INDEX_CORRUPT, MYF(0), index->name());
goto err_exit;
}
@ -6567,6 +6578,7 @@ oom:
that we hold at most a shared lock on the table. */
m_prebuilt->trx->error_info = NULL;
ctx->trx->error_state = DB_SUCCESS;
ctx->clear_added_indexes();
DBUG_RETURN(true);
}

19
storage/innobase/include/dict0dict.h
View File

@ -750,13 +750,9 @@ dict_index_is_spatial(
/*==================*/
const dict_index_t* index) /*!< in: index */
MY_ATTRIBUTE((warn_unused_result));
/** Check whether the index contains a virtual column.
@param[in] index index
@return nonzero for index on virtual column, zero for other indexes */
UNIV_INLINE
ulint
dict_index_has_virtual(
const dict_index_t* index);
#define dict_index_has_virtual(index) (index)->has_virtual()
/********************************************************************//**
Check whether the index is the insert buffer tree.
@return nonzero for insert buffer, zero for other indexes */
@ -1964,13 +1960,8 @@ dict_index_node_ptr_max_size(
/*=========================*/
const dict_index_t* index) /*!< in: index */
MY_ATTRIBUTE((warn_unused_result));
/** Check if a column is a virtual column
@param[in] col column
@return true if it is a virtual column, false otherwise */
UNIV_INLINE
bool
dict_col_is_virtual(
const dict_col_t* col);
#define dict_col_is_virtual(col) (col)->is_virtual()
/** encode number of columns and number of virtual columns in one
4 bytes value. We could do this because the number of columns in

24
storage/innobase/include/dict0dict.ic
View File

@ -72,16 +72,6 @@ dict_col_copy_type(
type->mbminlen = col->mbminlen;
type->mbmaxlen = col->mbmaxlen;
}
/** Check if a column is a virtual column
@param[in] col column
@return true if it is a virtual column, false otherwise */
UNIV_INLINE
bool
dict_col_is_virtual(
const dict_col_t* col)
{
return(col->prtype & DATA_VIRTUAL);
}
#ifdef UNIV_DEBUG
/*********************************************************************//**
@ -325,20 +315,6 @@ dict_index_is_spatial(
return(index->type & DICT_SPATIAL);
}
/** Check whether the index contains a virtual column
@param[in] index index
@return nonzero for the index has virtual column, zero for other indexes */
UNIV_INLINE
ulint
dict_index_has_virtual(
const dict_index_t* index)
{
ut_ad(index);
ut_ad(index->magic_n == DICT_INDEX_MAGIC_N);
return(index->type & DICT_VIRTUAL);
}
/********************************************************************//**
Check whether the index is the insert buffer tree.
@return nonzero for insert buffer, zero for other indexes */

42
storage/innobase/include/dict0mem.h
View File

@ -612,6 +612,13 @@ struct dict_col_t{
unsigned max_prefix:12; /*!< maximum index prefix length on
this column. Our current max limit is
3072 for Barracuda table */
/** @return whether this is a virtual column */
bool is_virtual() const { return prtype & DATA_VIRTUAL; }
/** Detach the column from an index.
@param[in] index index to be detached from */
inline void detach(const dict_index_t& index);
};
/** Index information put in a list of virtual column structure. Index
@ -981,10 +988,45 @@ struct dict_index_t{
return DICT_CLUSTERED == (type & (DICT_CLUSTERED | DICT_IBUF));
}
/** @return whether the index includes virtual columns */
bool has_virtual() const { return type & DICT_VIRTUAL; }
/** @return whether the index is corrupted */
inline bool is_corrupted() const;
/** Detach the columns from the index that is to be freed. */
void detach_columns()
{
if (has_virtual()) {
for (unsigned i = 0; i < n_fields; i++) {
fields[i].col->detach(*this);
}
n_fields = 0;
}
}
};
/** Detach a column from an index.
@param[in] index index to be detached from */
inline void dict_col_t::detach(const dict_index_t& index)
{
if (!is_virtual()) {
return;
}
if (dict_v_idx_list* v_indexes = reinterpret_cast<const dict_v_col_t*>
(this)->v_indexes) {
for (dict_v_idx_list::iterator i = v_indexes->begin();
i != v_indexes->end(); i++) {
if (i->index == &index) {
v_indexes->erase(i);
return;
}
}
}
}
/** The status of online index creation */
enum online_index_status {
/** the index is complete and ready for access */

2
storage/innobase/row/row0upd.cc
View File

@ -593,7 +593,7 @@ row_upd_changes_field_size_or_external(
/* We should ignore virtual field if the index is not
a virtual index */
if (upd_fld_is_virtual_col(upd_field)
&& dict_index_has_virtual(index) != DICT_VIRTUAL) {
&& !index->has_virtual()) {
continue;
}