From 3d61c1399dc74e651030418730da429ee0b4e38a Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 10 Feb 2012 21:19:12 +0100 Subject: [PATCH] lp:910817: Race condition in kill_threads_for_user() The code was accessing a pointer in a mem_root that might be freed by another concurrent thread. Fix by moving the access to be done while the LOCK_thd_data is held, preventing the memory from being freed too early. --- sql/sql_parse.cc | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 1676d4a09f4..ef0f784e945 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -7363,13 +7363,23 @@ static uint kill_threads_for_user(THD *thd, LEX_USER *user, if (!threads_to_kill.is_empty()) { List_iterator_fast it(threads_to_kill); - THD *ptr; - while ((ptr= it++)) + THD *next_ptr; + THD *ptr= it++; + do { ptr->awake(kill_signal); + /* + Careful here: The list nodes are allocated on the memroots of the + THDs to be awakened. + But those THDs may be terminated and deleted as soon as we release + LOCK_thd_data, which will make the list nodes invalid. + Since the operation "it++" dereferences the "next" pointer of the + previous list node, we need to do this while holding LOCK_thd_data. + */ + next_ptr= it++; pthread_mutex_unlock(&ptr->LOCK_thd_data); (*rows)++; - } + } while ((ptr= next_ptr)); } DBUG_RETURN(0); }