| Bug #45048 | prepared statements corrupt the heap | ||
|---|---|---|---|
| Submitted: | 23 May 2009 16:58 | Modified: | 11 Sep 2009 15:14 |
| Reporter: | Boyan Kasarov | Email Updates: | |
| Status: | Closed | Impact on me: | |
| Category: | Connector / C++ | Severity: | S1 (Critical) |
| Version: | 2009-05-22 10:56:07 | OS: | Linux (gentoo x86_64) |
| Assigned to: | Andrey Hristov | CPU Architecture: | Any |
[23 May 2009 17:00]
Boyan Kasarov
valgrind log from running unmodified test.
Attachment: valgrind_test_output.txt (text/plain), 32.77 KiB.
[23 May 2009 17:02]
Boyan Kasarov
valgrind log from running the test with the patch from the description.
Attachment: valgrind_test_patched.txt (text/plain), 65.19 KiB.
[23 May 2009 17:04]
Boyan Kasarov
Output from the test, same both for patched and not-patched test.
Attachment: test_output.txt (text/plain), 1.03 KiB.
[23 May 2009 17:12]
Boyan Kasarov
The valgrind result for test/unit/classes/preparedstatement from 1.0.4-beta is perfect, not a single error.
[25 May 2009 19:10]
MySQL Verification Team
Please verify if http://bugs.mysql.com/bug.php?id=45061 is related to this bug. Thanks in advance.
[26 May 2009 13:22]
Ulf Wendel
Interesting find. Can be reproduced but only when patching the test. ok 2 - preparedstatement::anonymousSelect ==3753== ==3753== Invalid write of size 8 ==3753== at 0x4DEE1DA: setup_one_fetch_function (in /usr/lib64/libmysqlclient_r.so.15.0.0) ==3753== by 0x4DF3330: mysql_stmt_execute (in /usr/lib64/libmysqlclient_r.so.15.0.0) ==3753== by 0x4B951F0: sql::mysql::MySQL_Prepared_Statement::do_query() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/driver/libmysqlcppconn.so.1.0.5) ==3753== by 0x4B930FB: sql::mysql::MySQL_Prepared_Statement::executeQuery() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/driver/libmysqlcppconn.so.1.0.5) ==3753== by 0x454A80: testsuite::classes::preparedstatement::InsertSelectAllTypes() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x456EB5: testsuite::TestCase<testsuite::classes::preparedstatement>::runTest() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x461C23: testsuite::Private::TestContainer::StorableTest::runTest() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x459042: testsuite::TestSuite::runTest() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x46187B: testsuite::TestsRunner::runTests() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x44A07F: main (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== Address 0x7498bb0 is 8 bytes inside a block of size 16 free'd ==3753== at 0x4A1F1DE: operator delete[](void*) (vg_replace_malloc.c:364) ==3753== by 0x4B95FF5: sql::mysql::util::my_array_guard<unsigned long>::~my_array_guard() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/driver/libmysqlcppconn.so.1.0.5) ==3753== by 0x4B9E2F6: sql::mysql::MySQL_ResultBind::~MySQL_ResultBind() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/driver/libmysqlcppconn.so.1.0.5) ==3753== by 0x4B9669A: std::auto_ptr<sql::mysql::MySQL_ResultBind>::~auto_ptr() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/driver/libmysqlcppconn.so.1.0.5) ==3753== by 0x4B9B115: sql::mysql::MySQL_Prepared_ResultSet::~MySQL_Prepared_ResultSet() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/driver/libmysqlcppconn.so.1.0.5) ==3753== by 0x449114: std::auto_ptr<sql::ResultSet>::reset(sql::ResultSet*) (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x454A61: testsuite::classes::preparedstatement::InsertSelectAllTypes() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x456EB5: testsuite::TestCase<testsuite::classes::preparedstatement>::runTest() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x461C23: testsuite::Private::TestContainer::StorableTest::runTest() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x459042: testsuite::TestSuite::runTest() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x46187B: testsuite::TestsRunner::runTests() (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement) ==3753== by 0x44A07F: main (in /home/nixnutz/src/connector-cpp-bzr/releases/v1_0_5/test/unit/classes/preparedstatement)
[14 Jul 2009 9:59]
Ulf Wendel
Andrey, this will happen only if one resets the auto_ptr twice. Meaning, it could be a user thingie and not a driver bug. However, I prefer having you check this instead of looking at it myself.
[15 Jul 2009 18:28]
Boyan Kasarov
Hello,
Here is how I see the problem:
Quote from mysql manual:
*******************************************************************************
mysql_stmt_bind_result() is used to associate (that is, bind) output columns in
the result set to data buffers and length buffers. When mysql_stmt_fetch() is
called to fetch data, the MySQL client/server protocol places the data for the
bound columns into the specified buffers.
*******************************************************************************
I undertand this as mysql_stmt_bind_result() sets the MYSQL_BIND where the client library places the result from query. This pointer stays in the MYSQL_STMT structure.
in
void MySQL_ResultBind::bindResult() mysql_resultbind.cpp:160
{
// ....
rbind[i].length = &len[i];
// ....
then in
mysql_stmt_bind_result(MYSQL_STMT *stmt, MYSQL_BIND *my_bind) libmysql.c:4420
{
/* ... */
if (stmt->bind != my_bind)
memcpy((char*) stmt->bind, (char*) my_bind,
sizeof(MYSQL_BIND) * bind_count);
/* ... */
This means we copy the the address of 'len' array (member of MySQL_ResultBind) inside MYSQL_STMT. If we execute one prepared statement, then delete the result and then execute another prepared statement, it results in access to free'd memory which usually corrupts the heap. This basically means you cannot execute a prepared statement query more than once (which makes the use of prepared statements pointless).
This happens not only to 'len' array, but also to 'is_null' and 'err' of MySQL_ResultBind.
I hope this helps to understand where the problem is, if its still unclear, please ask.
I hope we can find some solution to this.
[15 Jul 2009 19:05]
Boyan Kasarov
Continued from previous comment: Looking at this link http://java.sun.com/javase/6/docs/api/java/sql/ResultSet.html Quote: ******************************************************************************* A ResultSet object is automatically closed when the Statement object that generated it is closed, re-executed, or used to retrieve the next result from a sequence of multiple results. ******************************************************************************* Exactly "re-executed" is the part we are interested of, for prepared statements. I undestand this as only one 'live' ResultSet may exist from every statement, the arrays can be moved somewhere inside MySQL_Prepared_Statement, and not be bound to the ResultSet. I think this is how it was implemented in 1.0.4-beta release, which is unaffected of this bug. But in 1.0.4-beta MySQL_Prepared_Statement didn't have a way to close() the previous result when it executes a new query.
[19 Aug 2009 11:52]
Andrey Hristov
Fixed in trunk. Will be fixed in the next release 1.0.6
[11 Sep 2009 15:14]
Tony Bedford
An entry was added to the 1.1.0 (formerly 1.0.6) changelog: Using Prepared Statements caused corruption of the heap.
Description: In the following scenario, prepared statements stopped working after 1.0.5, in 1.0.4-beta they were working fine. Running test/unit/classes/preparedstatement with valgrind results in memory errors. My server version is 5.0.70. How to repeat: Apply this patch over the test suite and run preparedstatement test to trigger an access to free'd memory. === modified file 'test/unit/classes/preparedstatement.cpp' Just apply this patch and run prepared --- test/unit/classes/preparedstatement.cpp 2009-05-22 10:56:07 +0000 +++ test/unit/classes/preparedstatement.cpp 2009-05-23 16:05:03 +0000 @@ -64,6 +64,12 @@ checkResultSetScrolling(res); ASSERT(res->next()); + res.reset(); + res.reset(pstmt->executeQuery()); + checkResultSetScrolling(res); + ASSERT(res->next()); + + if (it->check_as_string && (res->getString(1) != it->as_string)) { sql.str("");