Skip to content

Commit

Permalink
Avoid double free on latency histogram data
Browse files Browse the repository at this point in the history
Before the fix, if semisync_source plugin is installed, used, and uninstalled
repeatedly, querying its status variables on a second or later installation
would result in a double free error on macOS. This was because plugin
uninstallation freed the histogram name variables but left their pointers
around, which got picked up on the later status variable query. This was not
visible under Linux because there the dynamic linker would clear the plugin
variables on every load.

rpl.rpl_semi_sync_alias test error under ASan:

```
=================================================================
==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
    facebook#1 0x103cb9828 in free_latency_histogram_sysvars(SHOW_VAR*) mysqld.cc:4668
    facebook#2 0x103cb99bc in prepare_latency_histogram_vars(latency_histogram*, SHOW_VAR*, unsigned long long*) mysqld.cc:4692
    facebook#3 0x17c65826c in rpl_semi_sync_master_trx_wait_histogram(THD*, SHOW_VAR*, char*) semisync_source_plugin.cc:581
    facebook#4 0x10be1b4cc in PFS_status_variable_cache::manifest(THD*, SHOW_VAR const*, System_status_var*, char const*, bool, bool) pfs_variable.cc:1366
    facebook#5 0x10be1ba90 in PFS_status_variable_cache::do_materialize_all(THD*) pfs_variable.cc:1172
    facebook#6 0x10c0ab33c in PFS_variable_cache<Status_variable>::materialize_all(THD*) pfs_variable.h:536
    facebook#7 0x10c0ab294 in table_session_status::rnd_init(bool) table_session_status.cc:111
    facebook#8 0x10bceb790 in ha_perfschema::rnd_init(bool) ha_perfschema.cc:1686
    facebook#9 0x1033c7cec in handler::ha_rnd_init(bool) handler.cc:3157
    facebook#10 0x103975380 in TableScanIterator::Init() basic_row_iterators.cc:230
    facebook#11 0x103a33a18 in FilterIterator::Init() composite_iterators.h:82
    facebook#12 0x103982ec0 in MaterializeIterator::MaterializeQueryBlock(MaterializeIterator::QueryBlock const&, unsigned long long*) composite_iterators.cc:845
    facebook#13 0x103981410 in MaterializeIterator::Init() composite_iterators.cc:660
    facebook#14 0x1049fc518 in Query_expression::ExecuteIteratorQuery(THD*) sql_union.cc:1293
    facebook#15 0x1049fd358 in Query_expression::execute(THD*) sql_union.cc:1355
    facebook#16 0x1047ae7ac in Sql_cmd_dml::execute_inner(THD*) sql_select.cc:870
    facebook#17 0x1047ac344 in Sql_cmd_dml::execute(THD*) sql_select.cc:618
    facebook#18 0x1047ffcc8 in Sql_cmd_show::execute(THD*) sql_show.cc:232
    facebook#19 0x10480ab58 in Sql_cmd_show_status::execute(THD*) sql_show.cc:894
    facebook#20 0x1045cea6c in mysql_execute_command(THD*, bool, unsigned long long*) sql_parse.cc:5323
    facebook#21 0x1045c5dcc in dispatch_sql_command(THD*, Parser_state*, unsigned long long*) sql_parse.cc:6093
    facebook#22 0x1045bb92c in dispatch_command(THD*, COM_DATA const*, enum_server_command) sql_parse.cc:2444
    facebook#23 0x1045c06f8 in do_command(THD*) sql_parse.cc:1636
    facebook#24 0x104cc4cc4 in handle_connection(void*) connection_handler_per_thread.cc:307
    facebook#25 0x10bd130d4 in pfs_spawn_thread(void*) pfs.cc:2983
    facebook#26 0x18ad47fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4)
    facebook#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)
    facebook#1 0x107febcfc in my_raw_free(void*) my_malloc.cc:269
    facebook#2 0x107feba48 in my_free(void*) my_malloc.cc:141
    facebook#3 0x103cb9828 in free_latency_histogram_sysvars(SHOW_VAR*) mysqld.cc:4668
    facebook#4 0x17c6231e8 in ReplSemiSyncMaster::~ReplSemiSyncMaster() semisync_source.cc:517
    facebook#5 0x17c623488 in ReplSemiSyncMaster::~ReplSemiSyncMaster() semisync_source.cc:516
    facebook#6 0x17c651484 in semi_sync_master_plugin_deinit(void*) semisync_source_plugin.cc:833
    facebook#7 0x10467aa90 in plugin_deinitialize(st_plugin_int*, bool) sql_plugin.cc:1123
    facebook#8 0x1046730b0 in reap_plugins() sql_plugin.cc:1192
    facebook#9 0x1046863b4 in mysql_uninstall_plugin(THD*, MYSQL_LEX_CSTRING) sql_plugin.cc:2602
    facebook#10 0x104685374 in Sql_cmd_uninstall_plugin::execute(THD*) sql_plugin.cc:3731
    facebook#11 0x1045cea6c in mysql_execute_command(THD*, bool, unsigned long long*) sql_parse.cc:5323
    facebook#12 0x1045c5dcc in dispatch_sql_command(THD*, Parser_state*, unsigned long long*) sql_parse.cc:6093
    facebook#13 0x1045bb92c in dispatch_command(THD*, COM_DATA const*, enum_server_command) sql_parse.cc:2444
    facebook#14 0x1045c06f8 in do_command(THD*) sql_parse.cc:1636
    facebook#15 0x104cc4cc4 in handle_connection(void*) connection_handler_per_thread.cc:307
    facebook#16 0x10bd130d4 in pfs_spawn_thread(void*) pfs.cc:2983
    facebook#17 0x18ad47fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4)
    facebook#18 0x18ad42d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c)
```

Squash with 19345e3
  • Loading branch information
laurynas-biveinis committed Apr 25, 2023
1 parent dde9e11 commit b4f9933
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion sql/mysqld.cc
Original file line number Diff line number Diff line change
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 b4f9933

Please sign in to comment.