Skip to content

Commit

Permalink
[mysql-5.6][PR] Semisync histogram double free
Browse files Browse the repository at this point in the history
Summary:
Avoid double free on latency histogram data

Before this fix, rpl.rpl_semi_sync_alias test under ASan with

```
=================================================================
==65389==ERROR: AddressSanitizer: heap-use-after-free on address 0x0001742e17d4 at pc 0x000107febaf0 bp 0x00016ea8f710 sp 0x00016ea8f708
READ of size 4 at 0x0001742e17d4 thread T80
    #0 0x107febaec in my_free(void*) my_malloc.cc:135
    #1 0x103cb9828 in free_latency_histogram_sysvars(SHOW_VAR*) mysqld.cc:4668
    #2 0x103cb99bc in prepare_latency_histogram_vars(latency_histogram*, SHOW_VAR*, unsigned long long*) mysqld.cc:4692
    #3 0x17c65826c in rpl_semi_sync_master_trx_wait_histogram(THD*, SHOW_VAR*, char*) semisync_source_plugin.cc:581
    #4 0x10be1b4cc in PFS_status_variable_cache::manifest(THD*, SHOW_VAR const*, System_status_var*, char const*, bool, bool) pfs_variable.cc:1366
    #5 0x10be1ba90 in PFS_status_variable_cache::do_materialize_all(THD*) pfs_variable.cc:1172
    #6 0x10c0ab33c in PFS_variable_cache<Status_variable>::materialize_all(THD*) pfs_variable.h:536
    #7 0x10c0ab294 in table_session_status::rnd_init(bool) table_session_status.cc:111
    #8 0x10bceb790 in ha_perfschema::rnd_init(bool) ha_perfschema.cc:1686
    #9 0x1033c7cec in handler::ha_rnd_init(bool) handler.cc:3157
    #10 0x103975380 in TableScanIterator::Init() basic_row_iterators.cc:230
    #11 0x103a33a18 in FilterIterator::Init() composite_iterators.h:82
    #12 0x103982ec0 in MaterializeIterator::MaterializeQueryBlock(MaterializeIterator::QueryBlock const&, unsigned long long*) composite_iterators.cc:845
    #13 0x103981410 in MaterializeIterator::Init() composite_iterators.cc:660
    #14 0x1049fc518 in Query_expression::ExecuteIteratorQuery(THD*) sql_union.cc:1293
    #15 0x1049fd358 in Query_expression::execute(THD*) sql_union.cc:1355
    #16 0x1047ae7ac in Sql_cmd_dml::execute_inner(THD*) sql_select.cc:870
    #17 0x1047ac344 in Sql_cmd_dml::execute(THD*) sql_select.cc:618
    #18 0x1047ffcc8 in Sql_cmd_show::execute(THD*) sql_show.cc:232
    #19 0x10480ab58 in Sql_cmd_show_status::execute(THD*) sql_show.cc:894
    #20 0x1045cea6c in mysql_execute_command(THD*, bool, unsigned long long*) sql_parse.cc:5323
    #21 0x1045c5dcc in dispatch_sql_command(THD*, Parser_state*, unsigned long long*) sql_parse.cc:6093
    #22 0x1045bb92c in dispatch_command(THD*, COM_DATA const*, enum_server_command) sql_parse.cc:2444
    #23 0x1045c06f8 in do_command(THD*) sql_parse.cc:1636
    #24 0x104cc4cc4 in handle_connection(void*) connection_handler_per_thread.cc:307
    #25 0x10bd130d4 in pfs_spawn_thread(void*) pfs.cc:2983
    #26 0x18ad47fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4)
    #27 0x18ad42d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c)

0x0001742e17d4 is located 4 bytes inside of 40-byte region [0x0001742e17d0,0x0001742e17f8)
freed by thread T80 here:
    #0 0x139ff6de4 in wrap_free+0x98 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ede4)
    #1 0x107febcfc in my_raw_free(void*) my_malloc.cc:269
    #2 0x107feba48 in my_free(void*) my_malloc.cc:141
    #3 0x103cb9828 in free_latency_histogram_sysvars(SHOW_VAR*) mysqld.cc:4668
    #4 0x17c6231e8 in ReplSemiSyncMaster::~ReplSemiSyncMaster() semisync_source.cc:517
    #5 0x17c623488 in ReplSemiSyncMaster::~ReplSemiSyncMaster() semisync_source.cc:516
    #6 0x17c651484 in semi_sync_master_plugin_deinit(void*) semisync_source_plugin.cc:833
    #7 0x10467aa90 in plugin_deinitialize(st_plugin_int*, bool) sql_plugin.cc:1123
    #8 0x1046730b0 in reap_plugins() sql_plugin.cc:1192
    #9 0x1046863b4 in mysql_uninstall_plugin(THD*, MYSQL_LEX_CSTRING) sql_plugin.cc:2602
    #10 0x104685374 in Sql_cmd_uninstall_plugin::execute(THD*) sql_plugin.cc:3731
    #11 0x1045cea6c in mysql_execute_command(THD*, bool, unsigned long long*) sql_parse.cc:5323
    #12 0x1045c5dcc in dispatch_sql_command(THD*, Parser_state*, unsigned long long*) sql_parse.cc:6093
    #13 0x1045bb92c in dispatch_command(THD*, COM_DATA const*, enum_server_command) sql_parse.cc:2444
    #14 0x1045c06f8 in do_command(THD*) sql_parse.cc:1636
    #15 0x104cc4cc4 in handle_connection(void*) connection_handler_per_thread.cc:307
    #16 0x10bd130d4 in pfs_spawn_thread(void*) pfs.cc:2983
    #17 0x18ad47fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4)
    #18 0x18ad42d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c)
```

It seems that the double invocation of `free_latency_histogram_sysvars` is
correct in this case, thus protect against the double free with resetting the
pointers to nullptr.

Squash with D21832889

Pull Request resolved: #1290
GitHub Author: Laurynas Biveinis <laurynas.biveinis@gmail.com>

Test Plan: Imported from GitHub, without a `Test Plan:` line.

Reviewers: chni

Reviewed By: chni

Subscribers: webscalesql-eng@fb.com

Differential Revision: https://phabricator.intern.facebook.com/D45277600

Tags: aarch64, accept2ship
  • Loading branch information
hermanlee authored and Herman Lee committed Apr 28, 2023
1 parent c2339a0 commit 37524d0
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion sql/mysqld.cc
Expand Up @@ -4666,8 +4666,10 @@ histogram_display_string histogram_bucket_to_display_string(
void free_latency_histogram_sysvars(SHOW_VAR *latency_histogram_data) {
size_t i;
for (i = 0; i < NUMBER_OF_HISTOGRAM_BINS; ++i) {
if (latency_histogram_data[i].name)
if (latency_histogram_data[i].name) {
my_free(const_cast<char *>(latency_histogram_data[i].name));
latency_histogram_data[i].name = nullptr;
}
}
}

Expand Down

0 comments on commit 37524d0

Please sign in to comment.