From cf50379b916774f1516e13f297ee1fbe962c8416 Mon Sep 17 00:00:00 2001 From: Oleksandr Byelkin Date: Tue, 27 Jun 2023 12:10:48 +0200 Subject: [PATCH] MDEV-25237 crash after setting global session_track_system_variables to an invalid value Fix of typo in checking variable list corectness. Fix of error handling in case of variable list parse error --- .../main/mysqltest_tracking_info.result | 30 +++++++++++++++++ mysql-test/main/mysqltest_tracking_info.test | 32 +++++++++++++++++++ .../main/mysqltest_tracking_info_debug.result | 21 ++++++++++++ .../main/mysqltest_tracking_info_debug.test | 30 +++++++++++++++++ sql/session_tracker.cc | 11 +++++-- sql/sys_vars.inl | 4 +++ 6 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 mysql-test/main/mysqltest_tracking_info_debug.result create mode 100644 mysql-test/main/mysqltest_tracking_info_debug.test diff --git a/mysql-test/main/mysqltest_tracking_info.result b/mysql-test/main/mysqltest_tracking_info.result index 3c474cee10f..47e22449912 100644 --- a/mysql-test/main/mysqltest_tracking_info.result +++ b/mysql-test/main/mysqltest_tracking_info.result @@ -57,3 +57,33 @@ ERROR 42000: Variable 'session_track_system_variables' can't be set to the value SET SESSION session_track_system_variables=NULL; ERROR 42000: Variable 'session_track_system_variables' can't be set to the value of 'NULL' # End of 10.3 tests +# +# MDEV-25237: crash after setting global session_track_system_variables +# to an invalid value +# +SET GLOBAL session_track_system_variables='a'; +ERROR HY000: Unknown system variable 'a' +SET GLOBAL event_scheduler=1; +# check that value really returns as it was +set GLOBAL session_track_system_variables='character_set_connection'; +SET GLOBAL session_track_system_variables='a'; +ERROR HY000: Unknown system variable 'a' +connect con,localhost,root,,test; +SET NAMES 'utf8'; +-- Tracker : SESSION_TRACK_SYSTEM_VARIABLES +-- character_set_connection +-- utf8 + +SET NAMES 'big5'; +-- Tracker : SESSION_TRACK_SYSTEM_VARIABLES +-- character_set_connection +-- big5 + +select @@session_track_system_variables; +@@session_track_system_variables +character_set_connection +connection default; +disconnect con; +SET GLOBAL session_track_system_variables=default; +SET GLOBAL event_scheduler=default; +# End of 10.4 test diff --git a/mysql-test/main/mysqltest_tracking_info.test b/mysql-test/main/mysqltest_tracking_info.test index ae52571b2b9..e73117a8c16 100644 --- a/mysql-test/main/mysqltest_tracking_info.test +++ b/mysql-test/main/mysqltest_tracking_info.test @@ -60,3 +60,35 @@ SET @@GLOBAL.session_track_system_variables=NULL; SET SESSION session_track_system_variables=NULL; --echo # End of 10.3 tests + +--echo # +--echo # MDEV-25237: crash after setting global session_track_system_variables +--echo # to an invalid value +--echo # + +--error ER_UNKNOWN_SYSTEM_VARIABLE +SET GLOBAL session_track_system_variables='a'; +SET GLOBAL event_scheduler=1; + + +--echo # check that value really returns as it was + +set GLOBAL session_track_system_variables='character_set_connection'; +--error ER_UNKNOWN_SYSTEM_VARIABLE +SET GLOBAL session_track_system_variables='a'; + +connect (con,localhost,root,,test); +--enable_session_track_info +SET NAMES 'utf8'; +SET NAMES 'big5'; +--disable_session_track_info + +select @@session_track_system_variables; + +connection default; +disconnect con; + +SET GLOBAL session_track_system_variables=default; +SET GLOBAL event_scheduler=default; + +--echo # End of 10.4 test diff --git a/mysql-test/main/mysqltest_tracking_info_debug.result b/mysql-test/main/mysqltest_tracking_info_debug.result new file mode 100644 index 00000000000..39d17c7ca34 --- /dev/null +++ b/mysql-test/main/mysqltest_tracking_info_debug.result @@ -0,0 +1,21 @@ +set @save_session_track_system_variables=@@session_track_system_variables; +# +# MDEV-25237: Assertion `global_system_variables. +# session_track_system_variables' failed in +# Session_sysvars_tracker::init | SIGSEGV's in __strlen_avx2 | +# UBSAN: runtime error: null pointer passed as argument 1, which +# is declared to never be null in my_strdup +# +# check that that parser problems do not lead to crash +SET @old_debug= @@session.debug; +set debug_dbug="+d,dbug_session_tracker_parse_error"; +SET GLOBAL session_track_system_variables='query_cache_size'; +ERROR HY001: Out of memory; restart server and try again (needed 1 bytes) +set debug_dbug=@old_debug; +SELECT @@global.session_track_system_variables; +@@global.session_track_system_variables +NULL +SET GLOBAL event_scheduler=1; +SET GLOBAL session_track_system_variables=default; +SET GLOBAL event_scheduler=default; +# End of 10.4 test diff --git a/mysql-test/main/mysqltest_tracking_info_debug.test b/mysql-test/main/mysqltest_tracking_info_debug.test new file mode 100644 index 00000000000..1699801f318 --- /dev/null +++ b/mysql-test/main/mysqltest_tracking_info_debug.test @@ -0,0 +1,30 @@ + +--source include/have_debug.inc +--source include/no_protocol.inc +--source include/not_embedded.inc + + +set @save_session_track_system_variables=@@session_track_system_variables; + +--echo # +--echo # MDEV-25237: Assertion `global_system_variables. +--echo # session_track_system_variables' failed in +--echo # Session_sysvars_tracker::init | SIGSEGV's in __strlen_avx2 | +--echo # UBSAN: runtime error: null pointer passed as argument 1, which +--echo # is declared to never be null in my_strdup +--echo # + +--echo # check that that parser problems do not lead to crash +SET @old_debug= @@session.debug; +set debug_dbug="+d,dbug_session_tracker_parse_error"; +--error ER_OUTOFMEMORY +SET GLOBAL session_track_system_variables='query_cache_size'; +set debug_dbug=@old_debug; +SELECT @@global.session_track_system_variables; + +SET GLOBAL event_scheduler=1; + +SET GLOBAL session_track_system_variables=default; +SET GLOBAL event_scheduler=default; + +--echo # End of 10.4 test diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 0544827534d..cfc83b8cb74 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -221,7 +221,7 @@ bool sysvartrack_validate_value(THD *thd, const char *str, size_t len) /* Remove leading/trailing whitespace. */ trim_whitespace(system_charset_info, &var); - if (!strcmp(var.str, "*") && !find_sys_var(thd, var.str, var.length)) + if (strcmp(var.str, "*") && !find_sys_var(thd, var.str, var.length)) return true; if (lasts) @@ -331,9 +331,8 @@ void Session_sysvars_tracker::init(THD *thd) mysql_mutex_assert_owner(&LOCK_global_system_variables); DBUG_ASSERT(thd->variables.session_track_system_variables == global_system_variables.session_track_system_variables); - DBUG_ASSERT(global_system_variables.session_track_system_variables); thd->variables.session_track_system_variables= - my_strdup(global_system_variables.session_track_system_variables, + my_strdup(safe_str(global_system_variables.session_track_system_variables), MYF(MY_WME | MY_THREAD_SPECIFIC)); } @@ -572,6 +571,12 @@ bool sysvartrack_global_update(THD *thd, char *str, size_t len) { LEX_STRING tmp= { str, len }; Session_sysvars_tracker::vars_list dummy; + DBUG_EXECUTE_IF("dbug_session_tracker_parse_error", + { + my_error(ER_OUTOFMEMORY, MYF(0), 1); + return true; + }); + if (!dummy.parse_var_list(thd, tmp, false, system_charset_info)) { dummy.construct_var_list(str, len + 1); diff --git a/sql/sys_vars.inl b/sql/sys_vars.inl index 84d1cd6b331..3e282de439a 100644 --- a/sql/sys_vars.inl +++ b/sql/sys_vars.inl @@ -620,7 +620,11 @@ public: { if (sysvartrack_global_update(thd, new_val, var->save_result.string_value.length)) + { + if (new_val) + my_free(new_val); new_val= 0; + } } global_update_finish(new_val); return (new_val == 0 && var->save_result.string_value.str != 0);