From 12614af1fe14a045f09dedaced7e2d4c4caf7bf4 Mon Sep 17 00:00:00 2001 From: Nikita Malyavin Date: Mon, 15 Jul 2019 20:30:45 +1000 Subject: [PATCH] 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. --- storage/innobase/handler/ha_innodb.cc | 11 ++--------- 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