Skip to content

Commit

Permalink
MDEV-19536 - Server crash or ASAN heap-use-after-free in is_temporary…
Browse files Browse the repository at this point in the history
…_table /

             read_statistics_for_tables_if_needed

Regression after 279a907, read_statistics_for_tables_if_needed() was
called after open_normal_and_derived_tables() failure.

Fixed by moving read_statistics_for_tables() call to a branch of
get_schema_stat_record() where result of open_normal_and_derived_tables()
is checked.

Removed THD::force_read_stats, added read_statistics_for_tables() instead.
Simplified away statistics_for_command_is_needed().
  • Loading branch information
Sergey Vojtovich committed Oct 7, 2019
1 parent e43791d commit adefaef
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 64 deletions.
1 change: 0 additions & 1 deletion sql/sql_class.cc
Expand Up @@ -986,7 +986,6 @@ THD::THD(bool is_wsrep_applier)
memset(&invoker_host, 0, sizeof(invoker_host));
prepare_derived_at_open= FALSE;
create_tmp_table_for_derived= FALSE;
force_read_stats= FALSE;
save_prep_leaf_list= FALSE;
/* Restore THR_THD */
set_current_thd(old_THR_THD);
Expand Down
3 changes: 0 additions & 3 deletions sql/sql_class.h
Expand Up @@ -2200,9 +2200,6 @@ class THD :public Statement,
*/
bool create_tmp_table_for_derived;

/* The flag to force reading statistics from EITS tables */
bool force_read_stats;

bool save_prep_leaf_list;

/* container for handler's private per-connection data */
Expand Down
5 changes: 1 addition & 4 deletions sql/sql_show.cc
Expand Up @@ -4272,7 +4272,6 @@ fill_schema_table_by_open(THD *thd, bool is_show_fields_or_keys,
SQLCOM_SHOW_FIELDS is used because it satisfies
'only_view_structure()'.
*/
thd->force_read_stats= get_schema_table_idx(schema_table) == SCH_STATISTICS;
lex->sql_command= SQLCOM_SHOW_FIELDS;
result= (open_temporary_tables(thd, table_list) ||
open_normal_and_derived_tables(thd, table_list,
Expand All @@ -4287,9 +4286,6 @@ fill_schema_table_by_open(THD *thd, bool is_show_fields_or_keys,
*/
lex->sql_command= old_lex->sql_command;

(void) read_statistics_for_tables_if_needed(thd, table_list);
thd->force_read_stats= false;

DEBUG_SYNC(thd, "after_open_table_ignore_flush");

/*
Expand Down Expand Up @@ -6165,6 +6161,7 @@ static int get_schema_stat_record(THD *thd, TABLE_LIST *tables,
KEY *key_info=show_table->s->key_info;
if (show_table->file)
{
(void) read_statistics_for_tables(thd, tables);
show_table->file->info(HA_STATUS_VARIABLE |
HA_STATUS_NO_LOCK |
HA_STATUS_TIME);
Expand Down
83 changes: 27 additions & 56 deletions sql/sql_statistics.cc
Expand Up @@ -2160,54 +2160,6 @@ int alloc_statistics_for_table(THD* thd, TABLE *table)
}


/**
@brief
Check whether any persistent statistics for the processed command is needed
@param
thd The thread handle
@details
The function checks whether any persitent statistics for the processed
command is needed to be read.
@retval
TRUE statistics is needed to be read
@retval
FALSE Otherwise
*/

static
inline bool statistics_for_command_is_needed(THD *thd)
{
if (thd->bootstrap || thd->variables.use_stat_tables == NEVER)
return FALSE;

if (thd->force_read_stats)
return TRUE;

switch(thd->lex->sql_command) {
case SQLCOM_SELECT:
case SQLCOM_INSERT:
case SQLCOM_INSERT_SELECT:
case SQLCOM_UPDATE:
case SQLCOM_UPDATE_MULTI:
case SQLCOM_DELETE:
case SQLCOM_DELETE_MULTI:
case SQLCOM_REPLACE:
case SQLCOM_REPLACE_SELECT:
case SQLCOM_CREATE_TABLE:
case SQLCOM_SET_OPTION:
case SQLCOM_DO:
break;
default:
return FALSE;
}

return TRUE;
}


/**
@brief
Allocate memory for the statistical data used by a table share
Expand Down Expand Up @@ -2255,9 +2207,6 @@ static int alloc_statistics_for_table_share(THD* thd, TABLE_SHARE *table_share)
DEBUG_SYNC(thd, "statistics_mem_alloc_start1");
DEBUG_SYNC(thd, "statistics_mem_alloc_start2");

if (!statistics_for_command_is_needed(thd))
DBUG_RETURN(1);

mysql_mutex_lock(&table_share->LOCK_share);

if (stats_cb->stats_can_be_read)
Expand Down Expand Up @@ -3110,9 +3059,6 @@ bool statistics_for_tables_is_needed(THD *thd, TABLE_LIST *tables)
{
if (!tables)
return FALSE;

if (!statistics_for_command_is_needed(thd))
return FALSE;

/*
Do not read statistics for any query that explicity involves
Expand Down Expand Up @@ -3244,15 +3190,40 @@ int read_histograms_for_table(THD *thd, TABLE *table, TABLE_LIST *stat_tables)
*/

int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables)
{
switch (thd->lex->sql_command) {
case SQLCOM_SELECT:
case SQLCOM_INSERT:
case SQLCOM_INSERT_SELECT:
case SQLCOM_UPDATE:
case SQLCOM_UPDATE_MULTI:
case SQLCOM_DELETE:
case SQLCOM_DELETE_MULTI:
case SQLCOM_REPLACE:
case SQLCOM_REPLACE_SELECT:
case SQLCOM_CREATE_TABLE:
case SQLCOM_SET_OPTION:
case SQLCOM_DO:
return read_statistics_for_tables(thd, tables);
default:
return 0;
}
}


int read_statistics_for_tables(THD *thd, TABLE_LIST *tables)
{
TABLE_LIST stat_tables[STATISTICS_TABLES];
Open_tables_backup open_tables_backup;

DBUG_ENTER("read_statistics_for_tables_if_needed");
DBUG_ENTER("read_statistics_for_tables");

if (thd->bootstrap || thd->variables.use_stat_tables == NEVER)
DBUG_RETURN(0);

for (TABLE_LIST *tl= tables; tl; tl= tl->next_global)
{
if (get_use_stat_tables_mode(thd) > NEVER && tl->table)
if (tl->table)
{
TABLE_SHARE *table_share= tl->table->s;
if (table_share && table_share->table_category == TABLE_CATEGORY_USER &&
Expand Down
1 change: 1 addition & 0 deletions sql/sql_statistics.h
Expand Up @@ -89,6 +89,7 @@ Use_stat_tables_mode get_use_stat_tables_mode(THD *thd)
}

int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables);
int read_statistics_for_tables(THD *thd, TABLE_LIST *tables);
int collect_statistics_for_table(THD *thd, TABLE *table);
void delete_stat_values_for_table_share(TABLE_SHARE *table_share);
int alloc_statistics_for_table(THD *thd, TABLE *table);
Expand Down

0 comments on commit adefaef

Please sign in to comment.