Many files:

  Fix remaining cases of Bug #3596: fix possible races caused by an obsolete value of thd->query_length in SHOW PROCESSLIST and SHOW INNODB STATUS; this fix depends on the fact that thd->query is always set to NULL before setting it to point to a new query
parent b4ae2577
...@@ -390,15 +390,16 @@ innobase_mysql_print_thd( ...@@ -390,15 +390,16 @@ innobase_mysql_print_thd(
len = 300; /* ADDITIONAL SAFETY: print at most len = 300; /* ADDITIONAL SAFETY: print at most
300 chars to reduce the probability of 300 chars to reduce the probability of
a seg fault if there is a race in a seg fault if there is a race in
thd->query_len in MySQL; on May 13, thd->query_length in MySQL; after
2004 we do not know */ May 14, 2004 probably no race any more,
but better be safe */
} }
for (i = 0; i < len && s[i]; i++); for (i = 0; i < len && s[i]; i++);
memcpy(buf, s, i); /* Use memcpy to reduce the timeframe memcpy(buf, s, i); /* Use memcpy to reduce the timeframe
for a race, compared to fwrite() */ for a race, compared to fwrite() */
buf[300] = '\0'; buf[300] = '\0'; /* not needed, just extra safety */
putc('\n', f); putc('\n', f);
fwrite(buf, 1, i, f); fwrite(buf, 1, i, f);
......
...@@ -1929,6 +1929,7 @@ end: ...@@ -1929,6 +1929,7 @@ end:
VOID(pthread_mutex_lock(&LOCK_thread_count)); VOID(pthread_mutex_lock(&LOCK_thread_count));
thd->db= 0; // prevent db from being freed thd->db= 0; // prevent db from being freed
thd->query= 0; // just to be sure thd->query= 0; // just to be sure
thd->query_length= 0;
VOID(pthread_mutex_unlock(&LOCK_thread_count)); VOID(pthread_mutex_unlock(&LOCK_thread_count));
// assume no convert for next query unless set explictly // assume no convert for next query unless set explictly
thd->variables.convert_set = 0; thd->variables.convert_set = 0;
......
...@@ -2691,6 +2691,7 @@ err: ...@@ -2691,6 +2691,7 @@ err:
IO_RPL_LOG_NAME, llstr(mi->master_log_pos,llbuff)); IO_RPL_LOG_NAME, llstr(mi->master_log_pos,llbuff));
VOID(pthread_mutex_lock(&LOCK_thread_count)); VOID(pthread_mutex_lock(&LOCK_thread_count));
thd->query = thd->db = 0; // extra safety thd->query = thd->db = 0; // extra safety
thd->query_length = 0;
VOID(pthread_mutex_unlock(&LOCK_thread_count)); VOID(pthread_mutex_unlock(&LOCK_thread_count));
if (mysql) if (mysql)
{ {
...@@ -2839,6 +2840,7 @@ the slave SQL thread with \"SLAVE START\". We stopped at log \ ...@@ -2839,6 +2840,7 @@ the slave SQL thread with \"SLAVE START\". We stopped at log \
err: err:
VOID(pthread_mutex_lock(&LOCK_thread_count)); VOID(pthread_mutex_lock(&LOCK_thread_count));
thd->query = thd->db = 0; // extra safety thd->query = thd->db = 0; // extra safety
thd->query_length = 0;
VOID(pthread_mutex_unlock(&LOCK_thread_count)); VOID(pthread_mutex_unlock(&LOCK_thread_count));
thd->proc_info = "Waiting for slave mutex on exit"; thd->proc_info = "Waiting for slave mutex on exit";
pthread_mutex_lock(&rli->run_lock); pthread_mutex_lock(&rli->run_lock);
......
...@@ -360,7 +360,24 @@ public: ...@@ -360,7 +360,24 @@ public:
struct rand_struct rand; // used for authentication struct rand_struct rand; // used for authentication
struct system_variables variables; // Changeable local variables struct system_variables variables; // Changeable local variables
pthread_mutex_t LOCK_delete; // Locked before thd is deleted pthread_mutex_t LOCK_delete; // Locked before thd is deleted
/*
Note that (A) if we set query = NULL, we must at the same time set
query_length = 0, and protect the whole operation with the
LOCK_thread_count mutex. And (B) we are ONLY allowed to set query to a
non-NULL value if its previous value is NULL. We do not need to protect
operation (B) with any mutex. To avoid crashes in races, if we do not
know that thd->query cannot change at the moment, one should print
thd->query like this:
(1) reserve the LOCK_thread_count mutex;
(2) check if thd->query is NULL;
(3) if not NULL, then print at most thd->query_length characters from
it. We will see the query_length field as either 0, or the right value
for it.
Assuming that the write and read of an n-bit memory field in an n-bit
computer is atomic, we can avoid races in the above way.
This printing is needed at least in SHOW PROCESSLIST and SHOW INNODB
STATUS.
*/
char *query; // Points to the current query, char *query; // Points to the current query,
/* /*
A pointer to the stack frame of handle_one_connection(), A pointer to the stack frame of handle_one_connection(),
......
...@@ -95,6 +95,7 @@ int mysql_create_db(THD *thd, char *db, uint create_options, bool silent) ...@@ -95,6 +95,7 @@ int mysql_create_db(THD *thd, char *db, uint create_options, bool silent)
{ {
VOID(pthread_mutex_lock(&LOCK_thread_count)); VOID(pthread_mutex_lock(&LOCK_thread_count));
thd->query= 0; thd->query= 0;
thd->query_length= 0;
VOID(pthread_mutex_unlock(&LOCK_thread_count)); VOID(pthread_mutex_unlock(&LOCK_thread_count));
} }
send_ok(&thd->net, result); send_ok(&thd->net, result);
...@@ -202,6 +203,7 @@ int mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent) ...@@ -202,6 +203,7 @@ int mysql_rm_db(THD *thd,char *db,bool if_exists, bool silent)
{ {
VOID(pthread_mutex_lock(&LOCK_thread_count)); VOID(pthread_mutex_lock(&LOCK_thread_count));
thd->query= 0; thd->query= 0;
thd->query_length= 0;
VOID(pthread_mutex_unlock(&LOCK_thread_count)); VOID(pthread_mutex_unlock(&LOCK_thread_count));
} }
send_ok(&thd->net,(ulong) deleted); send_ok(&thd->net,(ulong) deleted);
......
...@@ -1312,6 +1312,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd, ...@@ -1312,6 +1312,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
thd->proc_info=0; thd->proc_info=0;
thd->command=COM_SLEEP; thd->command=COM_SLEEP;
thd->query=0; thd->query=0;
thd->query_length=0;
thread_running--; thread_running--;
VOID(pthread_mutex_unlock(&LOCK_thread_count)); VOID(pthread_mutex_unlock(&LOCK_thread_count));
thd->packet.shrink(thd->variables.net_buffer_length); // Reclaim some memory thd->packet.shrink(thd->variables.net_buffer_length); // Reclaim some memory
......
...@@ -1141,7 +1141,11 @@ void mysqld_list_processes(THD *thd,const char *user, bool verbose) ...@@ -1141,7 +1141,11 @@ void mysqld_list_processes(THD *thd,const char *user, bool verbose)
thd_info->query=0; thd_info->query=0;
if (tmp->query) if (tmp->query)
{ {
/* query_length is always set before tmp->query */ /*
query_length is always set to 0 when we set query = NULL; see
the comment in sql_class.h why this prevents crashes in possible
races with query_length
*/
uint length= min(max_query_length, tmp->query_length); uint length= min(max_query_length, tmp->query_length);
thd_info->query=(char*) thd->memdup(tmp->query,length+1); thd_info->query=(char*) thd->memdup(tmp->query,length+1);
thd_info->query[length]=0; thd_info->query[length]=0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment