Bug #19022 Memory bug when switching db during trigger execution

Description:
Found this memory bug with Valgrind:

VALGRIND: 'Invalid read of size 1'
    COUNT: 2
    FUNCTION: strlen    FILES:    slave.err
    TESTS:    rpl_trigger
    STACK: at 0x4A1A792: strlen (mac_replace_strmem.c:243)
             by 0x60FE73: Query_log_event::Query_log_event(THD*, char const*, unsigned long, bool, bool) (sql_string.h:90)
             by 0x5F859F: mysql_insert(THD*, st_table_list*, List<Item>&, List<List<Item> >&, List<Item>&, List<Item>&, enum_duplicates, bool) (sql_insert.cc:569)
             by 0x5AD0F7: mysql_execute_command(THD*) (sql_parse.cc:3270)
             by 0x5B1EB1: mysql_parse(THD*, char*, unsigned) (sql_parse.cc:5710)
             by 0x610CB9: Query_log_event::exec_event(st_relay_log_info*, char const*, unsigned) (log_event.cc:1745)
             by 0x68C4DF: exec_relay_log_event(THD*, st_relay_log_info*) (slave.cc:3336)
             by 0x68A409: handle_slave_sql (slave.cc:3891)
             by 0x4C3CC63: start_thread (in /lib64/tls/libpthread-0.60.so)
             by 0x52F8242: clone (in /lib64/tls/libc-2.3.2.so)
           Address 0x57E48A0 is just below the stack ptr.  To suppress, use: --workaround-gcc296-bugs=yes

I tracked it down, and it is actually a fairly serious memory problem.

The bug happens in sp_head::execute() (sql/sp_head.cc). It happens only during replication, in the slave SQL thread.

This function first calls sp_use_new_db() to temporarily switch the current db to the one of the trigger to be executed. The old db is copied to the stack-allocated char olddb[128] variable.

Then after execution the old db is restored:

      err_status|= mysql_change_db(thd, olddb, 1);

But when running in the slave SQL thread, the mysql_change_db function essentially does this:

    thd->db = olddb;

Thus, upon return from sp_head::execute(), the thd->db is left pointing into deallocated stack space (olddb[128] is invalid after return from sp_head::execute(), causing Valgrind to complain). And the original thd->db pointer is lost.

How to repeat:
perl mysql-test-run.pl --valgrind rpl_trigger

Suggested fix:
That code really needs refactoring.

It currently has very complicated free() semantics for thd->db, which is very error prone, this is likely not the only bug lurking in there.

Alternatively, sp_head::execute() could itself check if thd->slave_thread is set, and if so could store the old thd->db initially and pass that to mysql_change_db() instead of the olddb[128] buffer.

Leaving a comment on possible relating, as a child, of bug#20579 to this case.

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/8358

Lars, please review or assign a reviewer.

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/8428

Second review done by IRC, patch approved.
The actual review is performed by Andrei Elkin.

Pushed into 5.0-runtime

Pushed into 5.0-release and 5.0

In other words, this bug made it into 5.0.23

Noted in 5.0.23 changelog.

Need three-part version number for 5.1 when
patch merges upward.

Merged into 5.1 tree currently tagged 5.1.12

Noted in 5.1.12 changelog.

Pushed into 5.0.12

5.1.12

Pushed into 5.4.4-alpha (revid:alik@sun.com-20090617073019-azsawauatv99124t) (version source revid:jon.hauglid@sun.com-20090608082432-o27d1guc3g6qmk5r) (merge vers: 5.4.4-alpha) (pib:11)

Pushed into 6.0.14-alpha (revid:alik@sun.com-20091102151658-j9o4wgro47m5v84d) (version source revid:alik@ibmvm-20091009114750-utkhs9029vewkluf) (merge vers: 6.0.14-alpha) (pib:13)

Noted in 5.4.4, 6.0.14 changelogs.

Pushed into 5.5.0-beta (revid:alik@sun.com-20091110093229-0bh5hix780cyeicl) (version source revid:mikael@mysql.com-20091103113702-p61dlwc6ml6fxg18) (merge vers: 5.5.0-beta) (pib:13)

Noted in 5.5.0 changelog.