MySQL Bugs: #54476: crash when group_concat and 'with rollup' in prepared statements

Bug #54476	crash when group_concat and 'with rollup' in prepared statements
Submitted:	14 Jun 2010 6:41	Modified:	6 Jan 2011 2:53
Reporter:	Shane Bester (Platinum Quality Contributor)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Optimizer	Severity:	S1 (Critical)
Version:	5.0.91, 5.1.47, 5.1.49-bzr, 5.5.3	OS:	Any
Assigned to:	Gleb Shchepa	CPU Architecture:	Any

Description:
5.1.47 stack trace:

mysqld.exe!find_order_in_list()[sql_select.cc:14523]
mysqld.exe!setup_order()[sql_select.cc:14670]
mysqld.exe!Item_func_group_concat::setup()[item_sum.cc:3287]
mysqld.exe!setup_sum_funcs()[sql_select.cc:15636]
mysqld.exe!JOIN::exec()[sql_select.cc:2170]
mysqld.exe!mysql_select()[sql_select.cc:2511]
mysqld.exe!handle_select()[sql_select.cc:269]
mysqld.exe!execute_sqlcom_select()[sql_parse.cc:5067]
mysqld.exe!mysql_execute_command()[sql_parse.cc:2263]
mysqld.exe!Prepared_statement::execute()[sql_prepare.cc:3583]
mysqld.exe!Prepared_statement::execute_loop()[sql_prepare.cc:3258]
mysqld.exe!mysql_sql_stmt_execute()[sql_prepare.cc:2531]
mysqld.exe!mysql_execute_command()[sql_parse.cc:2272]
mysqld.exe!mysql_parse()[sql_parse.cc:5990]
mysqld.exe!dispatch_command()[sql_parse.cc:1235]
mysqld.exe!do_command()[sql_parse.cc:878]
mysqld.exe!handle_one_connection()[sql_connect.cc:1134]
mysqld.exe!pthread_start()[my_winthread.c:85]
mysqld.exe!_callthreadstart()[thread.c:293]

How to repeat:
drop table if exists `t1`;
create table `t1`(`a` int)engine=myisam;
insert into `t1` values (0),(1);
prepare stmt from "select  	
group_concat(`t1`.`a` order by `t1`.`a`)
from `t1` join `t1` `t2` on (`t1`.`a`)
group by `t2`.`a` with rollup";
execute stmt;  
execute stmt;

Verified just as described:

Attempting backtrace. You can use the following information to find out
where mysqld died. If you see no messages after this, something went
terribly wrong...
stack_bottom = 0xb026af64 thread_stack 0x30000
0   mysqld                              0x00580bde my_print_stacktrace + 44
1   mysqld                              0x00101ae8 handle_segfault + 836
2   libSystem.B.dylib                   0x940472bb _sigtramp + 43
3   ???                                 0xffffffff 0x0 + 4294967295
4   mysqld                              0x00188475 _Z11setup_orderP3THDPP4ItemP10TABLE_LISTR4ListIS1_ES8_P8st_order + 83
5   mysqld                              0x0003bff7 _ZN22Item_func_group_concat5setupEP3THD + 747
6   mysqld                              0x00181b4c _ZN4JOIN13change_resultEP13select_result + 482
7   mysqld                              0x001a6c9f _ZN4JOIN8optimizeEv + 12283
8   mysqld                              0x001a870b _Z12mysql_selectP3THDPPP4ItemP10TABLE_LISTjR4ListIS1_ES2_jP8st_orderSB_S2_SB_yP13select_resultP18st_select_lex_unitP13st_select_lex + 741
9   mysqld                              0x001ae0bf _Z13handle_selectP3THDP6st_lexP13select_resultm + 569
10  mysqld                              0x001135fa _Z15update_precheckP3THDP10TABLE_LIST + 1150
11  mysqld                              0x00115cb6 _Z21mysql_execute_commandP3THD + 2936
12  mysqld                              0x001c0834 _ZN18Prepared_statement7executeEP6Stringb + 1112
13  mysqld                              0x001c4f57 _ZN18Prepared_statement12execute_loopEP6StringbPhS2_ + 327
14  mysqld                              0x001c5299 _Z22mysql_sql_stmt_executeP3THD + 491
15  mysqld                              0x00115cdf _Z21mysql_execute_commandP3THD + 2977
16  mysqld                              0x0011f677 _Z11mysql_parseP3THDPKcjPS2_ + 625
17  mysqld                              0x0012044d _Z16dispatch_command19enum_server_commandP3THDPcj + 3079
18  mysqld                              0x00121878 _Z10do_commandP3THD + 666
19  mysqld                              0x0010c21a handle_one_connection + 372
20  libSystem.B.dylib                   0x9400c095 _pthread_start + 321
21  libSystem.B.dylib                   0x9400bf52 thread_start + 34
Trying to get some variables.
Some pointers may be invalid and cause the dump to abort...
thd->query at 0x10b32e8 = select  
group_concat(`t1`.`a` order by `t1`.`a`)
from `t1` join `t1` `t2` on (`t1`.`a`)
group by `t2`.`a` with rollup
thd->thread_id=1

There is no crash when prepared statements are not used:

valeriy-kravchuks-macbook-pro:5.1 openxs$ bin/mysql -uroot test
Reading table information for completion of table and column names
You can turn off this feature to get a quicker startup with -A

Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 1
Server version: 5.1.49-debug Source distribution

Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved.
This software comes with ABSOLUTELY NO WARRANTY. This is free software,
and you are welcome to modify and redistribute it under the GPL v2 license

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

mysql> select  
    -> group_concat(`t1`.`a` order by `t1`.`a`)
    -> from `t1` join `t1` `t2` on (`t1`.`a`)
    -> group by `t2`.`a` with rollup;
+------------------------------------------+
| group_concat(`t1`.`a` order by `t1`.`a`) |
+------------------------------------------+
| 1                                        |
| 1                                        |
| 1,1                                      |
+------------------------------------------+
3 rows in set (0.01 sec)

mysql> select   group_concat(`t1`.`a` order by `t1`.`a`) from `t1` join `t1` `t2` on (`t1`.`a`) group by `t2`.`a` with rollup;
+------------------------------------------+
| group_concat(`t1`.`a` order by `t1`.`a`) |
+------------------------------------------+
| 1                                        |
| 1                                        |
| 1,1                                      |
+------------------------------------------+
3 rows in set (0.00 sec)

mysql> select   group_concat(`t1`.`a` order by `t1`.`a`) from `t1` join `t1` `t2` on (`t1`.`a`) group by `t2`.`a` with rollup;
+------------------------------------------+
| group_concat(`t1`.`a` order by `t1`.`a`) |
+------------------------------------------+
| 1                                        |
| 1                                        |
| 1,1                                      |
+------------------------------------------+
3 rows in set (0.01 sec)

Commit comments and patch:

      Bug #54476: crash when group_concat and 'with rollup' in
                           prepared statements
      
      Using GROUP_CONCAT() together with the WITH ROLLUP modifier
      could crash the server.
      
      The reason was a combination of several facts:
      
      1. The Item_func_group_concat class stores pointers to ORDER
      objects representing the columns in the ORDER BY clause of
      GROUP_CONCAT().
      
      2. find_order_in_list() called from
      Item_func_group_concat::setup() modifies the ORDER objects so
      that their 'item' member points to the arguments list
      allocated in the Item_func_group_concat constructor.
      
      3. In some cases (e.g. in JOIN::rollup_make_fields) a copy of
      the original Item_func_group_concat object could be created by
      using the Item_func_group_concat::Item_func_group_concat(THD
      *thd, Item_func_group_concat *item) copy constructor. The
      latter essentially creates a shallow copy of the source
      object. Memory for the arguments array is allocated on
      thd->mem_root, but the pointers for arguments and ORDER are
      copied verbatim.
      
      What happens in the test case is that when executing the query
      for the first time, after a copy of the original
      Item_func_group_concat object has been created by
      JOIN::rollup_make_fields(), find_order_in_list() is called for
      this new object. It then resolves ORDER BY by modifying the
      ORDER objects so that they point to elements of the arguments
      array which is local to the cloned object. When thd->mem_root
      is freed upon completing the execution, pointers in the ORDER
      objects become invalid. Those ORDER objects, however, are also
      shared with the original Item_func_group_concat object which is
      preserved between executions of a prepared statement. So the
      first call to find_order_in_list() for the original object on
      the second execution tries to dereference an invalid pointer.
      
      The solution is to create copies of the ORDER objects when
      copying Item_func_group_concat to not leave any stale pointers
      in other instances with different lifecycles.
     @ mysql-test/r/func_gconcat.result
        Test case for bug #54476.
     @ mysql-test/t/func_gconcat.test
        Test case for bug #54476.
     @ sql/item_sum.cc
        Copy the ORDER objects pointed to by the elements of the 
        'order' array in the copy constructor of 
        Item_func_group_concat.
     @ sql/table.h
        Removed the unused 'item_copy' member of the ORDER class.
=== modified file 'mysql-test/r/func_gconcat.result'
--- mysql-test/r/func_gconcat.result    2010-03-31 13:00:56 +0000
+++ mysql-test/r/func_gconcat.result    2010-07-23 11:52:54 +0000
@@ -995,6 +995,7 @@ SELECT 1 FROM
 1
 1
 DROP TABLE t1;
+End of 5.0 tests
 #
 # Bug #52397: another crash with explain extended and group_concat
 #
@@ -1010,4 +1011,22 @@ id       select_type     table   type    possible_keys
 Warnings:
 Note   1003    select 1 AS `1` from (select group_concat(`test`.`t1`.`a` order by `test`.`t1`.`a` ASC separator ',') AS `GROUP_CONCAT(t1.a ORDER BY t1.a ASC)` from `test`.`t1` `t2` join `test`.`t1` group by `test`.`t1`.`a`) `d`
 DROP TABLE t1;
-End of 5.0 tests
+#
+# Bug #54476: crash when group_concat and 'with rollup' in prepared statements
+# 
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (1), (2);
+PREPARE stmt FROM "SELECT GROUP_CONCAT(t1.a ORDER BY t1.a) FROM t1 JOIN t1 t2 GROUP BY t1.a WITH ROLLUP";
+EXECUTE stmt;
+GROUP_CONCAT(t1.a ORDER BY t1.a)
+1,1
+2,2
+1,1,2,2
+EXECUTE stmt;
+GROUP_CONCAT(t1.a ORDER BY t1.a)
+1,1
+2,2
+1,1,2,2
+DEALLOCATE PREPARE stmt;
+DROP TABLE t1;
+End of 5.1 tests

=== modified file 'mysql-test/t/func_gconcat.test'
--- mysql-test/t/func_gconcat.test      2010-03-31 13:00:56 +0000
+++ mysql-test/t/func_gconcat.test      2010-07-23 11:52:54 +0000
@@ -708,6 +708,7 @@ SELECT 1 FROM
 
 DROP TABLE t1;
 
+--echo End of 5.0 tests
 
 --echo #
 --echo # Bug #52397: another crash with explain extended and group_concat
@@ -719,5 +720,18 @@ EXPLAIN EXTENDED SELECT 1 FROM
     t1 t2, t1 GROUP BY t1.a) AS d;
 DROP TABLE t1;
 
+--echo #
+--echo # Bug #54476: crash when group_concat and 'with rollup' in prepared statements
+--echo # 
 
---echo End of 5.0 tests
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (1), (2);
+
+PREPARE stmt FROM "SELECT GROUP_CONCAT(t1.a ORDER BY t1.a) FROM t1 JOIN t1 t2 GROUP BY t1.a WITH ROLLUP";
+EXECUTE stmt;
+EXECUTE stmt;
+
+DEALLOCATE PREPARE stmt;
+DROP TABLE t1;
+
+--echo End of 5.1 tests

=== modified file 'sql/item_sum.cc'
--- sql/item_sum.cc     2010-06-10 20:45:22 +0000
+++ sql/item_sum.cc     2010-07-23 11:52:54 +0000
@@ -3034,7 +3034,6 @@ Item_func_group_concat::Item_func_group_
   tree(item->tree),
   unique_filter(item->unique_filter),
   table(item->table),
-  order(item->order),
   context(item->context),
   arg_count_order(item->arg_count_order),
   arg_count_field(item->arg_count_field),
@@ -3047,6 +3046,24 @@ Item_func_group_concat::Item_func_group_
 {
   quick_group= item->quick_group;
   result.set_charset(collation.collation);
+
+  /*
+    Since the ORDER structures pointed to by the elements of the 'order' array
+    may be modified in find_order_in_list() called from
+    Item_func_group_concat::setup(), create a copy of those structures so that
+    such modifications done in this object would not have any effect on the
+    object being copied.
+  */
+  ORDER *tmp;
+  if (!(order= (ORDER **) thd->alloc(sizeof(ORDER *) * arg_count_order +
+                                     sizeof(ORDER) * arg_count_order)))
+    return;
+  tmp= (ORDER *)(order + arg_count_order);
+  for (uint i= 0; i < arg_count_order; i++, tmp++)
+  {
+    memcpy(tmp, item->order[i], sizeof(ORDER));
+    order[i]= tmp;
+  }
 }
 
 

=== modified file 'sql/table.h'
--- sql/table.h 2010-06-10 20:45:22 +0000
+++ sql/table.h 2010-07-23 11:52:54 +0000
@@ -55,7 +55,6 @@ typedef struct st_order {
   struct st_order *next;
   Item  **item;                        /* Point at item in select fields */
   Item  *item_ptr;                     /* Storage for initial item */
-  Item   **item_copy;                  /* For SPs; the original item ptr */
   int    counter;                       /* position in SELECT list, correct
                                            only if counter_used is true*/
   bool  asc;                           /* true if ascending */

Pushed into mysql-5.1 5.1.51 (revid:build@mysql.com-20100819151858-muaaor6jojb5ouzj) (version source revid:build@mysql.com-20100819151858-muaaor6jojb5ouzj) (merge vers: 5.1.51) (pib:20)

Pushed into mysql-5.5 5.5.6-m3 (revid:alik@ibmvm-20100825102234-a3q8x0l7voa13ts3) (version source revid:alik@ibmvm-20100825102234-a3q8x0l7voa13ts3) (merge vers: 5.5.6-m3) (pib:20)

Pushed into mysql-trunk 5.6.1-m4 (revid:alik@sun.com-20100901130501-4g2k86dub29auj8y) (version source revid:alik@sun.com-20100901130012-9bmmvzcnnw6n5rw6) (merge vers: 5.6.1-m4) (pib:21)

Pushed into mysql-next-mr (revid:alik@sun.com-20100901130614-pgop3m80rmutewxn) (version source revid:alik@sun.com-20100901130033-8k19cjn6n2blm3py) (pib:21)

Noted in 5.1.51, 5.5.6, 5.6.1 changelogs.

GROUP_CONCAT() and WITH ROLLUP together could cause a server crash.

Pushed into mysql-5.1-telco-7.0 5.1.51-ndb-7.0.20 (revid:martin.skold@mysql.com-20101014082627-jrmy9xbfbtrebw3c) (version source revid:martin.skold@mysql.com-20101014082627-jrmy9xbfbtrebw3c) (merge vers: 5.1.51-ndb-7.0.20) (pib:21)

Pushed into mysql-5.1-telco-6.3 5.1.51-ndb-6.3.39 (revid:martin.skold@mysql.com-20101014083757-5qo48b86d69zjvzj) (version source revid:martin.skold@mysql.com-20101014083757-5qo48b86d69zjvzj) (merge vers: 5.1.51-ndb-6.3.39) (pib:21)

Pushed into mysql-5.1-telco-6.2 5.1.51-ndb-6.2.19 (revid:martin.skold@mysql.com-20101014084420-y54ecj85j5we27oa) (version source revid:martin.skold@mysql.com-20101014084420-y54ecj85j5we27oa) (merge vers: 5.1.51-ndb-6.2.19) (pib:21)

Already documented in the 5.1.51 changelog. No new changelog entries required. Reverting to Closed state.

CVE-2010-3837

Pushed into mysql-5.5 5.5.7-rc (revid:sunanda.menon@sun.com-20101109182959-otkxq8vo2dcd13la) (version source revid:jimmy.yang@oracle.com-20100804103744-vbpeghipkz6pyc9z) (merge vers: 5.1.51) (pib:21)

Pushed into mysql-trunk 5.6.99-m5 (revid:alexander.nozdrin@oracle.com-20101113155825-czmva9kg4n31anmu) (version source revid:jimmy.yang@oracle.com-20100804103744-vbpeghipkz6pyc9z) (merge vers: 5.1.51) (pib:21)

Pushed into mysql-next-mr (revid:alexander.nozdrin@oracle.com-20101113160336-atmtmfb3mzm4pz4i) (version source revid:jimmy.yang@oracle.com-20100804103744-vbpeghipkz6pyc9z) (pib:21)

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/126861

2892 Gleb Shchepa	2010-12-14
      backport of bug #54476 fix from 5.1-bugteam to 5.0-bugteam.
      
      Original revid: alexey.kopytov@sun.com-20100723115254-jjwmhq97b9wl932l
      
       > Bug #54476: crash when group_concat and 'with rollup' in
       >                      prepared statements
       >
       > Using GROUP_CONCAT() together with the WITH ROLLUP modifier
       > could crash the server.
       >
       > The reason was a combination of several facts:
       >
       > 1. The Item_func_group_concat class stores pointers to ORDER
       > objects representing the columns in the ORDER BY clause of
       > GROUP_CONCAT().
       >
       > 2. find_order_in_list() called from
       > Item_func_group_concat::setup() modifies the ORDER objects so
       > that their 'item' member points to the arguments list
       > allocated in the Item_func_group_concat constructor.
       >
       > 3. In some cases (e.g. in JOIN::rollup_make_fields) a copy of
       > the original Item_func_group_concat object could be created by
       > using the Item_func_group_concat::Item_func_group_concat(THD
       > *thd, Item_func_group_concat *item) copy constructor. The
       > latter essentially creates a shallow copy of the source
       > object. Memory for the arguments array is allocated on
       > thd->mem_root, but the pointers for arguments and ORDER are
       > copied verbatim.
       >
       > What happens in the test case is that when executing the query
       > for the first time, after a copy of the original
       > Item_func_group_concat object has been created by
       > JOIN::rollup_make_fields(), find_order_in_list() is called for
       > this new object. It then resolves ORDER BY by modifying the
       > ORDER objects so that they point to elements of the arguments
       > array which is local to the cloned object. When thd->mem_root
       > is freed upon completing the execution, pointers in the ORDER
       > objects become invalid. Those ORDER objects, however, are also
       > shared with the original Item_func_group_concat object which is
       > preserved between executions of a prepared statement. So the
       > first call to find_order_in_list() for the original object on
       > the second execution tries to dereference an invalid pointer.
       >
       > The solution is to create copies of the ORDER objects when
       > copying Item_func_group_concat to not leave any stale pointers
       > in other instances with different lifecycles.
     @ mysql-test/r/func_gconcat.result
        Test case for bug #54476.
     @ mysql-test/t/func_gconcat.test
        Test case for bug #54476.
     @ sql/item_sum.cc
        Copy the ORDER objects pointed to by the elements of the
        'order' array in the copy constructor of
        Item_func_group_concat.
     @ sql/table.h
        Removed the unused 'item_copy' member of the ORDER class.

Pushed into mysql-5.0 5.0.92 (revid:georgi.kodinov@oracle.com-20101217124230-1o4se00exicjd3uo) (version source revid:gleb.shchepa@oracle.com-20101214205253-j92ja0i8yuyf2tkx) (merge vers: 5.0.92) (pib:24)

Pushed into mysql-5.1 5.1.55 (revid:georgi.kodinov@oracle.com-20101217124435-9imm43geck5u55qw) (version source revid:gleb.shchepa@oracle.com-20101214205741-zmp7zt8io3j83oi0) (merge vers: 5.1.55) (pib:24)

Pushed into mysql-5.5 5.5.9 (revid:georgi.kodinov@oracle.com-20101217124733-p1ivu6higouawv8l) (version source revid:gleb.shchepa@oracle.com-20101214210243-j5c7u19uhiq6u310) (merge vers: 5.5.8) (pib:24)

Pushed into mysql-trunk 5.6.1 (revid:georgi.kodinov@oracle.com-20101217125013-y8pb3az32rtbplc9) (version source revid:mats.kindahl@oracle.com-20101215103340-a0wp0yq8t4byel1o) (merge vers: 5.6.1) (pib:24)

Noted in 5.0.92 changelog.