MDEV-29624 MDEV-29655 Fix ASAN errors on pushdown of derived table

Deallocation of TABLE_LIST::dt_handler and TABLE_LIST::pushdown_derived
was performed in multiple places if code. This not only made the code
more difficult to maintain but also led to memory leaks and
ASAN heap-use-after-free errors.
This commit puts deallocation of TABLE_LIST::dt_handler and
TABLE_LIST::pushdown_derived to the single point - JOIN::cleanup()

mysql-test/suite/federated/federatedx_create_handlers.result

@@ -469,6 +469,51 @@ a
 1
 2
 3
+#
+# MDEV-29655: ASAN heap-use-after-free in
+#             Pushdown_derived::Pushdown_derived
+#
+connection slave;
+DROP TABLE IF EXISTS federated.t1;
+CREATE TABLE federated.t1 (
+id int(20) NOT NULL,
+name varchar(16) NOT NULL default ''
+)
+DEFAULT CHARSET=latin1;
+INSERT INTO federated.t1 VALUES
+(3,'xxx'), (7,'yyy'), (4,'xxx'), (1,'zzz'), (5,'yyy');
+connection master;
+DROP TABLE IF EXISTS federated.t1;
+CREATE TABLE federated.t1 (
+id int(20) NOT NULL,
+name varchar(16) NOT NULL default ''
+)
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/federated/t1';
+use federated;
+SELECT * FROM (SELECT * FROM (SELECT * FROM (SELECT * FROM t1 where id=3) dt3
+WHERE id=2) dt2) dt;
+id	name
+connection slave;
+CREATE TABLE federated.t10 (a INT,b INT);
+CREATE TABLE federated.t11 (a INT, b INT);
+INSERT INTO federated.t10 VALUES (1,1),(2,2);
+INSERT INTO federated.t11 VALUES (1,1),(2,2);
+connection master;
+CREATE TABLE federated.t10
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/federated/t10';
+CREATE TABLE federated.t11
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/federated/t11';
+use federated;
+SELECT * FROM t10 LEFT JOIN 
+(t11, (SELECT * FROM (SELECT * FROM (SELECT * FROM t1 where id=3) dt3
+WHERE id=2) dt2) dt
+) ON t10.a=t11.a;
+a	b	a	b	id	name
+1	1	NULL	NULL	NULL	NULL
+2	2	NULL	NULL	NULL	NULL
 set global federated_pushdown=0;
 connection master;
 DROP TABLE IF EXISTS federated.t1;

mysql-test/suite/federated/federatedx_create_handlers.test

@@ -267,7 +267,6 @@ INSERT INTO federated.t2
   SELECT * FROM (SELECT * FROM federated.t1 LIMIT 70000) dt;
 SELECT COUNT(DISTINCT a) FROM federated.t2;
 
-
 --echo #
 --echo # MDEV-29640 FederatedX does not properly handle pushdown
 --echo #             in case of difference in local and remote table names
@@ -314,6 +313,64 @@ CREATE TABLE federated.t3 (a INT)
 EXPLAIN SELECT * FROM federated.t3;
 SELECT * FROM federated.t3;
 
+--echo #
+--echo # MDEV-29655: ASAN heap-use-after-free in
+--echo #             Pushdown_derived::Pushdown_derived
+--echo #
+
+connection slave;
+DROP TABLE IF EXISTS federated.t1;
+
+CREATE TABLE federated.t1 (
+  id int(20) NOT NULL,
+  name varchar(16) NOT NULL default ''
+)
+DEFAULT CHARSET=latin1;
+
+INSERT INTO federated.t1 VALUES
+  (3,'xxx'), (7,'yyy'), (4,'xxx'), (1,'zzz'), (5,'yyy');
+
+connection master;
+DROP TABLE IF EXISTS federated.t1;
+
+--replace_result $SLAVE_MYPORT SLAVE_PORT
+eval
+CREATE TABLE federated.t1 (
+  id int(20) NOT NULL,
+  name varchar(16) NOT NULL default ''
+)
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/federated/t1';
+
+use federated;
+SELECT * FROM (SELECT * FROM (SELECT * FROM (SELECT * FROM t1 where id=3) dt3
+  WHERE id=2) dt2) dt;
+
+connection slave;
+CREATE TABLE federated.t10 (a INT,b INT);
+CREATE TABLE federated.t11 (a INT, b INT);
+INSERT INTO federated.t10 VALUES (1,1),(2,2);
+INSERT INTO federated.t11 VALUES (1,1),(2,2);
+
+connection master;
+--replace_result $SLAVE_MYPORT SLAVE_PORT
+eval
+CREATE TABLE federated.t10
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/federated/t10';
+
+--replace_result $SLAVE_MYPORT SLAVE_PORT
+eval
+CREATE TABLE federated.t11
+ENGINE="FEDERATED" DEFAULT CHARSET=latin1
+CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/federated/t11';
+
+use federated;
+SELECT * FROM t10 LEFT JOIN 
+  (t11, (SELECT * FROM (SELECT * FROM (SELECT * FROM t1 where id=3) dt3
+    WHERE id=2) dt2) dt
+  ) ON t10.a=t11.a;
+
 set global federated_pushdown=0;
 
 source include/federated_cleanup.inc;

sql/derived_handler.cc

@@ -44,11 +44,6 @@ Pushdown_derived::Pushdown_derived(TABLE_LIST *tbl, derived_handler *h)
 }
 
 
-Pushdown_derived::~Pushdown_derived()
-{
-  delete handler;
-}
-
 
 int Pushdown_derived::execute()
 {

sql/sql_derived.cc

@@ -1000,11 +1000,7 @@ bool mysql_derived_optimize(THD *thd, LEX *lex, TABLE_LIST *derived)
     /* Create an object for execution of the query specifying the table */
     if (!(derived->pushdown_derived=
             new (thd->mem_root) Pushdown_derived(derived, derived->dt_handler)))
-    {
-      delete derived->dt_handler;
-      derived->dt_handler= NULL;
       DBUG_RETURN(TRUE);
-    }
   }
 
   lex->current_select= first_select;
@@ -1229,7 +1225,6 @@ bool mysql_derived_fill(THD *thd, LEX *lex, TABLE_LIST *derived)
     /* Execute the query that specifies the derived table by a foreign engine */
     res= derived->pushdown_derived->execute();
     unit->executed= true;
-    delete derived->pushdown_derived;
       DBUG_RETURN(res);
   }
 

sql/sql_select.cc

@@ -68,6 +68,7 @@
 #include "select_handler.h"
 #include "my_json_writer.h"
 #include "opt_trace.h"
+#include "derived_handler.h"
 
 /*
   A key part number that means we're using a fulltext scan.
@@ -14086,6 +14087,7 @@ void JOIN::cleanup(bool full)
         }
       }
     }
+    free_pushdown_handlers(*join_list);
   }
   /* Restore ref array to original state */
   if (current_ref_ptrs != items0)
@@ -14096,6 +14098,32 @@ void JOIN::cleanup(bool full)
   DBUG_VOID_RETURN;
 }
 
+/**
+  Clean up all derived pushdown handlers in this join.
+
+  @detail
+    Note that dt_handler is picked at the prepare stage (as opposed
+    to optimization stage where one could expect this).
+    Because of that, we have to do cleanups in this function that is called
+    from JOIN::cleanup() and not in JOIN_TAB::cleanup.
+ */
+void JOIN::free_pushdown_handlers(List<TABLE_LIST>& join_list)
+{
+  List_iterator<TABLE_LIST> li(join_list);
+  TABLE_LIST *table_ref;
+  while ((table_ref= li++))
+  {
+    if (table_ref->nested_join)
+      free_pushdown_handlers(table_ref->nested_join->join_list);
+    if (table_ref->pushdown_derived)
+    {
+      delete table_ref->pushdown_derived;
+      table_ref->pushdown_derived= NULL;
+    }
+    delete table_ref->dt_handler;
+    table_ref->dt_handler= NULL;
+  }
+}
 
 /**
   Remove the following expressions from ORDER BY and GROUP BY:
@@ -27400,12 +27428,6 @@ bool mysql_explain_union(THD *thd, SELECT_LEX_UNIT *unit, select_result *result)
                       result, unit, first);
   }
 
-  if (unit->derived && unit->derived->pushdown_derived)
-  {
-    delete unit->derived->pushdown_derived;
-    unit->derived->pushdown_derived= NULL;
-  }
-
   DBUG_RETURN(res || thd->is_error());
 }
 

sql/sql_select.h

@@ -1840,6 +1840,7 @@ private:
   bool add_having_as_table_cond(JOIN_TAB *tab);
   bool make_aggr_tables_info();
   bool add_fields_for_current_rowid(JOIN_TAB *cur, List<Item> *fields);
+  void free_pushdown_handlers(List<TABLE_LIST>& join_list);
 };
 
 enum enum_with_bush_roots { WITH_BUSH_ROOTS, WITHOUT_BUSH_ROOTS};
@@ -2507,8 +2508,6 @@ public:
 
   Pushdown_derived(TABLE_LIST *tbl, derived_handler *h);
 
-  ~Pushdown_derived();
-
   int execute(); 
 };