summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPraveenkumar Hulakund <praveenkumar.hulakund@oracle.com>2013-08-21 10:39:40 +0530
committerPraveenkumar Hulakund <praveenkumar.hulakund@oracle.com>2013-08-21 10:39:40 +0530
commit10a6aa256e965874b3a307f7d81393078a991544 (patch)
tree6c403dc88c8ca5b75a5384ae957f26dc42b865f8
parent55129f676accff001fcd7b60b049cdd0932442e8 (diff)
downloadmariadb-git-10a6aa256e965874b3a307f7d81393078a991544.tar.gz
Bug#11765252 - READ OF FREED MEMORY WHEN "USE DB" AND
"SHOW PROCESSLIST" Analysis: ---------- The problem here is, if one connection changes its default db and at the same time another connection executes "SHOW PROCESSLIST", when it wants to read db of the another connection then there is a chance of accessing the invalid memory. The db name stored in THD is not guarded while changing user DB and while reading the user DB in "SHOW PROCESSLIST". So, if THD.db is freed by thd "owner" thread and if another thread executing "SHOW PROCESSLIST" statement tries to read and copy THD.db at the same time then we may endup in the issue reported here. Fix: ---------- Used mutex "LOCK_thd_data" to guard THD.db while freeing it and while copying it to processlist.
-rw-r--r--mysql-test/include/have_valgrind.inc11
-rw-r--r--sql/sql_class.h9
-rw-r--r--sql/sql_db.cc10
-rw-r--r--sql/sql_parse.cc2
-rw-r--r--sql/sql_show.cc25
5 files changed, 41 insertions, 16 deletions
diff --git a/mysql-test/include/have_valgrind.inc b/mysql-test/include/have_valgrind.inc
new file mode 100644
index 00000000000..8374c2ed4f0
--- /dev/null
+++ b/mysql-test/include/have_valgrind.inc
@@ -0,0 +1,11 @@
+# include/have_valgrind.inc
+#
+# If some test should be run with only valgrind then skip it while running test
+# without it.
+#
+
+if (!$VALGRIND_TEST) {
+ --skip Need "--valgrind"
+}
+
+
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 0942e9274b1..9a9b2058e2b 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -1,5 +1,5 @@
/*
- Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.
+ Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -2284,6 +2284,12 @@ public:
*/
bool set_db(const char *new_db, size_t new_db_len)
{
+ /*
+ Acquiring mutex LOCK_thd_data as we either free the memory allocated
+ for the database and reallocate the memory for the new db or memcpy
+ the new_db to the db.
+ */
+ pthread_mutex_lock(&LOCK_thd_data);
/* Do not reallocate memory if current chunk is big enough. */
if (db && new_db && db_length >= new_db_len)
memcpy(db, new_db, new_db_len+1);
@@ -2293,6 +2299,7 @@ public:
db= new_db ? my_strndup(new_db, new_db_len, MYF(MY_WME)) : NULL;
}
db_length= db ? new_db_len : 0;
+ pthread_mutex_unlock(&LOCK_thd_data);
return new_db && !db;
}
diff --git a/sql/sql_db.cc b/sql/sql_db.cc
index d3178fa0b99..4ddfad7206e 100644
--- a/sql/sql_db.cc
+++ b/sql/sql_db.cc
@@ -1,5 +1,5 @@
/*
- Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.
+ Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -1469,10 +1469,12 @@ static void mysql_change_db_impl(THD *thd,
we just call THD::reset_db(). Since THD::reset_db() does not releases
the previous database name, we should do it explicitly.
*/
-
- x_free(thd->db);
-
+ pthread_mutex_lock(&thd->LOCK_thd_data);
+ if (thd->db)
+ x_free(thd->db);
+ DEBUG_SYNC(thd, "after_freeing_thd_db");
thd->reset_db(new_db_name->str, new_db_name->length);
+ pthread_mutex_unlock(&thd->LOCK_thd_data);
}
/* 2. Update security context. */
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 457be355f81..d9d9a603869 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -1231,7 +1231,9 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
if (save_user_connect)
decrease_user_connections(save_user_connect);
#endif /* NO_EMBEDDED_ACCESS_CHECKS */
+ pthread_mutex_lock(&thd->LOCK_thd_data);
x_free(save_db);
+ pthread_mutex_unlock(&thd->LOCK_thd_data);
x_free(save_security_ctx.user);
}
break;
diff --git a/sql/sql_show.cc b/sql/sql_show.cc
index a72ca4d1bfc..2ee074db884 100644
--- a/sql/sql_show.cc
+++ b/sql/sql_show.cc
@@ -1,4 +1,4 @@
-/* Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2000, 2013, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -1847,8 +1847,6 @@ void mysqld_list_processes(THD *thd,const char *user, bool verbose)
thd_info->host= thd->strdup(tmp_sctx->host_or_ip[0] ?
tmp_sctx->host_or_ip :
tmp_sctx->host ? tmp_sctx->host : "");
- if ((thd_info->db=tmp->db)) // Safe test
- thd_info->db=thd->strdup(thd_info->db);
thd_info->command=(int) tmp->command;
if ((mysys_var= tmp->mysys_var))
pthread_mutex_lock(&mysys_var->mutex);
@@ -1871,6 +1869,7 @@ void mysqld_list_processes(THD *thd,const char *user, bool verbose)
pthread_mutex_unlock(&mysys_var->mutex);
thd_info->start_time= tmp->start_time;
+
thd_info->query=0;
/* Lock THD mutex that protects its data when looking at it. */
pthread_mutex_lock(&tmp->LOCK_thd_data);
@@ -1879,7 +1878,11 @@ void mysqld_list_processes(THD *thd,const char *user, bool verbose)
uint length= min(max_query_length, tmp->query_length());
thd_info->query= (char*) thd->strmake(tmp->query(),length);
}
+
+ if ((thd_info->db= tmp->db)) // Safe test
+ thd_info->db= thd->strdup(thd_info->db);
pthread_mutex_unlock(&tmp->LOCK_thd_data);
+
thread_infos.append(thd_info);
}
}
@@ -1934,7 +1937,7 @@ int fill_schema_processlist(THD* thd, TABLE_LIST* tables, COND* cond)
{
Security_context *tmp_sctx= tmp->security_ctx;
struct st_my_thread_var *mysys_var;
- const char *val;
+ const char *val, *db;
if ((!tmp->vio_ok() && !tmp->system_thread) ||
(user && (!tmp_sctx->user || strcmp(tmp_sctx->user, user))))
@@ -1959,13 +1962,6 @@ int fill_schema_processlist(THD* thd, TABLE_LIST* tables, COND* cond)
else
table->field[2]->store(tmp_sctx->host_or_ip,
strlen(tmp_sctx->host_or_ip), cs);
- /* DB */
- if (tmp->db)
- {
- table->field[3]->store(tmp->db, strlen(tmp->db), cs);
- table->field[3]->set_notnull();
- }
-
if ((mysys_var= tmp->mysys_var))
pthread_mutex_lock(&mysys_var->mutex);
/* COMMAND */
@@ -2011,6 +2007,13 @@ int fill_schema_processlist(THD* thd, TABLE_LIST* tables, COND* cond)
tmp->query_length()), cs);
table->field[7]->set_notnull();
}
+
+ /* DB */
+ if ((db= tmp->db))
+ {
+ table->field[3]->store(db, strlen(db), cs);
+ table->field[3]->set_notnull();
+ }
pthread_mutex_unlock(&tmp->LOCK_thd_data);
if (schema_table_store_record(thd, table))