Bug#22136709 INFINITE RECURSION BY CALLING MY_MESSAGE FROM MYSQL_AUDIT_GENERAL_CLASS HANDLER

Problem:
========
Calling my_message from within an audit plugin during handling MYSQL_AUDIT_GENERAL_ERROR event
causes the audit error event generating mechanism to fall into infinite recursion, which ends
with the server crash.

Fix:
====
A condition has been embedded into event generating mechanism that stops infinite recursion,
when the stack size shrinks to a certain value. This fix does not prevent infinite recursion
but stops it instead. The server does not crash and the ER_STACK_OVERRUN_NEED_MORE error is
generated.

null_audit_event_order_check_consume_ignore_count global variable was introduced that prevents
clearing null_audit_event_order_check variable certain number of times. This allows to simulate
infinite and non-infinite recursion during testing.

Reviewed-by:
============
Georgi Kodinov <georgi.kodinov@oracle.com>
Ramil Kalimullin <ramil.kalimullin@oracle.com>
Evgeny Potemkin <evgeny.potemkin@oracle.com>

Author: maras007

mysql-test/r/audit_plugin_bugs.result

@@ -7,3 +7,30 @@
 # Load the plugin at startup and abort on STARTUP event with custom message
 # Search for custom abort message
 # Startup the server
+#
+# Bug #22136709: INFINITE RECURSION BY CALLING MY_MESSAGE FROM
+#   MYSQL_AUDIT_GENERAL_CLASS HANDLER
+INSTALL PLUGIN null_audit SONAME 'adt_null.so';
+SET @@GLOBAL.null_audit_event_order_check= "MYSQL_AUDIT_GENERAL_ERROR;;ABORT_RET";
+SET @@GLOBAL.null_audit_abort_message= "Abort message.";
+SET @@GLOBAL.null_audit_event_order_check_consume_ignore_count= 2;
+connect(localhost,wrong_root,,test,MASTER_PORT,MASTER_SOCKET);
+ERROR HY000: Abort message.
+SET @@GLOBAL.null_audit_event_order_check_consume_ignore_count= 10000;
+connect(localhost,wrong_root,,test,MASTER_PORT,MASTER_SOCKET);
+SET @@GLOBAL.null_audit_event_order_check= NULL;
+SET @@GLOBAL.null_audit_abort_message= NULL;
+SET @@GLOBAL.null_audit_event_order_check_consume_ignore_count= 0;
+# test
+SET @@null_audit_event_order_check= "MYSQL_AUDIT_GENERAL_ERROR;;ABORT_RET";
+SET @@null_audit_abort_message= "Abort message.";
+SET @@null_audit_event_order_check_consume_ignore_count= 10000;
+# Must not crash
+SELECT 1 FROM mysql.fictional_table;
+ERROR HY000: Thread stack overrun:  <NUMBER> used of a <NUMBER> stack, and <NUMBER> needed.  Use 'mysqld --thread_stack=#' to specify a bigger stack.
+# Clean up
+End of 5.7 tests
+# cleanup
+UNINSTALL PLUGIN null_audit;
+Warnings:
+Warning	1620	Plugin is busy and will be uninstalled on shutdown

mysql-test/t/audit_plugin_bugs.test

@@ -54,3 +54,61 @@ let SEARCH_PATTERN= \[ERROR\] Abort message custom;
 --enable_reconnect
 --source include/wait_until_connected_again.inc
 --disable_reconnect
+
+--echo #
+--echo # Bug #22136709: INFINITE RECURSION BY CALLING MY_MESSAGE FROM
+--echo #   MYSQL_AUDIT_GENERAL_CLASS HANDLER
+
+--replace_regex /\.dll/.so/
+eval INSTALL PLUGIN null_audit SONAME '$AUDIT_NULL';
+
+# Save the initial number of concurrent sessions
+--source include/count_sessions.inc
+
+SET @@GLOBAL.null_audit_event_order_check= "MYSQL_AUDIT_GENERAL_ERROR;;ABORT_RET";
+SET @@GLOBAL.null_audit_abort_message= "Abort message.";
+
+# 3 recursive my_message calls should not cause stack overrun
+SET @@GLOBAL.null_audit_event_order_check_consume_ignore_count= 2;
+--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
+--error ER_AUDIT_API_ABORT
+connect(user1_con,localhost,wrong_root,);
+
+# Infinite my_message calls cause stack overrun
+SET @@GLOBAL.null_audit_event_order_check_consume_ignore_count= 10000;
+--replace_result $MASTER_MYSOCK MASTER_SOCKET $MASTER_MYPORT MASTER_PORT
+--disable_result_log
+--error ER_STACK_OVERRUN_NEED_MORE
+connect(user1_con,localhost,wrong_root,);
+--enable_result_log
+
+# Clean up global variables
+SET @@GLOBAL.null_audit_event_order_check= NULL;
+SET @@GLOBAL.null_audit_abort_message= NULL;
+# Let's hope 10000 calls will overrun the stack frame
+SET @@GLOBAL.null_audit_event_order_check_consume_ignore_count= 0;
+
+--echo # test
+connect(user1_con,localhost,root,);
+
+SET @@null_audit_event_order_check= "MYSQL_AUDIT_GENERAL_ERROR;;ABORT_RET";
+SET @@null_audit_abort_message= "Abort message.";
+# Let's hope 10000 calls will overrun the stack frame
+SET @@null_audit_event_order_check_consume_ignore_count= 10000;
+
+--echo # Must not crash
+--replace_regex /[0-9]+ byte[s]?/<NUMBER>/
+--error ER_STACK_OVERRUN_NEED_MORE
+SELECT 1 FROM mysql.fictional_table;
+
+--echo # Clean up
+connection default;
+disconnect user1_con;
+
+# Wait till we reached the initial number of concurrent sessions
+--source include/wait_until_count_sessions.inc
+
+--echo End of 5.7 tests
+
+--echo # cleanup
+UNINSTALL PLUGIN null_audit;

plugin/audit_null/audit_null.c

@@ -127,6 +127,12 @@ static MYSQL_THDVAR_STR(event_order_check,
                         PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_MEMALLOC,
                         "Event order check string", NULL, NULL, NULL);
 
+static MYSQL_THDVAR_UINT(event_order_check_consume_ignore_count,
+                         PLUGIN_VAR_RQCMDARG,
+                         "Do not consume event order string specified "
+                         "number of times.",
+                         NULL, NULL, 0, 0, UINT_MAX, 1);
+
 static MYSQL_THDVAR_INT(event_order_started,
                         PLUGIN_VAR_RQCMDARG,
                         "Plugin is in the event order check.",
@@ -316,7 +322,8 @@ static void process_event_record(MYSQL_THD thd, LEX_CSTRING event_name,
   }
 }
 
-static int process_command(MYSQL_THD thd, LEX_CSTRING event_command)
+static int process_command(MYSQL_THD thd, LEX_CSTRING event_command,
+                           my_bool consume_event)
 {
   LEX_CSTRING abort_ret_command= { C_STRING_WITH_LEN("ABORT_RET") };
 
@@ -334,11 +341,15 @@ static int process_command(MYSQL_THD thd, LEX_CSTRING event_command)
     lex_cstring_set(&order_cstr, 
                     (const char *)THDVAR(thd, event_order_check));
 
-    memmove((char *)order_cstr.str,
-            (void *)status.str, status.length + 1);
+    /* Do not replace order string yet. */
+    if (consume_event)
+    {
+      memmove((char *) order_cstr.str,
+              (void *) status.str, status.length + 1);
 
-    THDVAR(thd, abort_value) = 1;
-    THDVAR(thd, abort_message) = 0;
+      THDVAR(thd, abort_value)= 1;
+      THDVAR(thd, abort_message)= 0;
+    }
 
     if (err_message)
     {
@@ -376,6 +387,7 @@ static int audit_null_notify(MYSQL_THD thd,
   LEX_CSTRING event_token= get_token(&order_str);
   LEX_CSTRING event_data= get_token(&order_str);
   LEX_CSTRING event_command= get_token(&order_str);
+  my_bool consume_event= TRUE;
 
   /* prone to races, oh well */
   number_of_calls++;
@@ -674,31 +686,45 @@ static int audit_null_notify(MYSQL_THD thd,
     else
     {
       LEX_CSTRING order_cstr;
+      ulong consume= THDVAR(thd, event_order_check_consume_ignore_count);
       lex_cstring_set(&order_cstr,
                       (const char *)THDVAR(thd, event_order_check));
 
       THDVAR(thd, event_order_started)= 1;
 
-      memmove((char*)order_cstr.str, (void*)order_str,
-        order_cstr.length - (order_str - order_cstr.str) + 1);
+      if (consume)
+      {
+        /*
+          Do not consume event this time. Just decrease value and wait until
+          the next event is matched.
+        */
+        THDVAR(thd, event_order_check_consume_ignore_count)= consume - 1;
+        consume_event= FALSE;
+      }
+      else
+      {
+        /* Consume matched event. */
+        memmove((char*)order_cstr.str, (void*)order_str,
+          order_cstr.length - (order_str - order_cstr.str) + 1);
 
-      /* Count new length. */
-      lex_cstring_set(&order_cstr, order_cstr.str);
+        /* Count new length. */
+        lex_cstring_set(&order_cstr, order_cstr.str);
 
-      if (order_cstr.length == 0)
-      {
-        LEX_CSTRING status = { C_STRING_WITH_LEN("EVENT-ORDER-OK") };
+        if (order_cstr.length == 0)
+        {
+          LEX_CSTRING status = { C_STRING_WITH_LEN("EVENT-ORDER-OK") };
 
-        memmove((char *)order_cstr.str,
-                (void *)status.str, status.length + 1);
+          memmove((char *)order_cstr.str,
+                  (void *)status.str, status.length + 1);
 
-        /* event_order_started contains message. Do not verify it. */
-        THDVAR(thd, event_order_started)= 0;
+          /* event_order_started contains message. Do not verify it. */
+          THDVAR(thd, event_order_started)= 0;
+        }
       }
     }
   }
 
-  return process_command(thd, event_command);
+  return process_command(thd, event_command, consume_event);
 }
 
 /*
@@ -729,6 +755,7 @@ static struct st_mysql_sys_var* system_variables[] = {
   MYSQL_SYSVAR(abort_value),
 
   MYSQL_SYSVAR(event_order_check),
+  MYSQL_SYSVAR(event_order_check_consume_ignore_count),
   MYSQL_SYSVAR(event_order_started),
   MYSQL_SYSVAR(event_order_check_exact),
 

sql/auth/sql_authentication.cc

@@ -2201,7 +2201,8 @@ acl_authenticate(THD *thd, enum_server_command command)
     acl_log_connect(mpvio.auth_info.user_name, mpvio.auth_info.host_or_ip,
       mpvio.auth_info.authenticated_as, mpvio.db.str, thd, command);
   }
-  if (!mpvio.can_authenticate() && res == CR_OK)
+  if (res == CR_OK &&
+      (!mpvio.can_authenticate() || thd->is_error()))
   {
     res= CR_ERROR;
   }
@@ -2259,6 +2260,9 @@ acl_authenticate(THD *thd, enum_server_command command)
         mpvio.auth_info.authenticated_as, mpvio.db.str, thd, command);
     }
 
+    if (thd->is_error())
+      DBUG_RETURN(1);
+
     if (is_proxy_user)
     {
       ACL_USER *acl_proxy_user;

sql/mysqld.cc

@@ -2215,7 +2215,8 @@ void my_message_sql(uint error, const char *str, myf MyFlags)
   }
 
 #ifndef EMBEDDED_LIBRARY
-  mysql_audit_general(thd, MYSQL_AUDIT_GENERAL_ERROR, error, str);
+  if (error != ER_STACK_OVERRUN_NEED_MORE)
+    mysql_audit_general(thd, MYSQL_AUDIT_GENERAL_ERROR, error, str);
 #endif
 
   if (thd)

sql/sql_audit.cc

@@ -21,6 +21,7 @@
 #include "sql_thd_internal_api.h"               // create_thd / destroy_thd
 #include "sql_plugin.h"                         // my_plugin_foreach
 #include "sql_rewrite.h"                        // mysql_rewrite_query
+#include "sql_parse.h"                          // check_stack_overrun
 
 /**
   @class Audit_error_handler
@@ -1259,6 +1260,19 @@ static int event_class_dispatch(THD *thd, mysql_event_class_t event_class,
   {
     plugin_ref *plugins, *plugins_last;
 
+    /*
+      Does not allow infinite recursive calls that crash the server.
+      This happens when error is reported from within a plugin that already
+      is receiving error event (MYSQL_AUDIT_GENERAL_ERROR). This condition
+      breaks the recursion, when the stack size gets close to its minimal
+      value.
+    */
+    if (check_stack_overrun(thd, STACK_MIN_SIZE * 5,
+                            reinterpret_cast<uchar *>(&event_generic)))
+    {
+      return 0;
+    }
+
     /* Use the cached set of audit plugins */
     plugins= thd->audit_class_plugins.begin();
     plugins_last= thd->audit_class_plugins.end();

sql/srv_session_service.cc

@@ -1,4 +1,4 @@
-/*  Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+/*  Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
 
     This program is free software; you can redistribute it and/or
     modify it under the terms of the GNU General Public License as
@@ -102,11 +102,24 @@ Srv_session* srv_session_open(srv_session_error_cb error_cb, void *plugin_ctx)
   }
   else
   {
-    if (session->open())
+    THD *current= current_thd;
+    THD *stack_thd= session->get_thd();
+
+    session->get_thd()->thread_stack= reinterpret_cast<char *>(&stack_thd);
+    session->get_thd()->store_globals();
+
+    bool result= session->open();
+
+    session->get_thd()->restore_globals();
+
+    if (result)
     {
       delete session;
       session= NULL;
     }
+
+    if (current)
+      current->store_globals();
   }
   DBUG_RETURN(session);
 }