Commit 12614af1 authored by Nikita Malyavin's avatar Nikita Malyavin

MDEV-17005 ASAN heap-use-after-free in innobase_get_computed_value

This is the race between DELETE and INSERT (or other any two operations accessing to the table).
What should happen in good case:
1. ALTER TABLE is issued. vc_templ->default_rec is initialized with temporary share's default_fields
2. temporary share is freed, but datadict is still there, with garbage in vc_templ->default_rec
3. DELETE is issued. It is first after ALTER TABLE finished.
4. ha_innobase::open() is called, ib_table->get_ref_count() should be one
5. we reinitialize vc_templ, so no garbage anymore

What actually happens:
3. DELETE is issued.
4. ha_innobase::open() is called and ib_table->get_ref_count() is 1
5. INSERT (or SELECT etc.) is issued in parallel
6. ha_innobase::open() is called and ib_table->get_ref_count() is 1
7. we check ib_table->get_ref_count()  and it is 2 in both threads when we want reinitialize vc_templ
8. garbage is there

Fix:
* Do not store pointers to SHARE memory in table dict, copy it instead.
* But then we don't need to refresh it each time when refcount=1.
parent b0b54852
...@@ -5951,7 +5951,8 @@ innobase_build_v_templ( ...@@ -5951,7 +5951,8 @@ innobase_build_v_templ(
s_templ->n_col = ncol; s_templ->n_col = ncol;
s_templ->n_v_col = n_v_col; s_templ->n_v_col = n_v_col;
s_templ->rec_len = table->s->reclength; s_templ->rec_len = table->s->reclength;
s_templ->default_rec = table->s->default_values; s_templ->default_rec = UT_NEW_ARRAY_NOKEY(uchar, s_templ->rec_len);
memcpy(s_templ->default_rec, table->s->default_values, s_templ->rec_len);
/* Mark those columns could be base columns */ /* Mark those columns could be base columns */
for (ulint i = 0; i < ib_table->n_v_cols; i++) { for (ulint i = 0; i < ib_table->n_v_cols; i++) {
...@@ -6400,14 +6401,6 @@ ha_innobase::open(const char* name, int, uint) ...@@ -6400,14 +6401,6 @@ ha_innobase::open(const char* name, int, uint)
mutex_enter(&dict_sys->mutex); mutex_enter(&dict_sys->mutex);
if (ib_table->vc_templ == NULL) { if (ib_table->vc_templ == NULL) {
ib_table->vc_templ = UT_NEW_NOKEY(dict_vcol_templ_t()); ib_table->vc_templ = UT_NEW_NOKEY(dict_vcol_templ_t());
} else if (ib_table->get_ref_count() == 1) {
/* Clean and refresh the template if no one else
get hold on it */
dict_free_vc_templ(ib_table->vc_templ);
ib_table->vc_templ->vtempl = NULL;
}
if (ib_table->vc_templ->vtempl == NULL) {
innobase_build_v_templ( innobase_build_v_templ(
table, ib_table, ib_table->vc_templ, NULL, table, ib_table, ib_table->vc_templ, NULL,
true); true);
......
...@@ -1505,6 +1505,9 @@ void ...@@ -1505,6 +1505,9 @@ void
dict_free_vc_templ( dict_free_vc_templ(
dict_vcol_templ_t* vc_templ) dict_vcol_templ_t* vc_templ)
{ {
UT_DELETE_ARRAY(vc_templ->default_rec);
vc_templ->default_rec = NULL;
if (vc_templ->vtempl != NULL) { if (vc_templ->vtempl != NULL) {
ut_ad(vc_templ->n_v_col > 0); ut_ad(vc_templ->n_v_col > 0);
for (ulint i = 0; i < vc_templ->n_col for (ulint i = 0; i < vc_templ->n_col
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment