Segfault on INSERT DELAYED and wsrep_replicate_myisam=1 due to unchecked dereference of NULL pointer

Bug #1165958 reported by Alex Yurchenko
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MySQL patches by Codership
Fix Released
Medium
Yan Zhang
Percona XtraDB Cluster moved to https://jira.percona.com/projects/PXC
Status tracked in 5.6
5.5
Fix Released
Undecided
Unassigned
5.6
New
Undecided
Unassigned

Bug Description

User reported a segfault, easily reproducible in his environment:

(gdb) bt
#0 0x000000344540c65c in __pthread_kill (threadid=<optimized out>, signo=11)
    at ../nptl/sysdeps/unix/sysv/linux/pthread_kill.c:63
#1 0x00000000006650f2 in handle_fatal_signal (sig=11)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/signal_handler.cc:247
#2 <signal handler called>
#3 0x000000000053faff in open_tables (thd=0x2385320, start=0x7f87fc62cc68,
    counter=0x7f87fc62cc8c, flags=0, prelocking_strategy=0x7f87fc62cf50)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_base.cc:5027
#4 0x0000000000540517 in open_and_lock_tables (thd=<optimized out>,
    tables=0x0, derived=true, flags=0, prelocking_strategy=<optimized out>)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_base.cc:5540
#5 0x000000000055f075 in open_and_lock_tables (flags=0, derived=true,
    tables=<optimized out>, thd=0x2385320)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_base.h:475
#6 open_and_lock_for_insert_delayed (thd=0x2385320,
    table_list=0x7f87a0004d50)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_insert.cc:573
#7 0x0000000000560208 in mysql_insert (thd=0x2385320,
    table_list=0x7f87a0004d50, fields=..., values_list=...,
    update_fields=..., update_values=..., duplic=DUP_ERROR, ignore=false)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_insert.cc:711
#8 0x0000000000570c28 in mysql_execute_command (thd=0x2385320)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_parse.cc:3227
#9 0x00000000005748e9 in mysql_parse (thd=0x2385320, rawbuf=<optimized out>,
    length=106, parser_state=0x7f87fc62f6a0)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_parse.cc:6203
#10 0x0000000000575710 in wsrep_mysql_parse (thd=0x2385320,
    rawbuf=0x7f87a0004ba0 "INSERT DELAYED INTO throttle_from_instance (_instance,_expire) VALUES ('6f47.515d7f9a.a6201.0',1365082010)", length=106,
    parser_state=0x7f87fc62f6a0)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_parse.cc:6036
#11 0x00000000005770d3 in dispatch_command (command=COM_QUERY, thd=0x2385320,
    packet=<optimized out>, packet_length=<optimized out>)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_parse.cc:1212
#12 0x000000000057783f in do_command (thd=0x2385320)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_parse.cc:869
#13 0x00000000006039ae in do_handle_one_connection (thd_arg=<optimized out>)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_connect.cc:878
#14 0x0000000000603a2a in handle_one_connection (arg=0x2381310)
    at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_connect.cc:790
#15 0x0000003445407d15 in start_thread (arg=0x7f87fc630700)
    at pthread_create.c:308
#16 0x00000034448f246d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:102

Offending line at /home/devel/galera/2.x/mysql-5.5.29_wsrep_23.7.3/sql/sql_base.cc:5027 is

  if ((thd->lex->sql_command== SQLCOM_INSERT ||
       thd->lex->sql_command== SQLCOM_INSERT_SELECT ||
       thd->lex->sql_command== SQLCOM_REPLACE ||
       thd->lex->sql_command== SQLCOM_REPLACE_SELECT ||
       thd->lex->sql_command== SQLCOM_UPDATE ||
       thd->lex->sql_command== SQLCOM_UPDATE_MULTI ||
       thd->lex->sql_command== SQLCOM_LOAD ||
       thd->lex->sql_command== SQLCOM_DELETE) &&
      wsrep_replicate_myisam &&
      (*start)->table && (*start)->table->file->ht->db_type == DB_TYPE_MYISAM)

Relevant variables:

(gdb) p thd->lex->sql_command
$1 = SQLCOM_INSERT
(gdb) p wsrep_replicate_myisam
$2 = 1 '\001'
(gdb) p *start
$3 = (TABLE_LIST *) 0x0
(gdb) p thd->query
Cannot take address of method query.
(gdb) p thd->query_string
$4 = {string = {
    str = 0x7f87a0004ba0 "INSERT DELAYED INTO throttle_from_instance (_instance,_expire) VALUES ('6f47.515d7f9a.a6201.0',1365082010)", length = 106},
  cs = 0xf0ec40 <my_charset_latin1>}
(gdb) p thd->rli_slave
$5 = (Relay_log_info *) 0x0
(gdb) p thd->slave_thread
$6 = false
(gdb) p thd->wsrep_applier
$7 = false

Table specification:

CREATE TABLE `throttle_from_instance` (
  `_instance` char(60) NOT NULL DEFAULT '',
  `_from` char(60) NOT NULL DEFAULT '',
  `_expire` int(10) unsigned NOT NULL DEFAULT '0',
  UNIQUE KEY `_instance` (`_instance`),
  KEY `_expire` (`_expire`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1;

Changed in codership-mysql:
milestone: 5.5.30-24.8 → 5.5.31-23.7.4
Changed in codership-mysql:
milestone: 5.5.31-23.7.5 → 5.5.32-23.7.6
Changed in codership-mysql:
milestone: 5.5.33-23.7.6 → 5.5.34-24.9
importance: Undecided → Medium
Changed in codership-mysql:
milestone: 5.5.34-25.9 → 5.5.34-25.10
Revision history for this message
Yan Zhang (yan.zhang) wrote :

it only happens on 5.5, not on 5.6.

fixed in http://bazaar.launchpad.net/~codership/codership-mysql/wsrep-5.5/revision/3973

to reproduce bug, the table specification should be MyISAM instead of InnoDB.
```
CREATE TABLE `throttle_from_instance` (
  `_instance` char(60) NOT NULL DEFAULT '',
  `_from` char(60) NOT NULL DEFAULT '',
  `_expire` int(10) unsigned NOT NULL DEFAULT '0',
  UNIQUE KEY `_instance` (`_instance`),
  KEY `_expire` (`_expire`)
) ENGINE=MyISAM DEFAULT CHARSET=latin1;
```

Changed in codership-mysql:
status: Confirmed → Fix Committed
assignee: Seppo Jaakola (seppo-jaakola) → Yan Zhang (yan.zhang)
Revision history for this message
Alex Yurchenko (ayurchen) wrote :

So the questions are
- why doesn't it happen on 5.6?
- apparently because control doesn't reach that codepath.
- So why does it not? And why does it on 5.5? Perhaps the real bug is that this codepath is reached, not that the pointer is not checked for NULL?

In other words, I find it strange that 5.5 needs that check and 5.6 does not.

Revision history for this message
Yan Zhang (yan.zhang) wrote :

yes. I have checked the difference of 5.5 and 5.6. code path is big different. And I can't figure out the meaning of code now, so I just say "not on 5.6". I know it's not good for reasoning problem, but it will take a little bit long time for me to figure it out the root cause.

in 5.5, code is like
```
for (tables= *start; tables; tables= tables->next_global)
  {
    TABLE *tbl= tables->table;
  }
if (... && (*start)->table && (*start)->table->file->ht->db_type == DB_TYPE_MYISAM))
```

but in 5.5, code is like
```
for (tables= *start; tables; tables= tables->next_global)
  {
    TABLE *tbl= tables->table;
    if (... && (*start)->table && (*start)->table->file->ht->db_type == DB_TYPE_MYISAM))
  }
```

the 'if' condition is out the loop in 5.5, and in the loop in 5.6.

To my intuition, 5.6 is relatively righter than 5.5. But I think 5.6 would be wrong too. To my intuition, 'if' condition in 5.6 should be (tbl && tbl->file->ht->db_type == DB_TYPE_MYISAM) instead.

Changed in codership-mysql:
status: Fix Committed → Fix Released
Revision history for this message
Shahriyar Rzayev (rzayev-sehriyar) wrote :

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PXC-1326

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.