Commit 01521a0a authored by Gleb Shchepa's avatar Gleb Shchepa

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.
parent a723fa87
......@@ -989,4 +989,22 @@ SELECT 1 FROM
1
1
DROP TABLE t1;
#
# 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.0 tests
......@@ -708,4 +708,18 @@ SELECT 1 FROM
DROP TABLE t1;
--echo #
--echo # Bug #54476: crash when group_concat and 'with rollup' in prepared statements
--echo #
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.0 tests
......@@ -3170,7 +3170,6 @@ Item_func_group_concat::Item_func_group_concat(THD *thd,
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),
......@@ -3183,6 +3182,24 @@ Item_func_group_concat::Item_func_group_concat(THD *thd,
{
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;
}
}
......
......@@ -31,7 +31,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 */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment