Bug #46652 ndb_restore_compat gives Valgrind error in open_binary_frm()
Submitted: 11 Aug 2009 14:39 Modified: 1 Sep 2009 17:50
Reporter: Guilhem Bichot Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S3 (Non-critical)
Version:5.4.2 Summit OS:Linux
Assigned to: Guilhem Bichot CPU Architecture:Any

[11 Aug 2009 14:39] Guilhem Bichot
Description:
I take the latest 5.1-performance-version (the tree for Summit):
revision-id:k.long@sun.com-20090721201009-ikszmudds8gwiyst
and I run
 ./mtr --mem --valgrind ndb_restore_compat --mysqld=--skip-innodb
then I see:
==16804== Invalid read of size 1
==16804==    at 0x729A34: _ZL15open_binary_frmP3THDP14st_table_sharePhi (table.cc:977)
==16804==    by 0x72C269: open_table_def(THD*, st_table_share*, unsigned) (table.cc:630)
==16804==    by 0x7FF3D5: ha_create_table_from_engine(THD*, char const*, char const*) (handler.cc:3605)
==16804==    by 0x71A81C: _ZL27get_table_share_with_createP3THDP10TABLE_LISTPcjjPi (sql_base.cc:481)
==16804==    by 0x71AA93: _ZL17open_unireg_entryP3THDP8st_tableP10TABLE_LISTPKcPcjP11st_mem_rootj (sql_base.cc:3837)
==16804==    by 0x71D785: open_table(THD*, TABLE_LIST*, st_mem_root*, bool*, unsigned) (sql_base.cc:2907)
==16804==    by 0x71E66D: open_tables(THD*, TABLE_LIST**, unsigned*, unsigned) (sql_base.cc:4570)
==16804==    by 0x71EF6A: open_and_lock_tables_derived(THD*, TABLE_LIST*, bool) (sql_base.cc:4976)
==16804==    by 0x6D4106: open_and_lock_tables(THD*, TABLE_LIST*) (mysql_priv.h:1557)
==16804==    by 0x6C61B2: _ZL21execute_sqlcom_selectP3THDP10TABLE_LIST (sql_parse.cc:5037)
==16804==    by 0x6C83BC: mysql_execute_command(THD*) (sql_parse.cc:2241)
==16804==    by 0x6D10FB: mysql_parse(THD*, char const*, unsigned, char const**) (sql_parse.cc:6000)
==16804==    by 0x6D1EB5: dispatch_command(enum_server_command, THD*, char*, unsigned) (sql_parse.cc:1225)
==16804==    by 0x6D3293: do_command(THD*) (sql_parse.cc:858)
==16804==    by 0x6BFAD2: handle_one_connection (sql_connect.cc:1119)
==16804==    by 0x4E323E9: start_thread (in /lib/libpthread-2.8.90.so)
==16804==  Address 0x72ea516 is 0 bytes after a block of size 14 alloc'd
==16804==    at 0x4C265AE: malloc (vg_replace_malloc.c:207)
==16804==    by 0xAF53F9: my_malloc (my_malloc.c:34)
==16804==    by 0x729547: _ZL15open_binary_frmP3THDP14st_table_sharePhi (table.cc:858)
==16804==    by 0x72C269: open_table_def(THD*, st_table_share*, unsigned) (table.cc:630)
==16804==    by 0x7FF3D5: ha_create_table_from_engine(THD*, char const*, char const*) (handler.cc:3605)
==16804==    by 0x71A81C: _ZL27get_table_share_with_createP3THDP10TABLE_LISTPcjjPi (sql_base.cc:481)
==16804==    by 0x71AA93: _ZL17open_unireg_entryP3THDP8st_tableP10TABLE_LISTPKcPcjP11st_mem_rootj (sql_base.cc:3837)
==16804==    by 0x71D785: open_table(THD*, TABLE_LIST*, st_mem_root*, bool*, unsigned) (sql_base.cc:2907)
==16804==    by 0x71E66D: open_tables(THD*, TABLE_LIST**, unsigned*, unsigned) (sql_base.cc:4570)
==16804==    by 0x71EF6A: open_and_lock_tables_derived(THD*, TABLE_LIST*, bool) (sql_base.cc:4976)
==16804==    by 0x6D4106: open_and_lock_tables(THD*, TABLE_LIST*) (mysql_priv.h:1557)
==16804==    by 0x6C61B2: _ZL21execute_sqlcom_selectP3THDP10TABLE_LIST (sql_parse.cc:5037)
==16804==    by 0x6C83BC: mysql_execute_command(THD*) (sql_parse.cc:2241)
==16804==    by 0x6D10FB: mysql_parse(THD*, char const*, unsigned, char const**) (sql_parse.cc:6000)
==16804==    by 0x6D1EB5: dispatch_command(enum_server_command, THD*, char*, unsigned) (sql_parse.cc:1225)
==16804==    by 0x6D3293: do_command(THD*) (sql_parse.cc:858)

It is not seen in pushbuild2 because after failing 10 tests for some reasons, mtr stops, so does not come far enough to run ndb_restore_compat, see here:
http://trollheim.norway.sun.com/archive/623247.log
"Too many tests(10) failed! Terminating.."

Problem matters to me because it also happens in mysql-trunk which is a child of 5.1-performance-version.

How to repeat:
see description.

Suggested fix:
The key is the version number: 5.4.2. Scenario:
- the invalid read is a read just one byte after the end of an allocated buffer:
table.cc, open_binary_frm(): buffer is allocated in
    if (!(next_chunk= buff= (uchar*) my_malloc(n_length, MYF(MY_WME))))
      goto err;
(around line 860)
and is overrun in the same function:
    else if (share->mysql_version >= 50110)
#endif
    {
      /* New auto_partitioned indicator introduced in 5.1.11 */
#ifdef WITH_PARTITION_STORAGE_ENGINE
      share->auto_partitioned= *next_chunk;
#endif
^ that's the invalid read;
If you look at the lines just before the invalid read, there is code inside
#if MYSQL_VERSION_ID < 50200
so in Summit, this #ifdef/endif piece is skipped, and thus "share->auto_partitioned= *next_chunk;" is always executed.
Whereas in 5.1, it is executed only if:
     if (share->mysql_version >= 50106 && share->mysql_version <= 50109)
is false and
else if (share->mysql_version >= 50110)
is true.
In the problematic case, share->mysql_version is 50044 (the test is testing backward compatibility), so both conditions are false and the invalid read line is not executed.
Suggested fix: the same as sp1r-serg@janus.mylan-20070713181440-16952
which is present in the 6.0-based azalea:
=== modified file 'sql/table.cc'
--- sql/table.cc	2007-07-12 11:54:47 +0000
+++ sql/table.cc	2007-07-13 18:14:40 +0000
@@ -912,8 +912,9 @@
       */
       next_chunk+= 4;
     }
-    else if (share->mysql_version >= 50110)
+    else
 #endif
+    if (share->mysql_version >= 50110)
     {
       /* New auto_partitioned indicator introduced in 5.1.11 */
 #ifdef WITH_PARTITION_STORAGE_ENGINE

This looks logical and eliminates the Valgrind error.
[11 Aug 2009 17:46] Bugs System
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/80621

2878 Kelly Long	2009-08-11
      Fix for Bug #46652.  Logic for mysql_version was not in correct #ifdef location.
[20 Aug 2009 11:01] Konstantin Osipov
The patch is already present in 5.4.4 (now betony, 6.0-based), good to go into Summit.
[24 Aug 2009 6:52] Alexander Nozdrin
The patch is already in mysql-trunk (Azalea release).
[1 Sep 2009 17:50] Paul Dubois
Noted in 5.4.2 changelog.

open_binary_frm() performed a version test incorrectly, resulting in
Valgrind errors.
[16 Sep 2009 6:45] Bugs System
Pushed into 5.4.4-alpha (revid:alik@sun.com-20090916063112-8hjmu6wkxfx5qxf4) (version source revid:alik@sun.com-20090916060909-uz4mt7vnvf68g6rk) (merge vers: 5.4.4-alpha) (pib:11)