andrey Tue, 27 Apr 2010 12:32:34 +0000 Revision: http://svn.php.net/viewvc?view=revision&revision=298657
Log: Fixed very rare memory leak in mysqlnd, when binding thousands of columns Changed paths: U php/php-src/branches/PHP_5_3/NEWS A php/php-src/branches/PHP_5_3/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt U php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_ps_codec.c A php/php-src/trunk/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt U php/php-src/trunk/ext/mysqlnd/mysqlnd_ps_codec.c
Modified: php/php-src/branches/PHP_5_3/NEWS =================================================================== --- php/php-src/branches/PHP_5_3/NEWS 2010-04-27 12:27:15 UTC (rev 298656) +++ php/php-src/branches/PHP_5_3/NEWS 2010-04-27 12:32:34 UTC (rev 298657) @@ -16,8 +16,10 @@ - Implemented FR#35638 (Adding udate to imap_fetch_overview results). (Charles_Duffy at dell dot com ) -- Fixed possible buffer overflows in mysqlnd_list_fields, mysqlnd_change_user +- Fixed possible buffer overflows in mysqlnd_list_fields, mysqlnd_change_user. (Andrey) +- Fixed very rare memory leak in mysqlnd, when binding thousands of columns. + (Andrey) - Fixed handling of session variable serialization on certain prefix characters. Reported by Stefan Esser (Ilia) Added: php/php-src/branches/PHP_5_3/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt =================================================================== --- php/php-src/branches/PHP_5_3/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt (rev 0) +++ php/php-src/branches/PHP_5_3/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt 2010-04-27 12:32:34 UTC (rev 298657) @@ -0,0 +1,76 @@ +--TEST-- +mysqli_stmt_bind_param() - Binding with very high number of columns +--SKIPIF-- +<?php +require_once('skipif.inc'); +require_once('skipifemb.inc'); +require_once('skipifconnectfailure.inc'); +?> +--FILE-- +<?php + /* + The way we test the INSERT and data types overlaps with + the mysqli_stmt_bind_result test in large parts. There is only + one difference. This test uses mysqli_query()/mysqli_fetch_assoc() to + fetch the inserted values. This way we test + mysqli_query()/mysqli_fetch_assoc() for all possible data types + in this file and we test mysqli_stmt_bind_result() in the other + test -- therefore the "duplicate" makes some sense to me. + */ + require_once("connect.inc"); + + if (!$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) { + printf("Cannot connect to the server using host=%s, user=%s, passwd=***, dbname=%s, port=%s, socket=%s\n", + $host, $user, $db, $port, $socket); + exit(1); + } + + if (!mysqli_query($link, 'DROP TABLE IF EXISTS ps_test')) { + printf("Failed to drop old test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + exit(1); + } + + $cols = 2500; + $str = array(); + for ($i = 1; $i <= $cols; $i++) { + $str[] ="a$i INT"; + } + $link->query("CREATE TABLE ps_test(" . implode(" , ", $str) . ")"); + if (mysqli_errno($link)) { + printf("Failed to create the test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + die(""); + } + $stmt = $link->prepare("INSERT INTO ps_test VALUES(".str_repeat("?, ", $cols-1) . "?)"); + var_dump($stmt->id); + $eval_str="\$stmt->bind_param(\"".str_repeat("i",$cols)."\", "; + for ($i = 1; $i < $cols; $i++) { + $eval_str.="\$i,"; + } + $eval_str.="\$i"; + $eval_str.=");"; + eval($eval_str); + var_dump($stmt->execute()); + + mysqli_stmt_close($stmt); + + + mysqli_close($link); + + print "done!"; +?> +--CLEAN-- +<?php + if (!$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) { + printf("Cannot connect to the server using host=%s, user=%s, passwd=***, dbname=%s, port=%s, socket=%s\n", + $host, $user, $db, $port, $socket); + exit(1); + } + if (!mysqli_query($link, 'DROP TABLE IF EXISTS ps_test')) { + printf("Failed to drop the test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + exit(1); + } +?> +--EXPECTF-- +int(1) +bool(true) +done! Modified: php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_ps_codec.c =================================================================== --- php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_ps_codec.c 2010-04-27 12:27:15 UTC (rev 298656) +++ php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_ps_codec.c 2010-04-27 12:32:34 UTC (rev 298657) @@ -599,6 +599,7 @@ { MYSQLND_STMT_DATA * stmt = s->data; unsigned int i = 0; + zend_uchar * provided_buffer = *buf; size_t left = (*buf_len - (*p - *buf)); size_t data_size = 0; zval **copies = NULL;/* if there are different types */ @@ -714,9 +715,17 @@ *buf_len = offset + data_size + 10; /* Allocate + 10 for safety */ tmp_buf = mnd_emalloc(*buf_len); memcpy(tmp_buf, *buf, offset); + /* + When too many columns the buffer provided to the function might not be sufficient. + In this case new buffer has been allocated above. When we allocate a buffer and then + allocate a bigger one here, we should free the first one. + */ + if (*buf != provided_buffer) { + mnd_efree(*buf); + } *buf = tmp_buf; /* Update our pos pointer */ - *p = *buf + offset; + *p = *buf + offset; } /* 2.3 Store the actual data */ Added: php/php-src/trunk/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt =================================================================== --- php/php-src/trunk/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt (rev 0) +++ php/php-src/trunk/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt 2010-04-27 12:32:34 UTC (rev 298657) @@ -0,0 +1,76 @@ +--TEST-- +mysqli_stmt_bind_param() - Binding with very high number of columns +--SKIPIF-- +<?php +require_once('skipif.inc'); +require_once('skipifemb.inc'); +require_once('skipifconnectfailure.inc'); +?> +--FILE-- +<?php + /* + The way we test the INSERT and data types overlaps with + the mysqli_stmt_bind_result test in large parts. There is only + one difference. This test uses mysqli_query()/mysqli_fetch_assoc() to + fetch the inserted values. This way we test + mysqli_query()/mysqli_fetch_assoc() for all possible data types + in this file and we test mysqli_stmt_bind_result() in the other + test -- therefore the "duplicate" makes some sense to me. + */ + require_once("connect.inc"); + + if (!$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) { + printf("Cannot connect to the server using host=%s, user=%s, passwd=***, dbname=%s, port=%s, socket=%s\n", + $host, $user, $db, $port, $socket); + exit(1); + } + + if (!mysqli_query($link, 'DROP TABLE IF EXISTS ps_test')) { + printf("Failed to drop old test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + exit(1); + } + + $cols = 2500; + $str = array(); + for ($i = 1; $i <= $cols; $i++) { + $str[] ="a$i INT"; + } + $link->query("CREATE TABLE ps_test(" . implode(" , ", $str) . ")"); + if (mysqli_errno($link)) { + printf("Failed to create the test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + die(""); + } + $stmt = $link->prepare("INSERT INTO ps_test VALUES(".str_repeat("?, ", $cols-1) . "?)"); + var_dump($stmt->id); + $eval_str="\$stmt->bind_param(\"".str_repeat("i",$cols)."\", "; + for ($i = 1; $i < $cols; $i++) { + $eval_str.="\$i,"; + } + $eval_str.="\$i"; + $eval_str.=");"; + eval($eval_str); + var_dump($stmt->execute()); + + mysqli_stmt_close($stmt); + + + mysqli_close($link); + + print "done!"; +?> +--CLEAN-- +<?php + if (!$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) { + printf("Cannot connect to the server using host=%s, user=%s, passwd=***, dbname=%s, port=%s, socket=%s\n", + $host, $user, $db, $port, $socket); + exit(1); + } + if (!mysqli_query($link, 'DROP TABLE IF EXISTS ps_test')) { + printf("Failed to drop the test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link)); + exit(1); + } +?> +--EXPECTF-- +int(1) +bool(true) +done! Modified: php/php-src/trunk/ext/mysqlnd/mysqlnd_ps_codec.c =================================================================== --- php/php-src/trunk/ext/mysqlnd/mysqlnd_ps_codec.c 2010-04-27 12:27:15 UTC (rev 298656) +++ php/php-src/trunk/ext/mysqlnd/mysqlnd_ps_codec.c 2010-04-27 12:32:34 UTC (rev 298657) @@ -599,6 +599,7 @@ { MYSQLND_STMT_DATA * stmt = s->data; unsigned int i = 0; + zend_uchar * provided_buffer = *buf; size_t left = (*buf_len - (*p - *buf)); size_t data_size = 0; zval **copies = NULL;/* if there are different types */ @@ -714,9 +715,17 @@ *buf_len = offset + data_size + 10; /* Allocate + 10 for safety */ tmp_buf = mnd_emalloc(*buf_len); memcpy(tmp_buf, *buf, offset); + /* + When too many columns the buffer provided to the function might not be sufficient. + In this case new buffer has been allocated above. When we allocate a buffer and then + allocate a bigger one here, we should free the first one. + */ + if (*buf != provided_buffer) { + mnd_efree(*buf); + } *buf = tmp_buf; /* Update our pos pointer */ - *p = *buf + offset; + *p = *buf + offset; } /* 2.3 Store the actual data */
-- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php