Bug#19070633 - POSSIBLE ACCESS TO FREED MEMORY IN IS_FREE_LOCK()

                AND IS_USED_LOCK().

Analysis:
-----------
In functions Item_func_is_free_lock::val_int() and 
Item_func_is_used_lock::val_int(), for the specified user lock
name, pointer to its "User_level_lock" object is obtained from hash
"hash_user_locks". Mutex "LOCK_user_locks" is acquired for this
and released immediately. And we are accessing members of
User_level_lock after releasing the mutex. If same user lock is
deleted(released) from concurrent thread then accessing members
results in invalid(freed) memory access issue.

Deleting of user lock is also protected from the mutex
"LOCK_user_locks". Since this mutex is released in "val_int" 
functions mentioned above, delete operation proceeds while concurrent
thread tries to access its members.

With the test case, valgrind reports invalid read issues in val_int
functions.

Fix:
-----------
To fix this issue, in "val_int" function of classes
"Item_func_is_free_lock" and "Item_func_is_used_lock", now releasing
mutex "LOCK_user_locks" after accessing User_level_lock members.

3 files changed, 121 insertions, 8 deletions

diff --git a/mysql-test/r/lock_sync.result b/mysql-test/r/lock_sync.result
index 8fe94679e70..3877e70372c 100644
--- a/mysql-test/r/lock_sync.result
+++ b/mysql-test/r/lock_sync.result
@@ -773,3 +773,54 @@ Warning	1356	View 'test.v1' references invalid table(s) or column(s) or function
 SET DEBUG_SYNC= 'RESET';
 DROP VIEW v1;
 DROP TABLE t1;
+#
+# Bug#19070633 - POSSIBLE ACCESS TO FREED MEMORY IN IS_FREE_LOCK() AND IS_USED_LOCK().
+#
+# Verifying issue for IS_FREE_LOCK() function.
+SELECT GET_LOCK("lock_19070633", 600);
+GET_LOCK("lock_19070633", 600)
+1
+connect  con1, localhost, root,,;
+# Waiting after getting user level lock info and releasing mutex.
+SET DEBUG_SYNC= 'after_getting_user_level_lock_info SIGNAL parked WAIT_FOR go';
+# Sending: SELECT IS_FREE_LOCK("lock_19070633");
+SELECT IS_FREE_LOCK("lock_19070633");
+connection default;
+SET DEBUG_SYNC= 'now WAIT_FOR parked';
+SELECT RELEASE_LOCK("lock_19070633");
+RELEASE_LOCK("lock_19070633")
+1
+# Signaling connection con1 after releasing the lock.
+# Without fix, accessing user level lock info in con1 would result in
+# crash or valgrind issue invalid read is reported.
+SET DEBUG_SYNC= 'now SIGNAL go';
+connection con1;
+# Reaping: SELECT IS_FREE_LOCK("lock_19070633");
+IS_FREE_LOCK("lock_19070633")
+0
+connection default;
+# Verifying issue for IS_USED_LOCK() function.
+SELECT GET_LOCK("lock_19070633", 600);
+GET_LOCK("lock_19070633", 600)
+1
+connection con1;
+# Waiting after getting user level lock info and releasing mutex.
+SET DEBUG_SYNC= 'after_getting_user_level_lock_info SIGNAL parked WAIT_FOR go';
+# Sending: SELECT IS_USED_LOCK("lock_19070633");
+SELECT IS_USED_LOCK("lock_19070633");
+connection default;
+SET DEBUG_SYNC= 'now WAIT_FOR parked';
+SELECT RELEASE_LOCK("lock_19070633");
+RELEASE_LOCK("lock_19070633")
+1
+# Signaling connection con1 after releasing the lock.
+# Without fix, accessing user level lock info in con1 would result in
+# crash or valgrind issue invalid read is reported.
+SET DEBUG_SYNC= 'now SIGNAL go';
+connection con1;
+# Reaping: SELECT IS_USED_LOCK("lock_19070633");
+IS_USED_LOCK("lock_19070633")
+#
+connection default;
+SET DEBUG_SYNC= 'RESET';
+disconnect con1;
diff --git a/mysql-test/t/lock_sync.test b/mysql-test/t/lock_sync.test
index d5ad7becd7d..bcb78b5b600 100644
--- a/mysql-test/t/lock_sync.test
+++ b/mysql-test/t/lock_sync.test
@@ -1136,6 +1136,61 @@ DROP TABLE t1;
 disconnect con1;
 disconnect con2;
 
+--echo #
+--echo # Bug#19070633 - POSSIBLE ACCESS TO FREED MEMORY IN IS_FREE_LOCK() AND IS_USED_LOCK().
+--echo #
+
+--enable_connect_log
+
+--echo # Verifying issue for IS_FREE_LOCK() function.
+SELECT GET_LOCK("lock_19070633", 600);
+
+connect (con1, localhost, root,,);
+--echo # Waiting after getting user level lock info and releasing mutex.
+SET DEBUG_SYNC= 'after_getting_user_level_lock_info SIGNAL parked WAIT_FOR go';
+--echo # Sending: SELECT IS_FREE_LOCK("lock_19070633");
+send SELECT IS_FREE_LOCK("lock_19070633");
+
+connection default;
+SET DEBUG_SYNC= 'now WAIT_FOR parked';
+SELECT RELEASE_LOCK("lock_19070633");
+--echo # Signaling connection con1 after releasing the lock.
+--echo # Without fix, accessing user level lock info in con1 would result in
+--echo # crash or valgrind issue invalid read is reported.
+SET DEBUG_SYNC= 'now SIGNAL go';
+
+connection con1;
+--echo # Reaping: SELECT IS_FREE_LOCK("lock_19070633");
+--reap
+
+connection default;
+--echo # Verifying issue for IS_USED_LOCK() function.
+SELECT GET_LOCK("lock_19070633", 600);
+
+connection con1;
+--echo # Waiting after getting user level lock info and releasing mutex.
+SET DEBUG_SYNC= 'after_getting_user_level_lock_info SIGNAL parked WAIT_FOR go';
+--echo # Sending: SELECT IS_USED_LOCK("lock_19070633");
+send SELECT IS_USED_LOCK("lock_19070633");
+
+connection default;
+SET DEBUG_SYNC= 'now WAIT_FOR parked';
+SELECT RELEASE_LOCK("lock_19070633");
+--echo # Signaling connection con1 after releasing the lock.
+--echo # Without fix, accessing user level lock info in con1 would result in
+--echo # crash or valgrind issue invalid read is reported.
+SET DEBUG_SYNC= 'now SIGNAL go';
+
+connection con1;
+--echo # Reaping: SELECT IS_USED_LOCK("lock_19070633");
+--replace_column 1 #
+--reap
+
+connection default;
+SET DEBUG_SYNC= 'RESET';
+disconnect con1;
+
+--disable_connect_log
 
 # Check that all connections opened by test cases in this file are really
 # gone so execution of other tests won't be affected by their presence.
diff --git a/sql/item_func.cc b/sql/item_func.cc
index 3e3079f317f..96cf07550d8 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -6159,21 +6159,24 @@ longlong Item_func_is_free_lock::val_int()
   DBUG_ASSERT(fixed == 1);
   String *res=args[0]->val_str(&value);
   User_level_lock *ull;
+  longlong ret_val= 0LL;
 
   null_value=0;
   if (!res || !res->length())
   {
     null_value=1;
-    return 0;
+    return ret_val;
   }
   
   mysql_mutex_lock(&LOCK_user_locks);
   ull= (User_level_lock *) my_hash_search(&hash_user_locks, (uchar*) res->ptr(),
                                           (size_t) res->length());
-  mysql_mutex_unlock(&LOCK_user_locks);
   if (!ull || !ull->locked)
-    return 1;
-  return 0;
+    ret_val= 1;
+  mysql_mutex_unlock(&LOCK_user_locks);
+  DEBUG_SYNC(current_thd, "after_getting_user_level_lock_info");
+
+  return ret_val;
 }
 
 longlong Item_func_is_used_lock::val_int()
@@ -6181,6 +6184,7 @@ longlong Item_func_is_used_lock::val_int()
   DBUG_ASSERT(fixed == 1);
   String *res=args[0]->val_str(&value);
   User_level_lock *ull;
+  my_thread_id thread_id= 0UL;
 
   null_value=1;
   if (!res || !res->length())
@@ -6189,12 +6193,15 @@ longlong Item_func_is_used_lock::val_int()
   mysql_mutex_lock(&LOCK_user_locks);
   ull= (User_level_lock *) my_hash_search(&hash_user_locks, (uchar*) res->ptr(),
                                           (size_t) res->length());
+  if ((ull != NULL) && ull->locked)
+  {
+    null_value= 0;
+    thread_id= ull->thread_id;
+  }
   mysql_mutex_unlock(&LOCK_user_locks);
-  if (!ull || !ull->locked)
-    return 0;
+  DEBUG_SYNC(current_thd, "after_getting_user_level_lock_info");
 
-  null_value=0;
-  return ull->thread_id;
+  return thread_id;
 }