PS-5952, PS-5956: Fix utility user bugs

PS-5952: Utility user visible in performance_schema.threads PS-5956: Root session must not be allowed to kill Utility user session
dutow · Nov 7, 2019 · c6ac4db · c6ac4db
1 parent cef861f
commit c6ac4db
Show file tree

Hide file tree

Showing 7 changed files with 35 additions and 3 deletions.
diff --git a/mysql-test/r/percona_utility_user.result b/mysql-test/r/percona_utility_user.result
@@ -129,4 +129,9 @@ PROCESSLIST_ID	ATTR_NAME	ATTR_VALUE	ORDINAL_POSITION
 SELECT COUNT(DISTINCT PROCESSLIST_ID) FROM performance_schema.session_connect_attrs;
 COUNT(DISTINCT PROCESSLIST_ID)
 1
+SELECT COUNT(*) from performance_schema.threads where type='FOREGROUND';
+COUNT(*)
+1
+KILL 2;
+ERROR HY000: You are not owner of thread 2
 REVOKE PROXY ON 'frank'@'%' FROM 'root'@'localhost';
diff --git a/mysql-test/t/percona_utility_user.test b/mysql-test/t/percona_utility_user.test
@@ -184,7 +184,18 @@ SELECT * FROM performance_schema.accounts WHERE USER="frank";
 SELECT * FROM performance_schema.session_account_connect_attrs;
 SELECT COUNT(DISTINCT PROCESSLIST_ID) FROM performance_schema.session_connect_attrs;
 
+# PS-5952: Utility user visible in performance_schema.threads
+SELECT COUNT(*) from performance_schema.threads where type='FOREGROUND';
+
+
+# PS-5956: Root session must not be allowed to kill Utility user session
+--let conn_id=`select connection_id()`
+
 connection default;
+
+--error ER_KILL_DENIED_ERROR
+--eval KILL $conn_id
+
 disconnect frank;
 
 REVOKE PROXY ON 'frank'@'%' FROM 'root'@'localhost';

diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
@@ -8022,7 +8022,13 @@ uint kill_one_thread(THD *thd, ulong id, bool only_kill_query)
       slayage if both are string-equal.
     */
 
-    if ((thd->security_ctx->master_access & SUPER_ACL) ||
+    const bool is_utility_connection =
+        acl_is_utility_user(tmp->security_ctx->priv_user,
+                            tmp->security_ctx->get_host()->ptr(),
+                            tmp->security_ctx->get_ip()->ptr());
+
+
+    if (((thd->security_ctx->master_access & SUPER_ACL) && !is_utility_connection) ||
         thd->security_ctx->user_matches(tmp->security_ctx))
     {
       /* process the kill only if thread is not already undergoing any kill

diff --git a/storage/perfschema/pfs.cc b/storage/perfschema/pfs.cc
@@ -2042,6 +2042,7 @@ static void set_thread_account_v1(const char *user, int user_len,
     so we keep this pfs session dirty. This fixes many, but not all tables.
     The remaining seems to honor m_enabled, so we also set that to false. */
     pfs->m_enabled= false;
+    pfs->m_disable_instrumentation = true;
     return;
   }
 

diff --git a/storage/perfschema/pfs_instr.cc b/storage/perfschema/pfs_instr.cc
@@ -960,6 +960,7 @@ PFS_thread* create_thread(PFS_thread_class *klass, const void *identity,
         pfs->m_stmt_lock.set_allocated();
         pfs->m_session_lock.set_allocated();
         pfs->m_enabled= true;
+        pfs->m_disable_instrumentation= false;
         pfs->m_class= klass;
         pfs->m_events_waits_current= & pfs->m_events_waits_stack[WAIT_STACK_BOTTOM];
         pfs->m_waits_history_full= false;

diff --git a/storage/perfschema/pfs_instr.h b/storage/perfschema/pfs_instr.h
@@ -376,6 +376,7 @@ struct PFS_ALIGNED PFS_thread : PFS_connection_slice
 
   /** Thread instrumentation flag. */
   bool m_enabled;
+  bool m_disable_instrumentation;
   /** Current wait event in the event stack. */
   PFS_events_waits *m_events_waits_current;
   /** Event ID counter */

diff --git a/storage/perfschema/table_threads.cc b/storage/perfschema/table_threads.cc
@@ -231,10 +231,13 @@ void table_threads::make_row(PFS_thread *pfs)
     m_row.m_processlist_state_length= 0;
   }
 
-  m_row.m_enabled_ptr= &pfs->m_enabled;
+  m_row.m_enabled_ptr= pfs->m_disable_instrumentation ? NULL : &pfs->m_enabled;
 
   if (pfs->m_lock.end_optimistic_lock(& lock))
     m_row_exists= true;
+
+  if(pfs->m_disable_instrumentation)
+    m_row_exists= false;
 }
 
 int table_threads::read_row_values(TABLE *table,
@@ -352,7 +355,8 @@ int table_threads::read_row_values(TABLE *table,
         f->set_null();
         break;
       case 13: /* INSTRUMENTED */
-        set_field_enum(f, (*m_row.m_enabled_ptr) ? ENUM_YES : ENUM_NO);
+        set_field_enum(f, (m_row.m_enabled_ptr && *m_row.m_enabled_ptr)
+			? ENUM_YES : ENUM_NO);
         break;
       default:
         DBUG_ASSERT(false);
@@ -391,6 +395,9 @@ int table_threads::update_row_values(TABLE *table,
       case 12: /* ROLE */
         return HA_ERR_WRONG_COMMAND;
       case 13: /* INSTRUMENTED */
+        if(m_row.m_enabled_ptr == NULL) {
+          return HA_ERR_WRONG_COMMAND;
+        }
         value= (enum_yes_no) get_field_enum(f);
         *m_row.m_enabled_ptr= (value == ENUM_YES) ? true : false;
         break;