From 508c74ac25f27caef8103ab08c34665a82b82b9d Mon Sep 17 00:00:00 2001 From: Praveenkumar Hulakund Date: Tue, 16 Sep 2014 11:28:46 +0530 Subject: [PATCH] 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. --- mysql-test/r/lock_sync.result | 51 ++++++++++++++++++++++++++++++++ mysql-test/t/lock_sync.test | 55 +++++++++++++++++++++++++++++++++++ sql/item_func.cc | 23 ++++++++++----- 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; }