Description:
This is an issue with all innodb MYSQL_SYSVAR_STR variables that can be dynamically updated, but I'm using innodb_ft_aux_table as an example.
In innodb_internal_table_validate we have this code:
static int innodb_internal_table_validate(
THD *thd, /*!< in: thread handle */
SYS_VAR *var, /*!< in: pointer to system
variable */
void *save, /*!< out: immediate result
for update function */
struct st_mysql_value *value) /*!< in: incoming string */
{
...
char buff[STRING_BUFFER_USUAL_SIZE];
...
table_name = value->val_str(value, buff, &len);
...
*static_cast<const char **>(save) = table_name;
}
Note that the memory for table_name is possibly backed by the stack allocated buff variable. This means that after the check function has finished, the memory is already invalid. This could lead to crashes.
How to repeat:
Run this under valgrind:
CREATE TABLE t1 ( id INT UNSIGNED AUTO_INCREMENT NOT NULL PRIMARY KEY, opening_line TEXT(500), author VARCHAR(200), title VARCHAR(200), FULLTEXT idx (opening_line)) ENGINE=InnoDB;
CREATE TABLE t2 ( id INT UNSIGNED AUTO_INCREMENT NOT NULL PRIMARY KEY, opening_line TEXT(500), author VARCHAR(200), title VARCHAR(200), FULLTEXT idx (opening_line)) ENGINE=InnoDB;
set @blah = 'test/t1';
SET GLOBAL innodb_ft_aux_table = @blah;
drop table t1;
drop table t2;
And get:
==2791670== Thread 38:
==2791670== Conditional jump or move depends on uninitialised value(s)
==2791670== at 0x76A4CB5: strlen (vg_replace_strmem.c:458)
==2791670== by 0x4111FFA: my_strdup(unsigned int, char const*, int) (my_malloc.cc:294)
==2791670== by 0x42A1E38: innodb_internal_table_update(THD*, SYS_VAR*, void*, void const*) (ha_innodb.cc:18242)
==2791670== by 0x28E5E90: sys_var_pluginvar::global_update(THD*, set_var*) (sql_plugin_var.cc:416)
==2791670== by 0x277B16F: sys_var::update(THD*, set_var*) (set_var.cc:264)
==2791670== by 0x277E1D2: set_var::update(THD*) (set_var.cc:1040)
==2791670== by 0x277D3A6: sql_set_variables(THD*, List<set_var_base>*, bool) (set_var.cc:782)
==2791670== by 0x28A02C2: mysql_execute_command(THD*, bool, unsigned long long*) (sql_parse.cc:3712)
==2791670== by 0x289A61B: mysql_parse(THD*, Parser_state*, bool, unsigned long long*) (sql_parse.cc:5398)
==2791670== by 0x2897323: dispatch_command(THD*, COM_DATA const*, enum_server_command) (sql_parse.cc:1815)
==2791670== by 0x2899776: do_command(THD*) (sql_parse.cc:1272)
==2791670== by 0x2A81238: handle_connection(void*) (connection_handler_per_thread.cc:334)
==2791670== by 0x49118BD: pfs_spawn_thread(void*) (pfs.cc:2836)
==2791670== by 0x830166D: start_thread (pthread_create.c:465)
==2791670== by 0x79D3E2E: clone (clone.S:95)
==2791670== Conditional jump or move depends on uninitialised value(s)
==2791670== at 0x76A4CC8: strlen (vg_replace_strmem.c:458)
==2791670== by 0x4111FFA: my_strdup(unsigned int, char const*, int) (my_malloc.cc:294)
==2791670== by 0x42A1E38: innodb_internal_table_update(THD*, SYS_VAR*, void*, void const*) (ha_innodb.cc:18242)
==2791670== by 0x28E5E90: sys_var_pluginvar::global_update(THD*, set_var*) (sql_plugin_var.cc:416)
==2791670== by 0x277B16F: sys_var::update(THD*, set_var*) (set_var.cc:264)
==2791670== by 0x277E1D2: set_var::update(THD*) (set_var.cc:1040)
==2791670== by 0x277D3A6: sql_set_variables(THD*, List<set_var_base>*, bool) (set_var.cc:782)
==2791670== by 0x28A02C2: mysql_execute_command(THD*, bool, unsigned long long*) (sql_parse.cc:3712)
==2791670== by 0x289A61B: mysql_parse(THD*, Parser_state*, bool, unsigned long long*) (sql_parse.cc:5398)
==2791670== by 0x2897323: dispatch_command(THD*, COM_DATA const*, enum_server_command) (sql_parse.cc:1815)
==2791670== by 0x2899776: do_command(THD*) (sql_parse.cc:1272)
==2791670== by 0x2A81238: handle_connection(void*) (connection_handler_per_thread.cc:334)
==2791670== by 0x49118BD: pfs_spawn_thread(void*) (pfs.cc:2836)
==2791670== by 0x830166D: start_thread (pthread_create.c:465)
==2791670== by 0x79D3E2E: clone (clone.S:95)
Suggested fix:
If the check function allocates 'save' info, then the update function should be responsible for either freeing it, or otherwise assigning it as the value (so that it gets freed at some point).
The only problem is that the update function is called without check when doing SET some_var = DEFAULT, but we still pass in 'save' info from &((sysvar_str_t*) plugin_var)->def_val (see sys_var_pluginvar::global_update).
This means that the update function might have to compare with def_val to see if it's safe to free or not.
This would probably be easier if the plugin interface were reworked.
Description: This is an issue with all innodb MYSQL_SYSVAR_STR variables that can be dynamically updated, but I'm using innodb_ft_aux_table as an example. In innodb_internal_table_validate we have this code: static int innodb_internal_table_validate( THD *thd, /*!< in: thread handle */ SYS_VAR *var, /*!< in: pointer to system variable */ void *save, /*!< out: immediate result for update function */ struct st_mysql_value *value) /*!< in: incoming string */ { ... char buff[STRING_BUFFER_USUAL_SIZE]; ... table_name = value->val_str(value, buff, &len); ... *static_cast<const char **>(save) = table_name; } Note that the memory for table_name is possibly backed by the stack allocated buff variable. This means that after the check function has finished, the memory is already invalid. This could lead to crashes. How to repeat: Run this under valgrind: CREATE TABLE t1 ( id INT UNSIGNED AUTO_INCREMENT NOT NULL PRIMARY KEY, opening_line TEXT(500), author VARCHAR(200), title VARCHAR(200), FULLTEXT idx (opening_line)) ENGINE=InnoDB; CREATE TABLE t2 ( id INT UNSIGNED AUTO_INCREMENT NOT NULL PRIMARY KEY, opening_line TEXT(500), author VARCHAR(200), title VARCHAR(200), FULLTEXT idx (opening_line)) ENGINE=InnoDB; set @blah = 'test/t1'; SET GLOBAL innodb_ft_aux_table = @blah; drop table t1; drop table t2; And get: ==2791670== Thread 38: ==2791670== Conditional jump or move depends on uninitialised value(s) ==2791670== at 0x76A4CB5: strlen (vg_replace_strmem.c:458) ==2791670== by 0x4111FFA: my_strdup(unsigned int, char const*, int) (my_malloc.cc:294) ==2791670== by 0x42A1E38: innodb_internal_table_update(THD*, SYS_VAR*, void*, void const*) (ha_innodb.cc:18242) ==2791670== by 0x28E5E90: sys_var_pluginvar::global_update(THD*, set_var*) (sql_plugin_var.cc:416) ==2791670== by 0x277B16F: sys_var::update(THD*, set_var*) (set_var.cc:264) ==2791670== by 0x277E1D2: set_var::update(THD*) (set_var.cc:1040) ==2791670== by 0x277D3A6: sql_set_variables(THD*, List<set_var_base>*, bool) (set_var.cc:782) ==2791670== by 0x28A02C2: mysql_execute_command(THD*, bool, unsigned long long*) (sql_parse.cc:3712) ==2791670== by 0x289A61B: mysql_parse(THD*, Parser_state*, bool, unsigned long long*) (sql_parse.cc:5398) ==2791670== by 0x2897323: dispatch_command(THD*, COM_DATA const*, enum_server_command) (sql_parse.cc:1815) ==2791670== by 0x2899776: do_command(THD*) (sql_parse.cc:1272) ==2791670== by 0x2A81238: handle_connection(void*) (connection_handler_per_thread.cc:334) ==2791670== by 0x49118BD: pfs_spawn_thread(void*) (pfs.cc:2836) ==2791670== by 0x830166D: start_thread (pthread_create.c:465) ==2791670== by 0x79D3E2E: clone (clone.S:95) ==2791670== Conditional jump or move depends on uninitialised value(s) ==2791670== at 0x76A4CC8: strlen (vg_replace_strmem.c:458) ==2791670== by 0x4111FFA: my_strdup(unsigned int, char const*, int) (my_malloc.cc:294) ==2791670== by 0x42A1E38: innodb_internal_table_update(THD*, SYS_VAR*, void*, void const*) (ha_innodb.cc:18242) ==2791670== by 0x28E5E90: sys_var_pluginvar::global_update(THD*, set_var*) (sql_plugin_var.cc:416) ==2791670== by 0x277B16F: sys_var::update(THD*, set_var*) (set_var.cc:264) ==2791670== by 0x277E1D2: set_var::update(THD*) (set_var.cc:1040) ==2791670== by 0x277D3A6: sql_set_variables(THD*, List<set_var_base>*, bool) (set_var.cc:782) ==2791670== by 0x28A02C2: mysql_execute_command(THD*, bool, unsigned long long*) (sql_parse.cc:3712) ==2791670== by 0x289A61B: mysql_parse(THD*, Parser_state*, bool, unsigned long long*) (sql_parse.cc:5398) ==2791670== by 0x2897323: dispatch_command(THD*, COM_DATA const*, enum_server_command) (sql_parse.cc:1815) ==2791670== by 0x2899776: do_command(THD*) (sql_parse.cc:1272) ==2791670== by 0x2A81238: handle_connection(void*) (connection_handler_per_thread.cc:334) ==2791670== by 0x49118BD: pfs_spawn_thread(void*) (pfs.cc:2836) ==2791670== by 0x830166D: start_thread (pthread_create.c:465) ==2791670== by 0x79D3E2E: clone (clone.S:95) Suggested fix: If the check function allocates 'save' info, then the update function should be responsible for either freeing it, or otherwise assigning it as the value (so that it gets freed at some point). The only problem is that the update function is called without check when doing SET some_var = DEFAULT, but we still pass in 'save' info from &((sysvar_str_t*) plugin_var)->def_val (see sys_var_pluginvar::global_update). This means that the update function might have to compare with def_val to see if it's safe to free or not. This would probably be easier if the plugin interface were reworked.