diff options
| author | Nikita Malyavin <nikitamalyavin@gmail.com> | 2019-07-15 20:30:45 +1000 |
|---|---|---|
| committer | Nikita Malyavin <nikitamalyavin@gmail.com> | 2019-07-22 20:29:42 +1000 |
| commit | 12614af1fe14a045f09dedaced7e2d4c4caf7bf4 (patch) | |
| tree | e1b5f0ba548598e9d8ea48a2161597bede61f52d | |
| parent | b0b54852514e5ae2d5816c3217d70f65384c9920 (diff) | |
| download | mariadb-git-12614af1fe14a045f09dedaced7e2d4c4caf7bf4.tar.gz | |
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.
| -rw-r--r-- | storage/innobase/handler/ha_innodb.cc | 11 | ||||
| -rw-r--r-- | storage/innobase/include/dict0dict.ic | 3 |
2 files changed, 5 insertions, 9 deletions
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index a598dbaefb9..be86f07cc17 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -5951,7 +5951,8 @@ innobase_build_v_templ( s_templ->n_col = ncol; s_templ->n_v_col = n_v_col; 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 */ for (ulint i = 0; i < ib_table->n_v_cols; i++) { @@ -6400,14 +6401,6 @@ no_such_table: mutex_enter(&dict_sys->mutex); if (ib_table->vc_templ == NULL) { 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( table, ib_table, ib_table->vc_templ, NULL, true); diff --git a/storage/innobase/include/dict0dict.ic b/storage/innobase/include/dict0dict.ic index 2af31f133d0..bb77bb7e6e6 100644 --- a/storage/innobase/include/dict0dict.ic +++ b/storage/innobase/include/dict0dict.ic @@ -1505,6 +1505,9 @@ void dict_free_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) { ut_ad(vc_templ->n_v_col > 0); for (ulint i = 0; i < vc_templ->n_col |