Skip to content

Commit 3bf25a7

Browse files
committed
Bug#35509371 - Freed memory read in I_S.PROCESSLIST due to Race Condition with concurrent EVENT
Executing INFORMATION_SCHEMA.PROCESSLIST resulted in a reported issue while copying the user name from THD::security_context to the row buffer. While populating a row for a connection/session, the username, host and IP information are copied from THD::Security_context. THD::Security_context holds information describing the authenticated user. Stored Programs and views allow specifying DEFINER clause to utilize the definer's Security_context for execution. While executing a query, THD::security_context is switched from default to a definer's security_context. Switching security_context is not guarded with a mutex for now. This leads to a race condition, especially when concurrently populating the INFORMATION_SCHEMA. PROCESSLIST table. To address race condition within I_S.PROCESSLIST with security_context switches, now security_context switch is guarded with a mutex. Please note that, this approach addresses only the specific race condition arising from concurrent thread switchig context and I_S.PROCESSLIST execution. It does not comprehensively secure access to the security_context by employing mutexes for all read/write operations. Change-Id: Ida1512a17109853438a180322fd1f88cfd75ddfa
1 parent b6f2e37 commit 3bf25a7

File tree

3 files changed

+33
-3
lines changed

3 files changed

+33
-3
lines changed

sql/sp_head.cc

+2
Original file line numberDiff line numberDiff line change
@@ -3079,6 +3079,8 @@ bool sp_head::execute_procedure(THD *thd, mem_root_deque<Item *> *args) {
30793079
Security_context *save_security_ctx = nullptr;
30803080
if (!err_status) err_status = set_security_ctx(thd, &save_security_ctx);
30813081

3082+
DEBUG_SYNC(thd, "after_switching_security_context");
3083+
30823084
opt_trace_disable_if_no_stored_proc_func_access(thd, this);
30833085

30843086
#ifdef HAVE_PSI_SP_INTERFACE

sql/sql_class.h

+12-1
Original file line numberDiff line numberDiff line change
@@ -1322,7 +1322,18 @@ class THD : public MDL_context_owner,
13221322
Security_context *m_security_ctx;
13231323

13241324
Security_context *security_context() const { return m_security_ctx; }
1325-
void set_security_context(Security_context *sctx) { m_security_ctx = sctx; }
1325+
void set_security_context(Security_context *sctx) {
1326+
if (sctx == m_security_ctx) return;
1327+
1328+
/*
1329+
To prevent race conditions arising from concurrent threads executing
1330+
I_S.PROCESSLIST, a mutex LOCK_thd_security_ctx safeguards the security
1331+
context switch.
1332+
*/
1333+
mysql_mutex_lock(&LOCK_thd_security_ctx);
1334+
m_security_ctx = sctx;
1335+
mysql_mutex_unlock(&LOCK_thd_security_ctx);
1336+
}
13261337
List<Security_context> m_view_ctx_list;
13271338

13281339
/**

sql/sql_show.cc

+19-2
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,9 @@
111111
#include "sql/query_result.h"
112112
#include "sql/rpl_source.h"
113113
#include "sql/set_var.h"
114-
#include "sql/sp.h" // sp_cache_routine
115-
#include "sql/sp_head.h" // sp_head
114+
#include "sql/sp.h" // sp_cache_routine
115+
#include "sql/sp_head.h" // sp_head
116+
#include "sql/sp_rcontext.h"
116117
#include "sql/sql_base.h" // close_thread_tables
117118
#include "sql/sql_bitmap.h"
118119
#include "sql/sql_check_constraint.h"
@@ -2840,6 +2841,14 @@ class List_process_list : public Do_THD_Impl {
28402841
return;
28412842
}
28422843

2844+
DBUG_EXECUTE_IF(
2845+
"enable_debug_sync_after_reading_sp_sctx",
2846+
if (inspect_thd->sp_runtime_ctx != nullptr &&
2847+
(strcmp(inspect_thd->sp_runtime_ctx->sp->m_name.str, "proc") ==
2848+
0)) {
2849+
DEBUG_SYNC(m_client_thd, "after_reading_security_context");
2850+
});
2851+
28432852
thd_info = new (m_client_thd->mem_root) thread_info;
28442853

28452854
/* ID */
@@ -3088,6 +3097,14 @@ class Fill_process_list : public Do_THD_Impl {
30883097
return;
30893098
}
30903099

3100+
DBUG_EXECUTE_IF(
3101+
"enable_debug_sync_after_reading_sp_sctx",
3102+
if (inspect_thd->sp_runtime_ctx != nullptr &&
3103+
(strcmp(inspect_thd->sp_runtime_ctx->sp->m_name.str, "proc") ==
3104+
0)) {
3105+
DEBUG_SYNC(m_client_thd, "after_reading_security_context");
3106+
});
3107+
30913108
DBUG_EXECUTE_IF(
30923109
"test_fill_proc_with_x_root",
30933110
if (0 == strcmp(inspect_sctx_user.str, "x_root")) {

0 commit comments

Comments
 (0)