From bf0aa99aeb7dd2bb7497f5ae9a82d1e72e7ee1d5 Mon Sep 17 00:00:00 2001 From: Brandon Nesterenko Date: Thu, 6 Jun 2024 11:46:27 -0600 Subject: [PATCH] MDEV-34237: On Startup: UBSAN: runtime error: call to function MDL_lock::lf_hash_initializer lf_hash_insert through pointer to incorrect function type 'void (*)(st_lf_hash *, void *, const void *)' A few different incorrect function type UBSAN issues have been grouped into this patch. The only real potentially undefined behavior is an error about show_func_mutex_instances_lost, which when invoked in sql_show.cc::show_status_array(), puts 5 arguments onto the stack; however, the implementing function only actually has 3 parameters (so only 3 would be popped). This was fixed by adding in the remaining parameters to satisfy the type mysql_show_var_func. The rest of the findings are pointer type mismatches that wouldn't lead to actual undefined behavior. The lf_hash_initializer function type definition is typedef void (*lf_hash_initializer)(LF_HASH *hash, void *dst, const void *src); but the MDL_lock and table cache's implementations of this function do not have that signature. The MDL_lock has specific MDL object parameters: static void lf_hash_initializer(LF_HASH *hash __attribute__((unused)), MDL_lock *lock, MDL_key *key_arg) and the table cache has specific TDC parameters: static void tdc_hash_initializer(LF_HASH *, TDC_element *element, LEX_STRING *key) leading to UBSAN runtime errors when invoking these functions. This patch fixes these type mis-matches by changing the implementing functions to use void * and const void * for their respective parameters, and later casting them to their expected type in the function body. Note too the functions tdc_hash_key and tc_purge_callback had a similar problem to tdc_hash_initializer and was fixed similarly. Reviewed By: ============ Sergei Golubchik --- sql/mdl.cc | 4 +++- sql/sql_show.cc | 2 +- sql/table_cache.cc | 13 +++++++++---- storage/perfschema/ha_perfschema.cc | 4 +++- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/sql/mdl.cc b/sql/mdl.cc index 6b8d3599c00..fb106a1af47 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -673,8 +673,10 @@ public: { ((MDL_lock*)(arg + LF_HASH_OVERHEAD))->~MDL_lock(); } static void lf_hash_initializer(LF_HASH *hash __attribute__((unused)), - MDL_lock *lock, MDL_key *key_arg) + void *_lock, const void *_key_arg) { + MDL_lock *lock= static_cast(_lock); + const MDL_key *key_arg= static_cast(_key_arg); DBUG_ASSERT(key_arg->mdl_namespace() != MDL_key::BACKUP); new (&lock->key) MDL_key(key_arg); if (key_arg->mdl_namespace() == MDL_key::SCHEMA) diff --git a/sql/sql_show.cc b/sql/sql_show.cc index eb84ef56419..ca291bd9e5d 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -3789,7 +3789,7 @@ static bool show_status_array(THD *thd, const char *wild, */ for (var=variables; var->type == SHOW_FUNC || var->type == SHOW_SIMPLE_FUNC; var= &tmp) - ((mysql_show_var_func)(var->value))(thd, &tmp, buff, + ((mysql_show_var_func)(var->value))(thd, &tmp, (void *) buff, status_var, scope); SHOW_TYPE show_type=var->type; diff --git a/sql/table_cache.cc b/sql/table_cache.cc index 499e82fafea..b76ba4fd239 100644 --- a/sql/table_cache.cc +++ b/sql/table_cache.cc @@ -293,9 +293,11 @@ static void tc_remove_all_unused_tables(TDC_element *element, periodicly flush all not used tables. */ -static my_bool tc_purge_callback(TDC_element *element, - Share_free_tables::List *purge_tables) +static my_bool tc_purge_callback(void *_element, void *_purge_tables) { + TDC_element *element= static_cast(_element); + Share_free_tables::List *purge_tables= + static_cast(_purge_tables); mysql_mutex_lock(&element->LOCK_table_share); tc_remove_all_unused_tables(element, purge_tables); mysql_mutex_unlock(&element->LOCK_table_share); @@ -566,17 +568,20 @@ static void lf_alloc_destructor(uchar *arg) static void tdc_hash_initializer(LF_HASH *, - TDC_element *element, LEX_STRING *key) + void *_element, const void *_key) { + TDC_element *element= static_cast(_element); + const LEX_STRING *key= static_cast(_key); memcpy(element->m_key, key->str, key->length); element->m_key_length= (uint)key->length; tdc_assert_clean_share(element); } -static uchar *tdc_hash_key(const TDC_element *element, size_t *length, +static uchar *tdc_hash_key(const unsigned char *_element, size_t *length, my_bool) { + const TDC_element *element= (const TDC_element *) _element; *length= element->m_key_length; return (uchar*) element->m_key; } diff --git a/storage/perfschema/ha_perfschema.cc b/storage/perfschema/ha_perfschema.cc index c6d2f23d653..8b7825f3816 100644 --- a/storage/perfschema/ha_perfschema.cc +++ b/storage/perfschema/ha_perfschema.cc @@ -133,7 +133,9 @@ static int pfs_done_func(void *p) DBUG_RETURN(0); } -static int show_func_mutex_instances_lost(THD *thd, SHOW_VAR *var, char *buff) +static int show_func_mutex_instances_lost(THD *thd, SHOW_VAR *var, void *buff, + struct system_status_var *status_var, + enum enum_var_type) { var->type= SHOW_LONG; var->value= buff;