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

Bug #1165958 reported by Alex Yurchenko on 2013-04-08
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MySQL patches by Codership
Medium
Yan Zhang
Percona XtraDB Cluster
Status tracked in 5.6
5.5
Undecided
Unassigned
5.6
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;

Related branches

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
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)
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.

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
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers