MDEV-26979 heap-use-after-free or SIGSEGV when accessing INNODB_SYS_TABLESTATS during DDL

i_s_dict_fill_sys_tablestats(): Read all fields of dict_table_t
while holding dict_sys.latch.

dict_sys_t::allow_eviction(): Remove.

2 files changed, 46 insertions, 77 deletions

diff --git a/storage/innobase/handler/i_s.cc b/storage/innobase/handler/i_s.cc
index bdf3a587e56..d4115f7be16 100644
--- a/storage/innobase/handler/i_s.cc
+++ b/storage/innobase/handler/i_s.cc
@@ -5010,71 +5010,61 @@ static ST_FIELD_INFO innodb_sys_tablestats_fields_info[]=
 };
 } // namespace Show
 
-/** Populate information_schema.innodb_sys_tablestats table with information
-from SYS_TABLES.
-@param[in]	thd		thread ID
-@param[in,out]	table		table
-@param[in]	ref_count	table reference count
-@param[in,out]	table_to_fill	fill this table
+/** Populate information_schema.innodb_sys_tablestats table with a table,
+and release exclusive dict_sys.latch.
+@param[in]	thd		connection
+@param[in,out]	table		InnoDB table metadata
+@param[in,out]	table_to_fill	INFORMATION_SCHEMA.INNODB_SYS_TABLESTATS
 @return 0 on success */
 static
 int
-i_s_dict_fill_sys_tablestats(
-	THD*		thd,
-	dict_table_t*	table,
-	ulint		ref_count,
-	TABLE*		table_to_fill)
+i_s_dict_fill_sys_tablestats(THD* thd, dict_table_t *table,
+                             TABLE* table_to_fill)
 {
-	Field**		fields;
-
-	DBUG_ENTER("i_s_dict_fill_sys_tablestats");
-
-	fields = table_to_fill->field;
-
-	OK(fields[SYS_TABLESTATS_ID]->store(longlong(table->id), TRUE));
+  DBUG_ENTER("i_s_dict_fill_sys_tablestats");
 
-	OK(field_store_string(fields[SYS_TABLESTATS_NAME],
-			      table->name.m_name));
+  Field **fields= table_to_fill->field;
 
-	{
-		table->stats_mutex_lock();
-		auto _ = make_scope_exit([table]() {
-			table->stats_mutex_unlock(); });
-
-		OK(fields[SYS_TABLESTATS_INIT]->store(table->stat_initialized,
-						      true));
-
-		if (table->stat_initialized) {
-			OK(fields[SYS_TABLESTATS_NROW]->store(
-				   table->stat_n_rows, true));
+  {
+    table->stats_mutex_lock();
+    auto _ = make_scope_exit([table]() {
+      table->stats_mutex_unlock(); dict_sys.unlock(); });
 
-			OK(fields[SYS_TABLESTATS_CLUST_SIZE]->store(
-				   table->stat_clustered_index_size, true));
+    OK(fields[SYS_TABLESTATS_ID]->store(longlong(table->id), TRUE));
 
-			OK(fields[SYS_TABLESTATS_INDEX_SIZE]->store(
-				   table->stat_sum_of_other_index_sizes,
-				   true));
+    OK(field_store_string(fields[SYS_TABLESTATS_NAME],
+                          table->name.m_name));
+    OK(fields[SYS_TABLESTATS_INIT]->store(table->stat_initialized, true));
 
-			OK(fields[SYS_TABLESTATS_MODIFIED]->store(
-				   table->stat_modified_counter, true));
-		} else {
-			OK(fields[SYS_TABLESTATS_NROW]->store(0, true));
+    if (table->stat_initialized)
+    {
+      OK(fields[SYS_TABLESTATS_NROW]->store(table->stat_n_rows, true));
 
-			OK(fields[SYS_TABLESTATS_CLUST_SIZE]->store(0, true));
+      OK(fields[SYS_TABLESTATS_CLUST_SIZE]->
+         store(table->stat_clustered_index_size, true));
 
-			OK(fields[SYS_TABLESTATS_INDEX_SIZE]->store(0, true));
+      OK(fields[SYS_TABLESTATS_INDEX_SIZE]->
+         store(table->stat_sum_of_other_index_sizes, true));
 
-			OK(fields[SYS_TABLESTATS_MODIFIED]->store(0, true));
-		}
-	}
-
-	OK(fields[SYS_TABLESTATS_AUTONINC]->store(table->autoinc, true));
+      OK(fields[SYS_TABLESTATS_MODIFIED]->
+         store(table->stat_modified_counter, true));
+    }
+    else
+    {
+      OK(fields[SYS_TABLESTATS_NROW]->store(0, true));
+      OK(fields[SYS_TABLESTATS_CLUST_SIZE]->store(0, true));
+      OK(fields[SYS_TABLESTATS_INDEX_SIZE]->store(0, true));
+      OK(fields[SYS_TABLESTATS_MODIFIED]->store(0, true));
+    }
 
-	OK(fields[SYS_TABLESTATS_TABLE_REF_COUNT]->store(ref_count, true));
+    OK(fields[SYS_TABLESTATS_AUTONINC]->store(table->autoinc, true));
 
-	OK(schema_table_store_record(thd, table_to_fill));
+    OK(fields[SYS_TABLESTATS_TABLE_REF_COUNT]->
+       store(table->get_ref_count(), true));
+  }
 
-	DBUG_RETURN(0);
+  OK(schema_table_store_record(thd, table_to_fill));
+  DBUG_RETURN(0);
 }
 
 /*******************************************************************//**
@@ -5109,23 +5099,17 @@ i_s_sys_tables_fill_table_stats(
 
 	while (rec) {
 		const char*	err_msg;
-		dict_table_t*	table_rec= 0;
+		dict_table_t*	table_rec = nullptr;
 
 		mtr.commit();
 		/* Fetch the dict_table_t structure corresponding to
 		this SYS_TABLES record */
 		err_msg = i_s_sys_tables_rec(pcur, nullptr, nullptr,
-                                             &table_rec);
+					     &table_rec);
 
 		if (UNIV_LIKELY(!err_msg)) {
-			bool evictable = dict_sys.prevent_eviction(table_rec);
-			ulint ref_count = table_rec->get_ref_count();
-			dict_sys.unlock();
-			i_s_dict_fill_sys_tablestats(thd, table_rec, ref_count,
+			i_s_dict_fill_sys_tablestats(thd, table_rec,
 						     tables->table);
-			if (!evictable) {
-				table_rec = nullptr;
-			}
 		} else {
 			ut_ad(!table_rec);
 			dict_sys.unlock();
@@ -5137,9 +5121,6 @@ i_s_sys_tables_fill_table_stats(
 		/* Get the next record */
 		mtr.start();
 		dict_sys.lock(SRW_LOCK_CALL);
-		if (table_rec) {
-			dict_sys.allow_eviction(table_rec);
-		}
 
 		rec = dict_getnext_system(&pcur, &mtr);
 	}
diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h
index e051cb38c1d..29673f5bc95 100644
--- a/storage/innobase/include/dict0dict.h
+++ b/storage/innobase/include/dict0dict.h
@@ -1487,28 +1487,16 @@ public:
   }
 #endif
 
-  /** Move a table to the non-LRU list from the LRU list.
-  @return whether the table was evictable */
-  bool prevent_eviction(dict_table_t *table)
+  /** Move a table to the non-LRU list from the LRU list. */
+  void prevent_eviction(dict_table_t *table)
   {
     ut_d(locked());
     ut_ad(find(table));
     if (!table->can_be_evicted)
-      return false;
+      return;
     table->can_be_evicted= false;
     UT_LIST_REMOVE(table_LRU, table);
     UT_LIST_ADD_LAST(table_non_LRU, table);
-    return true;
-  }
-  /** Move a table from the non-LRU list to the LRU list. */
-  void allow_eviction(dict_table_t *table)
-  {
-    ut_d(locked());
-    ut_ad(find(table));
-    ut_ad(!table->can_be_evicted);
-    table->can_be_evicted= true;
-    UT_LIST_REMOVE(table_non_LRU, table);
-    UT_LIST_ADD_FIRST(table_LRU, table);
   }
 
 #ifdef UNIV_DEBUG