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
    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)
```

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: facebook#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
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 37524d0

Please sign in to comment.