Skip to content
Permalink
Browse files
MDEV-29856 heap-use-after-poison in row_merge_spatial_rows() w/ colum… 
…n prefix

spatial_index_info: Replaces index_tuple_info_t. Always take
a memory heap as a parameter to the member functions.
Remove pointer indirection for m_dtuple_vec.

spatial_index_info::add(): Duplicate any PRIMARY KEY fields that would
point to within ext->buf because that buffer will be allocated in
a shorter-lifetime memory heap.
  • Loading branch information
@dr-m
dr-m committed Nov 8, 2022
1 parent b737d09 commit 2ef2e23
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 101 deletions.
10 mysql-test/suite/innodb_gis/r/alter_spatial_index.result
View file
@@ -794,4 +794,14 @@ ENGINE=InnoDB;
INSERT INTO t VALUES (REPEAT('MariaDB Corporation Ab ',351),POINT(0,0));
ALTER TABLE t FORCE;
DROP TABLE t;
#
# MDEV-29856 heap-use-after-poison in row_merge_spatial_rows()
# with PRIMARY KEY on column prefix
#
CREATE TABLE t (id INT, f TEXT, s POINT NOT NULL,
PRIMARY KEY(id,f(1)), SPATIAL(s)) ENGINE=InnoDB;
INSERT INTO t VALUES
(1,REPEAT('x',8192),@p:=ST_GeomFromText('POINT(0 0)')),(2,'',@p);
ALTER TABLE t FORCE;
DROP TABLE t;
# End of 10.3 tests
12 mysql-test/suite/innodb_gis/t/alter_spatial_index.test
View file
@@ -793,4 +793,16 @@ ALTER TABLE t FORCE;
# Cleanup
DROP TABLE t;

--echo #
--echo # MDEV-29856 heap-use-after-poison in row_merge_spatial_rows()
--echo # with PRIMARY KEY on column prefix
--echo #

CREATE TABLE t (id INT, f TEXT, s POINT NOT NULL,
PRIMARY KEY(id,f(1)), SPATIAL(s)) ENGINE=InnoDB;
INSERT INTO t VALUES
(1,REPEAT('x',8192),@p:=ST_GeomFromText('POINT(0 0)')),(2,'',@p);
ALTER TABLE t FORCE;
DROP TABLE t;

--echo # End of 10.3 tests
170 storage/innobase/row/row0merge.cc
View file
@@ -60,63 +60,48 @@ Completed by Sunny Bains and Marko Makela
/* Whether to disable file system cache */
char srv_disable_sort_file_cache;

/** Class that caches index row tuples made from a single cluster
/** Class that caches spatial index row tuples made from a single cluster
index page scan, and then insert into corresponding index tree */
class index_tuple_info_t {
class spatial_index_info {
public:
/** constructor
@param[in] heap memory heap
@param[in] index index to be created */
index_tuple_info_t(
mem_heap_t* heap,
dict_index_t* index) UNIV_NOTHROW
{
m_heap = heap;
m_index = index;
m_dtuple_vec = UT_NEW_NOKEY(idx_tuple_vec());
}

/** destructor */
~index_tuple_info_t()
{
UT_DELETE(m_dtuple_vec);
}

/** Get the index object
@return the index object */
dict_index_t* get_index() UNIV_NOTHROW
{
return(m_index);
}

/** Caches an index row into index tuple vector
@param[in] row table row
@param[in] ext externally stored column
prefixes, or NULL */
void add(
const dtuple_t* row,
const row_ext_t* ext) UNIV_NOTHROW
{
dtuple_t* dtuple;

dtuple = row_build_index_entry(row, ext, m_index, m_heap);

ut_ad(dtuple);

m_dtuple_vec->push_back(dtuple);
}
/** constructor
@param index spatial index to be created */
spatial_index_info(dict_index_t *index) : index(index)
{
ut_ad(index->is_spatial());
}

/** Caches an index row into index tuple vector
@param[in] row table row
@param[in] ext externally stored column prefixes, or NULL */
void add(const dtuple_t *row, const row_ext_t *ext, mem_heap_t *heap)
{
dtuple_t *dtuple= row_build_index_entry(row, ext, index, heap);
ut_ad(dtuple);
ut_ad(dtuple->n_fields == index->n_fields);
if (ext)
{
/* Replace any references to ext, because ext will be allocated
from row_heap. */
for (ulint i= 1; i < dtuple->n_fields; i++)
{
dfield_t &dfield= dtuple->fields[i];
if (dfield.data >= ext->buf &&
dfield.data <= &ext->buf[ext->n_ext * ext->max_len])
dfield_dup(&dfield, heap);
}
}
m_dtuple_vec.push_back(dtuple);
}

/** Insert spatial index rows cached in vector into spatial index
@param[in] trx_id transaction id
@param[in,out] row_heap memory heap
@param[in] pcur cluster index scanning cursor
@param[in,out] heap temporary memory heap
@param[in,out] scan_mtr mini-transaction for pcur
@return DB_SUCCESS if successful, else error number */
inline dberr_t insert(
trx_id_t trx_id,
mem_heap_t* row_heap,
btr_pcur_t* pcur,
mtr_t* scan_mtr)
inline dberr_t insert(trx_id_t trx_id, btr_pcur_t* pcur,
mem_heap_t* heap, mtr_t* scan_mtr)
{
big_rec_t* big_rec;
rec_t* rec;
@@ -130,14 +115,12 @@ class index_tuple_info_t {
| BTR_NO_LOCKING_FLAG
| BTR_KEEP_SYS_FLAG | BTR_CREATE_FLAG;

ut_ad(dict_index_is_spatial(m_index));

DBUG_EXECUTE_IF("row_merge_instrument_log_check_flush",
log_sys.check_flush_or_checkpoint = true;
);

for (idx_tuple_vec::iterator it = m_dtuple_vec->begin();
it != m_dtuple_vec->end();
for (idx_tuple_vec::iterator it = m_dtuple_vec.begin();
it != m_dtuple_vec.end();
++it) {
dtuple = *it;
ut_ad(dtuple);
@@ -153,14 +136,14 @@ class index_tuple_info_t {
}

mtr.start();
m_index->set_modified(mtr);
index->set_modified(mtr);

ins_cur.index = m_index;
rtr_init_rtr_info(&rtr_info, false, &ins_cur, m_index,
ins_cur.index = index;
rtr_init_rtr_info(&rtr_info, false, &ins_cur, index,
false);
rtr_info_update_btr(&ins_cur, &rtr_info);

btr_cur_search_to_nth_level(m_index, 0, dtuple,
btr_cur_search_to_nth_level(index, 0, dtuple,
PAGE_CUR_RTREE_INSERT,
BTR_MODIFY_LEAF, &ins_cur,
__FILE__, __LINE__,
@@ -169,44 +152,44 @@ class index_tuple_info_t {
/* It need to update MBR in parent entry,
so change search mode to BTR_MODIFY_TREE */
if (rtr_info.mbr_adj) {
mtr_commit(&mtr);
mtr.commit();
rtr_clean_rtr_info(&rtr_info, true);
rtr_init_rtr_info(&rtr_info, false, &ins_cur,
m_index, false);
index, false);
rtr_info_update_btr(&ins_cur, &rtr_info);
mtr_start(&mtr);
m_index->set_modified(mtr);
mtr.start();
index->set_modified(mtr);
btr_cur_search_to_nth_level(
m_index, 0, dtuple,
index, 0, dtuple,
PAGE_CUR_RTREE_INSERT,
BTR_MODIFY_TREE, &ins_cur,
__FILE__, __LINE__, &mtr);
}

error = btr_cur_optimistic_insert(
flag, &ins_cur, &ins_offsets, &row_heap,
flag, &ins_cur, &ins_offsets, &heap,
dtuple, &rec, &big_rec, 0, NULL, &mtr);

if (error == DB_FAIL) {
ut_ad(!big_rec);
mtr.commit();
mtr.start();
m_index->set_modified(mtr);
index->set_modified(mtr);

rtr_clean_rtr_info(&rtr_info, true);
rtr_init_rtr_info(&rtr_info, false,
&ins_cur, m_index, false);
&ins_cur, index, false);

rtr_info_update_btr(&ins_cur, &rtr_info);
btr_cur_search_to_nth_level(
m_index, 0, dtuple,
index, 0, dtuple,
PAGE_CUR_RTREE_INSERT,
BTR_MODIFY_TREE,
&ins_cur, __FILE__, __LINE__, &mtr);

error = btr_cur_pessimistic_insert(
flag, &ins_cur, &ins_offsets,
&row_heap, dtuple, &rec,
&heap, dtuple, &rec,
&big_rec, 0, NULL, &mtr);
}

@@ -229,30 +212,26 @@ class index_tuple_info_t {
}
}

mtr_commit(&mtr);
mtr.commit();

rtr_clean_rtr_info(&rtr_info, true);
}

m_dtuple_vec->clear();
m_dtuple_vec.clear();

return(error);
}

private:
/** Cache index rows made from a cluster index scan. Usually
for rows on single cluster index page */
typedef std::vector<dtuple_t*, ut_allocator<dtuple_t*> >
idx_tuple_vec;
/** Cache index rows made from a cluster index scan. Usually
for rows on single cluster index page */
typedef std::vector<dtuple_t*, ut_allocator<dtuple_t*> > idx_tuple_vec;

/** vector used to cache index rows made from cluster index scan */
idx_tuple_vec* m_dtuple_vec;

/** the index being built */
dict_index_t* m_index;

/** memory heap for creating index tuples */
mem_heap_t* m_heap;
/** vector used to cache index rows made from cluster index scan */
idx_tuple_vec m_dtuple_vec;
public:
/** the index being built */
dict_index_t*const index;
};

/* Maximum pending doc memory limit in bytes for a fts tokenization thread */
@@ -1566,7 +1545,6 @@ row_mtuple_cmp(
@param[in] trx_id transaction id
@param[in] sp_tuples cached spatial rows
@param[in] num_spatial number of spatial indexes
@param[in,out] row_heap heap for insert
@param[in,out] sp_heap heap for tuples
@param[in,out] pcur cluster index cursor
@param[in,out] mtr mini transaction
@@ -1575,9 +1553,8 @@ static
dberr_t
row_merge_spatial_rows(
trx_id_t trx_id,
index_tuple_info_t** sp_tuples,
spatial_index_info** sp_tuples,
ulint num_spatial,
mem_heap_t* row_heap,
mem_heap_t* sp_heap,
btr_pcur_t* pcur,
mtr_t* mtr)
@@ -1591,7 +1568,7 @@ row_merge_spatial_rows(
ut_ad(sp_heap != NULL);

for (ulint j = 0; j < num_spatial; j++) {
err = sp_tuples[j]->insert(trx_id, row_heap, pcur, mtr);
err = sp_tuples[j]->insert(trx_id, pcur, sp_heap, mtr);

if (err != DB_SUCCESS) {
return(err);
@@ -1714,8 +1691,7 @@ row_merge_read_clustered_index(
os_event_t fts_parallel_sort_event = NULL;
ibool fts_pll_sort = FALSE;
int64_t sig_count = 0;
index_tuple_info_t** sp_tuples = NULL;
mem_heap_t* sp_heap = NULL;
spatial_index_info** sp_tuples = NULL;
ulint num_spatial = 0;
BtrBulk* clust_btr_bulk = NULL;
bool clust_temp_file = false;
@@ -1805,19 +1781,15 @@ row_merge_read_clustered_index(
if (num_spatial > 0) {
ulint count = 0;

sp_heap = mem_heap_create(512);

sp_tuples = static_cast<index_tuple_info_t**>(
sp_tuples = static_cast<spatial_index_info**>(
ut_malloc_nokey(num_spatial
* sizeof(*sp_tuples)));

for (ulint i = 0; i < n_index; i++) {
if (dict_index_is_spatial(index[i])) {
sp_tuples[count]
= UT_NEW_NOKEY(
index_tuple_info_t(
sp_heap,
index[i]));
spatial_index_info(index[i]));
count++;
}
}
@@ -1948,7 +1920,7 @@ row_merge_read_clustered_index(
/* Insert the cached spatial index rows. */
err = row_merge_spatial_rows(
trx->id, sp_tuples, num_spatial,
row_heap, sp_heap, &pcur, &mtr);
row_heap, &pcur, &mtr);

if (err != DB_SUCCESS) {
goto func_exit;
@@ -2329,7 +2301,7 @@ row_merge_read_clustered_index(
continue;
}

ut_ad(sp_tuples[s_idx_cnt]->get_index()
ut_ad(sp_tuples[s_idx_cnt]->index
== buf->index);

/* If the geometry field is invalid, report
@@ -2339,7 +2311,7 @@ row_merge_read_clustered_index(
break;
}

sp_tuples[s_idx_cnt]->add(row, ext);
sp_tuples[s_idx_cnt]->add(row, ext, buf->heap);
s_idx_cnt++;

continue;
@@ -2469,7 +2441,7 @@ row_merge_read_clustered_index(
err = row_merge_spatial_rows(
trx->id, sp_tuples,
num_spatial,
row_heap, sp_heap,
row_heap,
&pcur, &mtr);

if (err != DB_SUCCESS) {
@@ -2847,10 +2819,6 @@ row_merge_read_clustered_index(
UT_DELETE(sp_tuples[i]);
}
ut_free(sp_tuples);

if (sp_heap) {
mem_heap_free(sp_heap);
}
}

/* Update the next Doc ID we used. Table should be locked, so

0 comments on commit 2ef2e23

Please sign in to comment.