Bug #41411 Access invalid memory for sequence after table truncate
Submitted: 11 Dec 2008 19:48 Modified: 15 May 2009 12:59
Reporter: Kelly Long Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Falcon storage engine Severity:S2 (Serious)
Version:6.0.8 OS:Any
Assigned to: Kevin Lewis CPU Architecture:Any
Tags: F_DDL

[11 Dec 2008 19:48] Kelly Long
Description:
Truncating a table does not restart the sequence value at zero as it is accessing a pointer that was invalidated but not re-set for the new sequence.

Valgrind output:
==25002== Thread 23:
==25002== Invalid read of size 4
==25002==    at 0x943685: Sequence::update(long long, Transaction*) (Sequence.cpp:46)
==25002==    by 0x8AA8A7: StorageInterface::get_auto_increment(unsigned long long, unsigned long long, unsigned long long, unsigned long long*, unsigned long long*) (ha_falcon.cpp:2049)
==25002==    by 0x791672: handler::update_auto_increment() (handler.cc:2338)
==25002==    by 0x8AE17D: StorageInterface::write_row(unsigned char*) (ha_falcon.cpp:1108)
==25002==    by 0x7943AE: handler::ha_write_row(unsigned char*) (handler.cc:5362)
==25002==    by 0x72097A: write_record(THD*, TABLE*, st_copy_info*) (sql_insert.cc:1609)
==25002==    by 0x7257BA: mysql_insert(THD*, TABLE_LIST*, List<Item>&, List<List<Item> >&, List<Item>&, List<Item>&, enum_duplicates, bool) (sql_insert.cc:835)
==25002==    by 0x6A5D72: mysql_execute_command(THD*) (sql_parse.cc:3109)
==25002==    by 0x6A92C2: mysql_parse(THD*, char const*, unsigned, char const**) (sql_parse.cc:5737)
==25002==    by 0x6A9B99: dispatch_command(enum_server_command, THD*, char*, unsigned) (sql_parse.cc:1006)
==25002==    by 0x6AA625: do_command(THD*) (sql_parse.cc:689)
==25002==    by 0x69C883: handle_one_connection (sql_connect.cc:1156)
==25002==    by 0x3EFF206406: start_thread (in /lib64/libpthread-2.7.so)
==25002==    by 0x3EFE6D4B0C: clone (in /lib64/libc-2.7.so)
==25002==  Address 0x1210C3D8 is 32 bytes inside a block of size 64 free'd
==25002==    at 0x4A0560B: free (vg_replace_malloc.c:233)
==25002==    by 0x91C324: MemMgrPoolRelease(MemMgr*, void*) (MemMgrV0SysDefault.cpp:289)
==25002==    by 0x9436D7: Sequence::~Sequence() (MemoryManager.h:92)
==25002==    by 0x942DB5: SequenceManager::deleteSequence(char const*, char const*) (SequenceManager.cpp:166)
==25002==    by 0x942FC8: SequenceManager::recreateSequence(Sequence*) (SequenceManager.cpp:143)
==25002==    by 0x8F4BC5: Database::truncateTable(Table*, Sequence*, Transaction*) (Database.cpp:1519)
==25002==    by 0x8B4365: StorageDatabase::truncateTable(StorageConnection*, StorageTableShare*) (StorageDatabase.cpp:608)
==25002==    by 0x735429: mysql_delete(THD*, TABLE_LIST*, Item*, st_sql_list*, unsigned long long, unsigned long long, bool) (sql_delete.cc:140)
==25002==    by 0x735E6A: mysql_truncate(THD*, TABLE_LIST*, bool) (sql_delete.cc:1116)
==25002==    by 0x6A6754: mysql_execute_command(THD*) (sql_parse.cc:3217)
==25002==    by 0x6A92C2: mysql_parse(THD*, char const*, unsigned, char const**) (sql_parse.cc:5737)
==25002==    by 0x6A9B99: dispatch_command(enum_server_command, THD*, char*, unsigned) (sql_parse.cc:1006)
==25002==    by 0x6AA625: do_command(THD*) (sql_parse.cc:689)
==25002==    by 0x69C883: handle_one_connection (sql_connect.cc:1156)
==25002==    by 0x3EFF206406: start_thread (in /lib64/libpthread-2.7.so)
==25002==    by 0x3EFE6D4B0C: clone (in /lib64/libc-2.7.so)

How to repeat:
1. Compile falcon code Without falcon memory manager.
2. execute test for but 22173

./mysql-test-run.pl --suite=falcon --force falcon_bug_22173

This test will fail.  It is expecting the automatic sequence to be 1, 2 after truncate but the sequence is 3, 4.  This is because the old sequence memory is being used after it has been free'd.

This does not currently show up with falcons memory manager as the memory manager will re-use the same memory address for the new sequence as it just previously deleted.  Thus the dangling pointer is actually invalid for a moment then valid again.  If falcon is under load such that the memory manager uses this block for something else then the dangling pointer will point to memory that is NOT a sequence and things will get VERY scary.

Suggested fix:
=== modified file 'storage/falcon/StorageTableShare.cpp'
--- storage/falcon/StorageTableShare.cpp    2008-11-05 14:51:37 +0000
+++ storage/falcon/StorageTableShare.cpp    2008-12-11 19:20:19 +0000
@@ -258,6 +258,9 @@
 int StorageTableShare::truncateTable(StorageConnection *storageConnection)
 {
    int res = storageDatabase->truncateTable(storageConnection, this);
+   // BEGIN KELLY fix for falcon_bug_22173
+   sequence = storageDatabase->findSequence(name, schemaName);
+   // END KELLY fix for falcon_bug_22173
    
    return res;
[9 Jan 2009 16:42] Kevin Lewis
This is an internal bug that manifests only after switching out the memory manager.  So it is not currently seen outside engineering.
[9 Jan 2009 16:45] Kevin Lewis
Changing Impact based on Kelly's previous comment
[9 Jan 2009 22:06] 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/62905

2966 Kevin Lewis	2009-01-09
      Bug#41411 - The sequence pointer must be frefreshed 
      because Database::truncate() does this;
         sequence = sequence->recreate();
      In a hoghly concurrent environment or with a different
      memory manager, the sequence pointer might be different 
      from before.  So the pointer in StoragetableShare must
      be refreshed as well.
[9 Jan 2009 22:21] Kevin Lewis
The suggested fix by Kelly is reviewed and implemented by Kevin, who checked it in.
[13 Feb 2009 7:24] Bugs System
Pushed into 6.0.10-alpha (revid:alik@sun.com-20090211182317-uagkyj01fk30p1f8) (version source revid:olav@sun.com-20090113103017-41jbad7qlvlwpwxh) (merge vers: 6.0.10-alpha) (pib:6)
[15 May 2009 12:59] MC Brown
An entry has been added to the 6.0.10 changelog: 

When using TRUNCATE on a Falcon table, the sequence value for auto increment columns would not be reset correctly.