1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

MDEV-23841: Memory leak in innodb_monitor_validate()

Browse Source 
innodb_monitor_validate(): Let item_val_str() allocate the memory
in THD, so that it will be available to innodb_monitor_update().
In this way, there is no need to allocate another buffer, and
no problem if the call to innodb_monitor_update() is skipped due
to an invalid value that is passed to another configuration parameter.

There are some other callers to st_mysql_sys_var::val_str()
that validate configuration parameters that are related to FULLTEXT INDEX,
but they will allocate memory by invoking thd_strmake().
This commit is contained in:
Marko Mäkelä 2023-09-11 10:27:21 +03:00
parent 5299f0c45e
commit 65c99207e0
3 changed files with 22 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
mysql-test/suite/innodb/r/monitor.result
View File

@ -637,6 +637,8 @@ LIKE 'buffer_page_written_index_leaf';
NAME COUNT > 0
buffer_page_written_index_leaf 0
SET GLOBAL innodb_monitor_enable='%';
SET GLOBAL innodb_monitor_reset_all= '%', innodb_compression_algorithm= foo;
ERROR 42000: Variable 'innodb_compression_algorithm' can't be set to the value of 'foo'
INSERT INTO t1 VALUES (5), (6), (7), (8);
FLUSH TABLES t1 FOR EXPORT;
UNLOCK TABLES;

2
mysql-test/suite/innodb/t/monitor.test
View File

@ -408,6 +408,8 @@ SELECT NAME, COUNT > 0 FROM INFORMATION_SCHEMA.INNODB_METRICS WHERE NAME
LIKE 'buffer_page_written_index_leaf';
SET GLOBAL innodb_monitor_enable='%';
--error ER_WRONG_VALUE_FOR_VAR
SET GLOBAL innodb_monitor_reset_all= '%', innodb_compression_algorithm= foo;
INSERT INTO t1 VALUES (5), (6), (7), (8); FLUSH TABLES t1 FOR EXPORT;
UNLOCK TABLES;
SELECT NAME, COUNT > 0 FROM INFORMATION_SCHEMA.INNODB_METRICS WHERE NAME

78
storage/innobase/handler/ha_innodb.cc
View File

@ -17808,13 +17808,7 @@ innodb_monitor_validate_wildcard_name(
Validate the passed in monitor name, find and save the
corresponding monitor name in the function parameter "save".
@return 0 if monitor name is valid */
static
int
innodb_monitor_valid_byname(
/*========================*/
void* save, /*!< out: immediate result
for update function */
const char* name) /*!< in: incoming monitor name */
static int innodb_monitor_valid_byname(const char *name)
{
ulint use;
monitor_info_t* monitor_info;
@ -17862,9 +17856,6 @@ innodb_monitor_valid_byname(
}
}
/* Save the configure name for innodb_monitor_update() */
*static_cast<const char**>(save) = name;
return(0);
}
/*************************************************************//**
@ -17880,41 +17871,18 @@ innodb_monitor_validate(
for update function */
struct st_mysql_value* value) /*!< in: incoming string */
{
const char* name;
char* monitor_name;
char buff[STRING_BUFFER_USUAL_SIZE];
int len = sizeof(buff);
int ret;
int ret= 0;
ut_a(save != NULL);
ut_a(value != NULL);
if (const char *name= value->val_str(value, nullptr, &ret))
{
ret= innodb_monitor_valid_byname(name);
if (!ret)
*static_cast<const char**>(save)= name;
}
else
ret= 1;
name = value->val_str(value, buff, &len);
/* monitor_name could point to memory from MySQL
or buff[]. Always dup the name to memory allocated
by InnoDB, so we can access it in another callback
function innodb_monitor_update() and free it appropriately */
if (name) {
monitor_name = my_strdup(//PSI_INSTRUMENT_ME,
name, MYF(0));
} else {
return(1);
}
ret = innodb_monitor_valid_byname(save, monitor_name);
if (ret) {
/* Validation failed */
my_free(monitor_name);
} else {
/* monitor_name will be freed in separate callback function
innodb_monitor_update(). Assert "save" point to
the "monitor_name" variable */
ut_ad(*static_cast<char**>(save) == monitor_name);
}
return(ret);
return ret;
}
/****************************************************************//**
@ -17930,11 +17898,9 @@ innodb_monitor_update(
formal string goes */
const void* save, /*!< in: immediate result
from check function */
mon_option_t set_option, /*!< in: the set option,
mon_option_t set_option) /*!< in: the set option,
whether to turn on/off or
reset the counter */
ibool free_mem) /*!< in: whether we will
need to free the memory */
{
monitor_info_t* monitor_info;
ulint monitor_id;
@ -18018,12 +17984,6 @@ exit:
sql_print_warning("InnoDB: Monitor %s is already enabled.",
srv_mon_get_name((monitor_id_t) err_monitor));
}
if (free_mem && name) {
my_free((void*) name);
}
return;
}
#ifdef UNIV_DEBUG
@ -18114,7 +18074,7 @@ innodb_enable_monitor_update(
const void* save) /*!< in: immediate result
from check function */
{
innodb_monitor_update(thd, var_ptr, save, MONITOR_TURN_ON, TRUE);
innodb_monitor_update(thd, var_ptr, save, MONITOR_TURN_ON);
}
/****************************************************************//**
@ -18131,7 +18091,7 @@ innodb_disable_monitor_update(
const void* save) /*!< in: immediate result
from check function */
{
innodb_monitor_update(thd, var_ptr, save, MONITOR_TURN_OFF, TRUE);
innodb_monitor_update(thd, var_ptr, save, MONITOR_TURN_OFF);
}
/****************************************************************//**
@ -18149,7 +18109,7 @@ innodb_reset_monitor_update(
const void* save) /*!< in: immediate result
from check function */
{
innodb_monitor_update(thd, var_ptr, save, MONITOR_RESET_VALUE, TRUE);
innodb_monitor_update(thd, var_ptr, save, MONITOR_RESET_VALUE);
}
/****************************************************************//**
@ -18167,8 +18127,7 @@ innodb_reset_all_monitor_update(
const void* save) /*!< in: immediate result
from check function */
{
innodb_monitor_update(thd, var_ptr, save, MONITOR_RESET_ALL_VALUE,
TRUE);
innodb_monitor_update(thd, var_ptr, save, MONITOR_RESET_ALL_VALUE);
}
static
@ -18213,10 +18172,9 @@ innodb_enable_monitor_at_startup(
for (char* option = my_strtok_r(str, sep, &last);
option;
option = my_strtok_r(NULL, sep, &last)) {
char* option_name;
if (!innodb_monitor_valid_byname(&option_name, option)) {
if (!innodb_monitor_valid_byname(option)) {
innodb_monitor_update(NULL, NULL, &option,
MONITOR_TURN_ON, FALSE);
MONITOR_TURN_ON);
} else {
sql_print_warning("Invalid monitor counter"
" name: '%s'", option);