Hi, Oleksandr, On Jul 07, Oleksandr Byelkin wrote:
revision-id: d8e04ef367b (mariadb-10.4.30-9-gd8e04ef367b) parent(s): 423c28f0aa4 author: Oleksandr Byelkin committer: Oleksandr Byelkin timestamp: 2023-07-03 17:50:49 +0200 message:
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
first, if you see a bug with a very long summary, particularly if it's basically makes no sense to a user - in that case, please, don't hesitate to make it shorter and more meaningful. For example "crash after setting session_track_system_variables to an invalid value"
--- a/sql/sys_vars.inl +++ b/sql/sys_vars.inl @@ -620,7 +620,12 @@ class Sys_var_sesvartrack: public Sys_var_charptr_base { if (sysvartrack_global_update(thd, new_val, var->save_result.string_value.length)) - new_val= 0; + { + if (new_val) + my_free(new_val); + // keep the old value in case of error + new_val= (char*)my_strdup(global_var(const char*), MYF(MY_WME)); + }
No, this is wrong. Well, not wrong, but an incorrect bug fix. update should not fail, this is the contract. check() is supposed to verify that the value is valid, so basically update must succeed. technically it can fail because of OOM, I suppose, so restoring the value here might be needed still. But in the case of OOM, strdup() might be not such a great idea. Perhaps you can make sure that OOM is impossible and update truly cannot fail? Anyway, the actual fix for MDEV-25237 is to make check fail on an invalid value. In fact, it was a simple typo: @@ -221,7 +221,7 @@ bool sysvartrack_validate_value(THD *thd, const char *str, > /* 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;
} global_update_finish(new_val); return (new_val == 0 && var->save_result.string_value.str != 0);
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org