MDEV-31432 tmp_table field accessed after free

Before this patch, the code in Item_field::print() used
this convention (described in sql_explain.h:ExplainDataStructureLifetime):

- By default, the table that Item_field refers to is accessible.
- ANALYZE and SHOW {EXPLAIN|ANALYZE} may print Items after some
  temporary tables have been dropped. They use
  QT_DONT_ACCESS_TMP_TABLES flag. When it is ON, Item_field::print
  will not access the table it refers to, if it is a temp.table

The bug was that EXPLAIN statement also may compute subqueries (depending
on subquery context and @@expensive_subquery_limit setting). After the
computation, the subquery calls JOIN::cleanup(true) which drops some of
its temporary tables. Calling Item_field::print() that refer to such table
will cause an access to free'd memory.

In this patch, we take into account that query optimization can compute
a subquery and discard its temporary tables. Item_field::print() now
assumes that any temporary table might have already been dropped.
This means QT_DONT_ACCESS_TMP_TABLES flag is not needed - we imply it is
always present.

But we also make one exception: derived tables are not freed in
JOIN::cleanup() call. They are freed later in close_thread_tables(),
at the same time when regular tables are closed.
Because of that, Item_field::print may assume that temp.tables
representing derived tables are available.

Initial patch by: Rex Jonston
Reviewed by: Monty <monty@mariadb.org>

Author: spetrunia

mysql-test/main/show_analyze.result

@@ -434,3 +434,42 @@ ANALYZE
   }
 }
 DROP TABLE t1;
+#
+# MDEV-31432 tmp_table field accessed after free
+# testing for the above (MDEV-28201) caused use after free error
+#
+create table t1 (x int) engine=myisam;
+insert into t1 values(1);
+set @tmp=@@optimizer_trace;
+set @@optimizer_trace=1;
+SELECT
+1 IN
+((
+SELECT
+1 IN (SELECT 1 AS x0
+FROM
+(
+SELECT *
+FROM (SELECT 1 AS x) AS x5
+GROUP BY x,x
+HAVING
+x IN (
+SELECT *
+FROM t1 AS x1
+WHERE
+x IN (SELECT 1 AS x
+FROM t1 AS x3
+GROUP BY x
+HAVING
+x IN (SELECT 0 FROM t1 AS x4)
+)
+)
+) AS x6
+)
+FROM
+t1
+)) as VAL;
+VAL
+0
+set optimizer_trace=@tmp;
+drop table t1;

mysql-test/main/show_analyze.test

@@ -364,3 +364,44 @@ ANALYZE format=json
 SELECT 1 FROM t1 GROUP BY convert_tz('1969-12-31 22:00:00',a,'+10:00');
 DROP TABLE t1;
 
+--echo #
+--echo # MDEV-31432 tmp_table field accessed after free
+--echo # testing for the above (MDEV-28201) caused use after free error
+--echo #
+create table t1 (x int) engine=myisam;
+insert into t1 values(1);
+set @tmp=@@optimizer_trace;
+set @@optimizer_trace=1;
+# Different warning text is produced in regular and --ps-protocol runs:
+--disable_warnings
+SELECT
+  1 IN
+  ((
+     SELECT
+       1 IN (SELECT 1 AS x0
+             FROM
+               (
+                SELECT *
+                FROM (SELECT 1 AS x) AS x5
+                GROUP BY x,x
+                HAVING
+                  x IN (
+                    SELECT *
+                    FROM t1 AS x1
+                    WHERE
+                      x IN (SELECT 1 AS x
+                            FROM t1 AS x3
+                            GROUP BY x
+                            HAVING
+                              x IN (SELECT 0 FROM t1 AS x4)
+                           )
+                  )
+               ) AS x6
+            )
+     FROM
+       t1
+  )) as VAL;
+--enable_warnings
+set optimizer_trace=@tmp;
+drop table t1;
+

sql/item.cc

@@ -3138,7 +3138,7 @@ void Item_field::set_field(Field *field_par)
 
   if (field->table->s->tmp_table == SYSTEM_TMP_TABLE ||
       field->table->s->tmp_table == INTERNAL_TMP_TABLE)
-    set_refers_to_temp_table(true);
+    set_refers_to_temp_table();
 }
 
 
@@ -3615,7 +3615,7 @@ Item *Item_field::get_tmp_table_item(THD *thd)
   if (new_item)
   {
     new_item->field= new_item->result_field;
-    new_item->set_refers_to_temp_table(true);
+    new_item->set_refers_to_temp_table();
   }
   return new_item;
 }
@@ -3626,9 +3626,14 @@ longlong Item_field::val_int_endpoint(bool left_endp, bool *incl_endp)
   return null_value? LONGLONG_MIN : res;
 }
 
-void Item_field::set_refers_to_temp_table(bool value)
+void Item_field::set_refers_to_temp_table()
 {
-  refers_to_temp_table= value;
+  /*
+    Derived temp. tables have non-zero derived_select_number.
+    We don't need to distingish between other kinds of temp.tables currently.
+  */
+  refers_to_temp_table= (field->table->derived_select_number != 0)?
+                        REFERS_TO_DERIVED_TMP : REFERS_TO_OTHER_TMP;
 }
 
 
@@ -6292,7 +6297,7 @@ void Item_field::cleanup()
   field= 0;
   item_equal= NULL;
   null_value= FALSE;
-  refers_to_temp_table= FALSE;
+  refers_to_temp_table= NO_TEMP_TABLE;
   DBUG_VOID_RETURN;
 }
 
@@ -7860,14 +7865,15 @@ void Item_field::print(String *str, enum_query_type query_type)
 {
   /*
     If the field refers to a constant table, print the value.
-    (1): But don't attempt to do that if
-          * the field refers to a temporary (work) table, and
-          * temp. tables might already have been dropped.
+    There are two exceptions:
+    1. For temporary (aka "work") tables, we can only access the derived temp.
+       tables. Other kinds of tables might already have been dropped.
+    2. Don't print constants if QT_NO_DATA_EXPANSION or QT_VIEW_INTERNAL is
+       specified.
   */
-  if (!(refers_to_temp_table &&                      // (1)
-        (query_type & QT_DONT_ACCESS_TMP_TABLES)) && // (1)
-      field && field->table->const_table &&
-      !(query_type & (QT_NO_DATA_EXPANSION | QT_VIEW_INTERNAL)))
+  if ((refers_to_temp_table != REFERS_TO_OTHER_TMP) &&             // (1)
+      !(query_type & (QT_NO_DATA_EXPANSION | QT_VIEW_INTERNAL)) && // (2)
+      field && field->table->const_table)
   {
     print_value(str);
     return;
@@ -9145,7 +9151,7 @@ Item* Item_cache_wrapper::get_tmp_table_item(THD *thd)
   {
     auto item_field= new (thd->mem_root) Item_field(thd, result_field);
     if (item_field)
-      item_field->set_refers_to_temp_table(true);
+      item_field->set_refers_to_temp_table();
     return item_field;
   }
   return copy_or_same(thd);

sql/item.h

@@ -3568,27 +3568,18 @@ class Item_field :public Item_ident,
 
 private:
   /*
-    Setting this member to TRUE (via set_refers_to_temp_table())
-    ensures print() function continues to work even if the table
-    has been dropped.
+    Indicates whether this Item_field refers to a regular or some kind of
+    temporary table.
+    This is needed for print() to work: it may be called even after the table
+    referred by the Item_field has been dropped.
 
-    We need this for "ANALYZE statement" feature. Query execution has
-    these steps:
-      1. Run the query.
-      2. Cleanup starts. Temporary tables are destroyed
-      3. print "ANALYZE statement" output, if needed
-      4. Call close_thread_table() for regular tables.
-
-    Step #4 is done after step #3, so "ANALYZE stmt" has no problem printing
-    Item_field objects that refer to regular tables.
-
-    However, Step #3 is done after Step #2. Attempt to print Item_field objects
-    that refer to temporary tables will cause access to freed memory.
-
-    To resolve this, we use refers_to_temp_table member to refer to items 
-    in temporary (work) tables.
+    See ExplainDataStructureLifetime in sql_explain.h for details.
   */
-  bool refers_to_temp_table= false;
+  enum {
+    NO_TEMP_TABLE= 0,
+    REFERS_TO_DERIVED_TMP= 1,
+    REFERS_TO_OTHER_TMP=2
+  } refers_to_temp_table = NO_TEMP_TABLE;
 
 public:
   Item_field(THD *thd, Name_resolution_context *context_arg,
@@ -3804,7 +3795,7 @@ class Item_field :public Item_ident,
     return field->table->pos_in_table_list->outer_join;
   }
   bool check_index_dependence(void *arg) override;
-  void set_refers_to_temp_table(bool value);
+  void set_refers_to_temp_table();
   friend class Item_default_value;
   friend class Item_insert_value;
   friend class st_select_lex_unit;

sql/item_func.cc

@@ -749,7 +749,7 @@ Item *Item_func::get_tmp_table_item(THD *thd)
   {
     auto item_field= new (thd->mem_root) Item_field(thd, result_field);
     if (item_field)
-      item_field->set_refers_to_temp_table(true);
+      item_field->set_refers_to_temp_table();
     return item_field;
   }
   return copy_or_same(thd);

sql/item_subselect.cc

@@ -1030,7 +1030,7 @@ Item *Item_subselect::get_tmp_table_item(THD *thd_arg)
     auto item_field=
         new (thd->mem_root) Item_field(thd_arg, result_field);
     if (item_field)
-      item_field->set_refers_to_temp_table(true);
+      item_field->set_refers_to_temp_table();
     return item_field;
   }
   return copy_or_same(thd_arg);
@@ -5310,7 +5310,7 @@ bool subselect_hash_sj_engine::make_semi_join_conds()
     Item_field *right_col_item= new (thd->mem_root)
         Item_field(thd, context, tmp_table->field[i]);
     if (right_col_item)
-      right_col_item->set_refers_to_temp_table(true);
+      right_col_item->set_refers_to_temp_table();
 
     if (!right_col_item ||
         !(eq_cond= new (thd->mem_root)

sql/item_sum.cc

@@ -562,7 +562,7 @@ Item *Item_sum::get_tmp_table_item(THD *thd)
           auto item_field=
             new (thd->mem_root) Item_field(thd, result_field_tmp++);
           if (item_field)
-            item_field->set_refers_to_temp_table(true);
+            item_field->set_refers_to_temp_table();
           sum_item->args[i]= item_field;
         }
       }

sql/mysqld.h

@@ -905,11 +905,7 @@ enum enum_query_type
   // don't reveal values.
   QT_NO_DATA_EXPANSION= (1 << 9),
   // Remove wrappers added for TVC when creating or showing view
-  QT_NO_WRAPPERS_FOR_TVC_IN_VIEW= (1 << 12),
-
-  // The temporary tables used by the query might be freed by the time
-  // this print() call is made.
-  QT_DONT_ACCESS_TMP_TABLES= (1 << 13)
+  QT_NO_WRAPPERS_FOR_TVC_IN_VIEW= (1 << 12)
 };
 
 

sql/sql_base.cc

@@ -871,6 +871,9 @@ int close_thread_tables(THD *thd)
     TODO: Probably even better approach is to simply associate list of
           derived tables with (sub-)statement instead of thread and destroy
           them at the end of its execution.
+
+    Note: EXPLAIN/ANALYZE depends on derived tables being freed here. See
+    sql_explain.h:ExplainDataStructureLifetime.
   */
   if (thd->derived_tables)
   {