Bug #96178 mysqldump leaks memory when selected tables are dumped with --order-by-primary

Description:
The order_by global variable in mysqldump.cc needs to be freed after every invocation of dump_tables(). This is not done in dump_selected_tables(). Also, to be safe we should always free order_by after calling get_table_structure().

How to repeat:
Run this test case with ASAN build.

--disable_warnings
DROP TABLE IF EXISTS table1;
drop database if exists database1;
--enable_warnings

CREATE DATABASE database1;
USE database1;
CREATE TABLE table1(a INT PRIMARY KEY, b INT UNIQUE) engine = InnoDB;
CREATE TABLE table2(a INT PRIMARY KEY, b INT UNIQUE) engine = InnoDB;

INSERT INTO table1 VALUES (1, 3);
INSERT INTO table1 VALUES (2, 2);
INSERT INTO table1 VALUES (3, 1);

INSERT INTO table2 VALUES (1, 3);
INSERT INTO table2 VALUES (2, 2);
INSERT INTO table2 VALUES (3, 1);

--echo ==== mysqldump with --order-by-primary ====
--replace_regex /-- Server version.*/-- SERVER VERSION/ /-- MySQL dump.*[)]/-- MYSQLDUMP VERSION/
--exec $MYSQL_DUMP --skip-dump-date --order-by-primary --extended-insert=FALSE database1 table1 table2

DROP TABLE table1;
DROP TABLE table2;
drop database database1;

Suggested fix:
diff --git a/client/mysqldump.cc b/client/mysqldump.cc
index 0b047e53e1a..a8c3f8ff8f8 100644
--- a/client/mysqldump.cc
+++ b/client/mysqldump.cc
@@ -4836,6 +4836,8 @@ static int dump_all_tables_in_db(char *database) {
         verbose_msg(
             "-- Warning: get_table_structure() failed with some internal "
             "error for 'general_log' table\n");
+      my_free(order_by);
+      order_by = 0;
     }
     if (slow_log_table_exists) {
       if (!get_table_structure((char *)"slow_log", database, table_type,
@@ -4843,6 +4845,8 @@ static int dump_all_tables_in_db(char *database) {
         verbose_msg(
             "-- Warning: get_table_structure() failed with some internal "
             "error for 'slow_log' table\n");
+      my_free(order_by);
+      order_by = 0;
     }
   }
   if (flush_privileges && using_mysql_db) {
@@ -5039,6 +5043,8 @@ static int dump_selected_tables(char *db, char **table_names, int tables) {
   for (pos = dump_tables; pos < end; pos++) {
     DBUG_PRINT("info", ("Dumping table %s", *pos));
     dump_table(*pos, db);
+    my_free(order_by);
+    order_by = 0;
     if (opt_dump_triggers && mysql_get_server_version(mysql) >= 50009) {
       if (dump_triggers_for_table(*pos, db)) {
         if (path) my_fclose(md_result_file, MYF(MY_WME));
@@ -5091,8 +5097,6 @@ static int dump_selected_tables(char *db, char **table_names, int tables) {
     dump_routines_for_db(db);
   }
   free_root(&root, MYF(0));
-  my_free(order_by);
-  order_by = 0;
   if (opt_xml) {
     fputs("</database>\n", md_result_file);
     check_io(md_result_file);

Hello Abhinav Sharma,

Thank you for the report and test case.
Verified as described on 5.7.26 ASAN build.

Thanks,
Umesh

Test results - 5.7.26

Attachment: 96178_5.7.26.results (application/octet-stream, text), 2.96 KiB.

Posted by developer:
 
Changelog entry added for MySQL 8.0.29 and 5.7.38:

A memory leak occurred if mysqldump was used on more than one table with the --order-by-primary option. The memory allocated for sorting each table’s rows is now freed after every table, rather than only once.