MDEV-33216 stack-use-after-return in Wsrep_schema_impl::open_table()

Fix a case of stack-use-after-return reported by ASAN in
Wsrep_schema_impl::open_table(). This function has a stack allocated
TABLE_LIST object and return TABLE_LIST::table to the caller.
Changed the function to take a TABLE_LIST pointer as argument.
Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>

sql/wsrep_schema.cc

@@ -245,58 +245,51 @@ static void finish_stmt(THD* thd) {
   close_thread_tables(thd);
 }
 
-static int open_table(THD* thd,
-               const LEX_CSTRING *schema_name,
-               const LEX_CSTRING *table_name,
-               enum thr_lock_type const lock_type,
-               TABLE** table) {
-  assert(table);
-  *table= NULL;
-
+static int open_table(THD *thd, const LEX_CSTRING *schema_name,
+                      const LEX_CSTRING *table_name,
+                      enum thr_lock_type const lock_type,
+                      TABLE_LIST *table_list)
+{
+  assert(table_list);
   DBUG_ENTER("Wsrep_schema::open_table()");
-
-  TABLE_LIST tables;
-  uint flags= (MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK |
-               MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY |
-               MYSQL_OPEN_IGNORE_FLUSH |
-               MYSQL_LOCK_IGNORE_TIMEOUT);
-
-  tables.init_one_table(schema_name,
-                        table_name,
-                        NULL, lock_type);
+  const uint flags= (MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK |
+                     MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY |
+                     MYSQL_OPEN_IGNORE_FLUSH | MYSQL_LOCK_IGNORE_TIMEOUT);
+  table_list->init_one_table(schema_name, table_name, NULL, lock_type);
   thd->lex->query_tables_own_last= 0;
-
-  if (!open_n_lock_single_table(thd, &tables, tables.lock_type, flags)) {
+  if (!open_n_lock_single_table(thd, table_list, table_list->lock_type, flags))
+  {
     close_thread_tables(thd);
     DBUG_RETURN(1);
   }
 
-  *table= tables.table;
-  (*table)->use_all_columns();
+  table_list->table->use_all_columns();
 
   DBUG_RETURN(0);
 }
 
-
-static int open_for_write(THD* thd, const char* table_name, TABLE** table) {
+static int open_for_write(THD* thd, const char* table_name, TABLE_LIST* table_list)
+{
   LEX_CSTRING schema_str= { wsrep_schema_str.c_str(), wsrep_schema_str.length() };
   LEX_CSTRING table_str= { table_name, strlen(table_name) };
   if (Wsrep_schema_impl::open_table(thd, &schema_str, &table_str, TL_WRITE,
-                                    table)) {
+                                    table_list))
+  {
     // No need to log an error if the query was bf aborted,
     // thd client will get ER_LOCK_DEADLOCK in the end.
     const bool interrupted= thd->killed ||
       (thd->is_error() &&
        (thd->get_stmt_da()->sql_errno() == ER_QUERY_INTERRUPTED));
-    if (!interrupted) {
+    if (!interrupted)
+    {
       WSREP_ERROR("Failed to open table %s.%s for writing",
                   schema_str.str, table_name);
     }
     return 1;
   }
-  empty_record(*table);
-  (*table)->use_all_columns();
-  restore_record(*table, s->default_values);
+  empty_record(table_list->table);
+  table_list->table->use_all_columns();
+  restore_record(table_list->table, s->default_values);
   return 0;
 }
 
@@ -438,19 +431,21 @@ static int delete_row(TABLE* table) {
   return 0;
 }
 
-static int open_for_read(THD* thd, const char* table_name, TABLE** table) {
-
+static int open_for_read(THD *thd, const char *table_name,
+                         TABLE_LIST *table_list)
+{
   LEX_CSTRING schema_str= { wsrep_schema_str.c_str(), wsrep_schema_str.length() };
   LEX_CSTRING table_str= { table_name, strlen(table_name) };
   if (Wsrep_schema_impl::open_table(thd, &schema_str, &table_str, TL_READ,
-                                    table)) {
+                                    table_list))
+  {
     WSREP_ERROR("Failed to open table %s.%s for reading",
                 schema_str.str, table_name);
     return 1;
   }
-  empty_record(*table);
-  (*table)->use_all_columns();
-  restore_record(*table, s->default_values);
+  empty_record(table_list->table);
+  table_list->table->use_all_columns();
+  restore_record(table_list->table, s->default_values);
   return 0;
 }
 
@@ -706,8 +701,10 @@ int Wsrep_schema::store_view(THD* thd, const Wsrep_view& view)
   assert(view.status() == Wsrep_view::primary);
   int ret= 1;
   int error;
+  TABLE_LIST cluster_table_l;
   TABLE* cluster_table= 0;
-  TABLE* members_table= 0;
+  TABLE_LIST members_table_l;
+  TABLE* members_table = 0;
 #ifdef WSREP_SCHEMA_MEMBERS_HISTORY
   TABLE* members_history_table= 0;
 #endif /* WSREP_SCHEMA_MEMBERS_HISTORY */
@@ -732,11 +729,13 @@ int Wsrep_schema::store_view(THD* thd, const Wsrep_view& view)
     Store cluster view info
   */
   Wsrep_schema_impl::init_stmt(thd);
-  if (Wsrep_schema_impl::open_for_write(thd, cluster_table_str.c_str(), &cluster_table))
+  if (Wsrep_schema_impl::open_for_write(thd, cluster_table_str.c_str(), &cluster_table_l))
   {
     goto out;
   }
 
+  cluster_table= cluster_table_l.table;
+
   Wsrep_schema_impl::store(cluster_table, 0, view.state_id().id());
   Wsrep_schema_impl::store(cluster_table, 1, view.view_seqno().get());
   Wsrep_schema_impl::store(cluster_table, 2, view.state_id().seqno().get());
@@ -756,12 +755,14 @@ int Wsrep_schema::store_view(THD* thd, const Wsrep_view& view)
   */
   Wsrep_schema_impl::init_stmt(thd);
   if (Wsrep_schema_impl::open_for_write(thd, members_table_str.c_str(),
-                                        &members_table))
+                                        &members_table_l))
   {
     WSREP_ERROR("failed to open wsrep.members table");
     goto out;
   }
 
+  members_table= members_table_l.table;
+
   for (size_t i= 0; i < view.members().size(); ++i)
   {
     Wsrep_schema_impl::store(members_table, 0, view.members()[i].id());
@@ -815,8 +816,10 @@ Wsrep_view Wsrep_schema::restore_view(THD* thd, const Wsrep_id& own_id) const {
   int ret= 1;
   int error;
 
+  TABLE_LIST cluster_table_l;
   TABLE* cluster_table= 0;
   bool end_cluster_scan= false;
+  TABLE_LIST members_table_l;
   TABLE* members_table= 0;
   bool end_members_scan= false;
 
@@ -842,8 +845,12 @@ Wsrep_view Wsrep_schema::restore_view(THD* thd, const Wsrep_id& own_id) const {
     Read cluster info from cluster table
    */
   Wsrep_schema_impl::init_stmt(thd);
-  if (Wsrep_schema_impl::open_for_read(thd, cluster_table_str.c_str(), &cluster_table) ||
-      Wsrep_schema_impl::init_for_scan(cluster_table)) {
+  if (Wsrep_schema_impl::open_for_read(thd, cluster_table_str.c_str(), &cluster_table_l)) {
+    goto out;
+  }
+  cluster_table = cluster_table_l.table;
+
+  if (Wsrep_schema_impl::init_for_scan(cluster_table)) {
     goto out;
   }
 
@@ -867,8 +874,14 @@ Wsrep_view Wsrep_schema::restore_view(THD* thd, const Wsrep_id& own_id) const {
     Read members from members table
   */
   Wsrep_schema_impl::init_stmt(thd);
-  if (Wsrep_schema_impl::open_for_read(thd, members_table_str.c_str(), &members_table) ||
-      Wsrep_schema_impl::init_for_scan(members_table)) {
+  if (Wsrep_schema_impl::open_for_read(thd, members_table_str.c_str(),
+                                       &members_table_l))
+  {
+    goto out;
+  }
+
+  members_table= members_table_l.table;
+  if (Wsrep_schema_impl::init_for_scan(members_table)) {
     goto out;
   }
   end_members_scan= true;
@@ -972,14 +985,15 @@ int Wsrep_schema::append_fragment(THD* thd,
   Wsrep_schema_impl::sql_safe_updates sql_safe_updates(thd);
   Wsrep_schema_impl::init_stmt(thd);
 
-  TABLE* frag_table= 0;
-  if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table))
+  TABLE_LIST frag_table_l;
+  if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table_l))
   {
     trans_rollback_stmt(thd);
     thd->lex->restore_backup_query_tables_list(&query_tables_list_backup);
     DBUG_RETURN(1);
   }
 
+  TABLE* frag_table= frag_table_l.table;
   Wsrep_schema_impl::store(frag_table, 0, server_id);
   Wsrep_schema_impl::store(frag_table, 1, transaction_id.get());
   Wsrep_schema_impl::store(frag_table, 2, seqno.get());
@@ -1023,13 +1037,15 @@ int Wsrep_schema::update_fragment_meta(THD* thd,
   uchar *key=NULL;
   key_part_map key_map= 0;
   TABLE* frag_table= 0;
+  TABLE_LIST frag_table_l;
 
   Wsrep_schema_impl::init_stmt(thd);
-  if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table))
+  if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table_l))
   {
     thd->lex->restore_backup_query_tables_list(&query_tables_list_backup);
     DBUG_RETURN(1);
   }
+  frag_table= frag_table_l.table;
 
   /* Find record with the given uuid, trx id, and seqno -1 */
   Wsrep_schema_impl::store(frag_table, 0, ws_meta.server_id());
@@ -1152,12 +1168,14 @@ int Wsrep_schema::remove_fragments(THD* thd,
   thd->reset_n_backup_open_tables_state(&open_tables_backup);
 
   TABLE* frag_table= 0;
-  if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table))
+  TABLE_LIST frag_table_l;
+  if (Wsrep_schema_impl::open_for_write(thd, sr_table_str.c_str(), &frag_table_l))
   {
     ret= 1;
   }
   else
   {
+    frag_table= frag_table_l.table;
     for (std::vector<wsrep::seqno>::const_iterator i= fragments.begin();
          i != fragments.end(); ++i)
     {
@@ -1218,6 +1236,7 @@ int Wsrep_schema::replay_transaction(THD* orig_thd,
   int ret= 1;
   int error;
   TABLE* frag_table= 0;
+  TABLE_LIST frag_table_l;
   uchar *key=NULL;
   key_part_map key_map= 0;
 
@@ -1225,12 +1244,13 @@ int Wsrep_schema::replay_transaction(THD* orig_thd,
        i != fragments.end(); ++i)
   {
     Wsrep_schema_impl::init_stmt(&thd);
-    if ((error= Wsrep_schema_impl::open_for_read(&thd, sr_table_str.c_str(), &frag_table)))
+    if ((error= Wsrep_schema_impl::open_for_read(&thd, sr_table_str.c_str(), &frag_table_l)))
     {
       WSREP_WARN("Could not open SR table for read: %d", error);
       Wsrep_schema_impl::finish_stmt(&thd);
       DBUG_RETURN(1);
     }
+    frag_table= frag_table_l.table;
 
     Wsrep_schema_impl::store(frag_table, 0, ws_meta.server_id());
     Wsrep_schema_impl::store(frag_table, 1, ws_meta.transaction_id().get());
@@ -1276,12 +1296,13 @@ int Wsrep_schema::replay_transaction(THD* orig_thd,
 
     if ((error= Wsrep_schema_impl::open_for_write(&thd,
                                                   sr_table_str.c_str(),
-                                                  &frag_table)))
+                                                  &frag_table_l)))
     {
       WSREP_WARN("Could not open SR table for write: %d", error);
       Wsrep_schema_impl::finish_stmt(&thd);
       DBUG_RETURN(1);
     }
+    frag_table= frag_table_l.table;
 
     error= Wsrep_schema_impl::init_for_index_scan(frag_table,
                                                   key,
@@ -1323,7 +1344,9 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd)
                              (char*) &storage_thd);
   wsrep_assign_from_threadvars(&storage_thd);
   TABLE* frag_table= 0;
+  TABLE_LIST frag_table_l;
   TABLE* cluster_table= 0;
+  TABLE_LIST cluster_table_l;
   Wsrep_storage_service storage_service(&storage_thd);
   Wsrep_schema_impl::binlog_off binlog_off(&storage_thd);
   Wsrep_schema_impl::wsrep_off wsrep_off(&storage_thd);
@@ -1338,10 +1361,15 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd)
 
   Wsrep_schema_impl::init_stmt(&storage_thd);
   storage_thd.wsrep_skip_locking= FALSE;
-  if (Wsrep_schema_impl::open_for_read(&storage_thd,
-                                       cluster_table_str.c_str(),
-                                       &cluster_table) ||
-      Wsrep_schema_impl::init_for_scan(cluster_table))
+  if (Wsrep_schema_impl::open_for_read(&storage_thd, cluster_table_str.c_str(),
+                                       &cluster_table_l))
+  {
+    Wsrep_schema_impl::finish_stmt(&storage_thd);
+    DBUG_RETURN(1);
+  }
+  cluster_table= cluster_table_l.table;
+
+  if (Wsrep_schema_impl::init_for_scan(cluster_table))
   {
     Wsrep_schema_impl::finish_stmt(&storage_thd);
     DBUG_RETURN(1);
@@ -1379,12 +1407,19 @@ int Wsrep_schema::recover_sr_transactions(THD *orig_thd)
     Open the table for reading and writing so that fragments without
     valid seqno can be deleted.
   */
-  if (Wsrep_schema_impl::open_for_write(&storage_thd, sr_table_str.c_str(), &frag_table) ||
-      Wsrep_schema_impl::init_for_scan(frag_table))
+  if (Wsrep_schema_impl::open_for_write(&storage_thd, sr_table_str.c_str(),
+                                        &frag_table_l))
   {
     WSREP_ERROR("Failed to open SR table for write");
     goto out;
   }
+  frag_table= frag_table_l.table;
+
+  if (Wsrep_schema_impl::init_for_scan(frag_table))
+  {
+    WSREP_ERROR("Failed to init for index scan");
+    goto out;
+  }
 
   while (true)
   {