[MDEV-3987] uninitialized read in Item_cond::fix_fields leads to crash: select .. where .. in ( select ... ) Created: 2012-12-28  Updated: 2013-01-09  Resolved: 2013-01-09

Status: Closed
Project: MariaDB Server
Component/s: None
Affects Version/s: 10.0.0, 5.5.28a
Fix Version/s: 10.0.2, 5.5.29

Type: Bug Priority: Major
Reporter: sbester1 Assignee: Sergei Golubchik
Resolution: Fixed Votes: 0
Labels: None
Environment:

windows, linux

Attachments: Text File mariadb10.0.0_valgrind.txt    

 Description   

How to repeat:
----------------
#Run mysqld in valgrind. Execute:

drop table if exists `t1`;
 
create table `t1`(`a` char(1) character set utf8)engine=innodb;
 
select 1 from `t1` where `a` in (select group_concat(`a`) from t1);

Windows call stack:
---------------------

Version: '10.0.0-MariaDB-log'  socket: ''  port: 3306  Source distribution
 
121228  7:46:33 [ERROR] mysqld got exception 0xc0000005 ;
 
mysqld.exe!setup_jtbm_semi_joins()[opt_subselect.cc:5224]
 
mysqld.exe!JOIN::optimize_inner()[sql_select.cc:1123]
 
mysqld.exe!JOIN::optimize()[sql_select.cc:992]
 
mysqld.exe!mysql_select()[sql_select.cc:3176]
 
mysqld.exe!handle_select()[sql_select.cc:362]
 
mysqld.exe!execute_sqlcom_select()[sql_parse.cc:4937]
 
mysqld.exe!mysql_execute_command()[sql_parse.cc:2421]
 
mysqld.exe!mysql_parse()[sql_parse.cc:6061]
 
mysqld.exe!dispatch_command()[sql_parse.cc:1219]
 
mysqld.exe!do_command()[sql_parse.cc:951]
 
mysqld.exe!threadpool_process_request()[threadpool_common.cc:225]
 
mysqld.exe!io_completion_callback()[threadpool_win.cc:568]

See attached file for full valgrind outputs.

 Comments   
Comment by Elena Stepanova [ 2012-12-28 ]

On a debug version, causes assertion `0' failure in bool subselect_hash_sj_engine::init(List<Item>*, uint).
The assertion failure is also reproducible with a non-empty MyISAM table:

SET optimizer_switch = 'materialization=on';

create table `t1`(`a` char(1) character set utf8) engine=myisam;
insert into t1 values ('a'),('b');

select 1 from `t1` where `a` in (select group_concat(a) from t1);

Minimal optimizer_switch: materialization=on.

Comment by Timour Katchaounov (Inactive) [ 2013-01-07 ]

Analysis:
The following patch:
revno: 2502.494.2
committer: Sergey Petrunya <psergey@askmonty.org>
branch nick: 5.3-look41
timestamp: Wed 2011-12-07 19:21:51 +0400
message:
BUG#868908: Crash in check_simple_equality() with semijoin + materialization + prepared statement

  • Part2: safety and code cleanup

Adds an assert that makes sure that if non-SJ materialization was chosen, then the temporary table for the materialized subquery was created successfully. This assumes that subquery_types_allow_materialization() is complete, and it would reject all cases when a unique index cannot be created over the materialized subquery.

However, if the subquery to be materialized selects group_concat(), and the collation is utf-8, the test in subquery_types_allow_materialization() is not consistent with the corresponding logic in Item_func_group_concat::make_string_field(). The latter function would create a field of type BLOB for the temporary table if

(max_length / collation.collation->mbminlen > CONVERT_IF_BIGGER_TO_BLOB)

This field is not unique-indexable, so the temporary table for the subquery cannot be used to perform lookups and to compute the IN predicate.

At the same time subquery_types_allow_materialization() tests for

(inner->max_length / inner->collation.collation->mbmaxlen > CONVERT_IF_BIGGER_TO_BLOB)

and thus it decides that materialization is possible.

Comment by Timour Katchaounov (Inactive) [ 2013-01-07 ]

The fix is to correct Item_func_group_concat::make_string_field() to use collation.collation->mbmaxlen instead of collation.collation->mbminlen.

After a discussion with Serg, he said that he created a patch that that removes almost all comparisons with CONVERT_IF_BIGGER_TO_BLOB, and fixes all such inconsistencies.
The test case for this bug will be pushed after Serg pushes his patch.

Comment by Patryk Pomykalski [ 2013-01-07 ]

A quick and dirty solution:
— sql/item_sum.cc 2012-11-22 09:19:31 +0000
+++ sql/item_sum.cc 2013-01-07 16:40:56 +0000
@@ -3213,7 +3213,7 @@
max_characters * CS->mbmaxlen.
*/
const uint32 max_characters= max_length / collation.collation->mbminlen;

  • if (max_characters > CONVERT_IF_BIGGER_TO_BLOB)
    + if (max_characters > collation.collation->mbmaxlen*CONVERT_IF_BIGGER_TO_BLOB)
    field= new Field_blob(max_characters * collation.collation->mbmaxlen,
    maybe_null, name, collation.collation, TRUE);
    else

As a side effect some tests output changes from text to varchar(1024).

Comment by Timour Katchaounov (Inactive) [ 2013-01-09 ]

I approve serg's patch:
http://bazaar.launchpad.net/~maria-captains/maria/5.5-serg/revision/3609

Generated at Thu Feb 08 06:52:51 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.