uw                                       Wed, 16 Sep 2009 17:03:44 +0000

Revision: http://svn.php.net/viewvc?view=revision&revision=288379

Log:
Fix (by Andrey) and test for bug #49442 . Don't use efree() for memory 
allocated with malloc()... If a connection gets created by mysqli_init(), 
mysqlnd makes it 'persistent'. 'Persistent' means that mysqlnd uses malloc(). 
mysqlnd does use malloc() instead of ealloc() because it is unknown if the 
connection will become a true persistent connection in the sense of ext/mysqli. 
It is unknown if the user wants a persistent connection or not until the user 
calls mysqli_real_connect(). To avoid tricky conversions mysqlnd uses malloc(), 
which sets a private persistent flag in the mysqlnd structures. A precondition 
for the crash to happen was that the private persistent flag is set. The flag 
is also set when creating a real persistent connection (in the sense of 
ext/mysqli) and so the bug can happen with mysql_init()/mysqli_real_connect() 
and mysql_connect('p:<host>', ...). Therefore we test both cases. Note the 
(tricky?) difference between the implementation detail'mysqlnd private !
 persistent flag = use malloc()' and persistent connections from a user 
perspective. Although mysqlnd will always set its private persistent flag and  
use malloc() for connections created with mysqli_init() it is still up to the 
user to decide in mysqli_real_connect() if the connection shall become a (true) 
persistent connection or not.

Bug: http://bugs.php.net/49442 (Assigned) Some queries crash PHP with 
mysqli_real_connect()
      
Changed paths:
    A   php/php-src/branches/PHP_5_3/ext/mysqli/tests/bug49442.phpt
    U   php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd.h
    U   php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_priv.h
    U   php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_wireprotocol.c
    A   php/php-src/trunk/ext/mysqli/tests/bug49442.phpt
    U   php/php-src/trunk/ext/mysqlnd/mysqlnd.h
    U   php/php-src/trunk/ext/mysqlnd/mysqlnd_priv.h
    U   php/php-src/trunk/ext/mysqlnd/mysqlnd_wireprotocol.c

Added: php/php-src/branches/PHP_5_3/ext/mysqli/tests/bug49442.phpt
===================================================================
--- php/php-src/branches/PHP_5_3/ext/mysqli/tests/bug49442.phpt	                        (rev 0)
+++ php/php-src/branches/PHP_5_3/ext/mysqli/tests/bug49442.phpt	2009-09-16 17:03:44 UTC (rev 288379)
@@ -0,0 +1,119 @@
+--TEST--
+Bug #49422 (mysqlnd: mysqli_real_connect() and LOAD DATA INFILE crash)
+--SKIPIF--
+<?php
+require_once('skipif.inc');
+require_once('skipifconnectfailure.inc');
+?>
+--INI--
+mysqli.allow_local_infile=1
+mysqli.allow_persistent=1
+mysqli.max_persistent=1
+--FILE--
+<?php
+	include ("connect.inc");
+
+	$link = mysqli_init();
+	if (!mysqli_real_connect($link, $host, $user, $passwd, $db, $port, $socket)) {
+		printf("[001] Connect failed, [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error());
+	}
+
+	if (!mysqli_query($link, 'DROP TABLE IF EXISTS test')) {
+		printf("[002] Failed to drop old test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link));
+	}
+
+	if (!mysqli_query($link, 'CREATE TABLE test(id INT, label CHAR(1), PRIMARY KEY(id)) ENGINE=' . $engine)) {
+		printf("[003] Failed to create test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link));
+	}
+
+	include("local_infile_tools.inc");
+	$file = create_standard_csv(4);
+
+	if (!...@mysqli_query($link, sprintf("LOAD DATA LOCAL INFILE '%s'
+			INTO TABLE test
+			FIELDS TERMINATED BY ';' OPTIONALLY ENCLOSED BY '\''
+			LINES TERMINATED BY '\n'",
+			mysqli_real_escape_string($link, $file)))) {
+			printf("[005] [%d] %s\n",  mysqli_errno($link), mysqli_error($link));
+	}
+
+	if (!$res = mysqli_query($link, "SELECT * FROM test ORDER BY id"))
+		printf("[006] [%d] %s\n",  mysqli_errno($link), mysqli_error($link));
+
+	$rows = array();
+	while ($row = mysqli_fetch_assoc($res)) {
+		var_dump($row);
+		$rows[] = $row;
+	}
+
+	mysqli_free_result($res);
+
+	mysqli_query($link, "DELETE FROM test");
+	mysqli_close($link);
+
+	if ($IS_MYSQLND) {
+		/*
+			mysqlnd makes a connection created through mysql_init()/mysqli_real_connect() always a 'persistent' one.
+			At this point 'persistent' is not to be confused with what a user calls a 'persistent' - in this case
+			'persistent' means that mysqlnd uses malloc() instead of emalloc(). nothing else. ext/mysqli will
+			not consider it as a 'persistent' connection in a user sense, ext/mysqli will not appy max_persistent etc.
+			Its only about malloc() vs. emalloc().
+
+			However, the bug is about malloc() and efree(). You can make make mysqlnd use malloc() by either using
+			pconnect or mysql_init() - so we should test pconnect as well..
+		*/
+		$host = 'p:' . $host;
+		if (!$link = mysqli_connect($host, $user, $passwd, $db, $port, $socket)) {
+			printf("[007] Connect failed, [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error());
+		}
+
+		/* bug happened during query processing */
+		if (!...@mysqli_query($link, sprintf("LOAD DATA LOCAL INFILE '%s'
+			INTO TABLE test
+			FIELDS TERMINATED BY ';' OPTIONALLY ENCLOSED BY '\''
+			LINES TERMINATED BY '\n'",
+			mysqli_real_escape_string($link, $file)))) {
+			printf("[008] [%d] %s\n",  mysqli_errno($link), mysqli_error($link));
+		}
+
+		/* we survived? that's good enough... */
+
+		if (!$res = mysqli_query($link, "SELECT * FROM test ORDER BY id"))
+			printf("[009] [%d] %s\n",  mysqli_errno($link), mysqli_error($link));
+
+		$i = 0;
+		while ($row = mysqli_fetch_assoc($res)) {
+			if (($row['id'] != $rows[$i]['id']) || ($row['label'] != $rows[$i]['label'])) {
+				printf("[010] Wrong values, check manually!\n");
+			}
+			$i++;
+		}
+		mysqli_close($link);
+	}
+
+	print "done!";
+?>
+--CLEAN--
+<?php
+	require_once("clean_table.inc");
+?>
+--EXPECTF--
+array(2) {
+  [%u|b%"id"]=>
+  %unicode|string%(2) "97"
+  [%u|b%"label"]=>
+  %unicode|string%(1) "x"
+}
+array(2) {
+  [%u|b%"id"]=>
+  %unicode|string%(2) "98"
+  [%u|b%"label"]=>
+  %unicode|string%(1) "y"
+}
+array(2) {
+  [%u|b%"id"]=>
+  %unicode|string%(2) "99"
+  [%u|b%"label"]=>
+  %unicode|string%(1) "z"
+}
+done!
\ No newline at end of file

Modified: php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd.h
===================================================================
--- php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd.h	2009-09-16 15:00:54 UTC (rev 288378)
+++ php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd.h	2009-09-16 17:03:44 UTC (rev 288379)
@@ -26,7 +26,7 @@
 #define MYSQLND_VERSION_ID 50005

 /* This forces inlining of some accessor functions */
-#define MYSQLND_USE_OPTIMISATIONS 1
+#define MYSQLND_USE_OPTIMISATIONS 0

 #define MYSQLND_STRING_TO_INT_CONVERSION
 /*

Modified: php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_priv.h
===================================================================
--- php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_priv.h	2009-09-16 15:00:54 UTC (rev 288378)
+++ php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_priv.h	2009-09-16 17:03:44 UTC (rev 288379)
@@ -104,10 +104,12 @@
 		if ((buf)) { \
 			pefree((buf), (persistent)); \
 		} \
-		(buf) = (message); \
+		if ((message)) { \
+			(buf) = pestrndup((message), (len), (persistent)); \
+		} else { \
+			buf = NULL; \
+		} \
 		(buf_len) = (len); \
-		/* Transfer ownership*/ \
-		(message) = NULL; \
 	}

 #define SET_EMPTY_MESSAGE(buf, buf_len, persistent) \

Modified: php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_wireprotocol.c
===================================================================
--- php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_wireprotocol.c	2009-09-16 15:00:54 UTC (rev 288378)
+++ php/php-src/branches/PHP_5_3/ext/mysqlnd/mysqlnd_wireprotocol.c	2009-09-16 17:03:44 UTC (rev 288379)
@@ -813,7 +813,7 @@

 	/* There is a message */
 	if (packet->header.size > p - buf && (i = php_mysqlnd_net_field_length(&p))) {
-		packet->message = pestrndup((char *)p, MIN(i, sizeof(buf) - (p - buf)), conn->persistent);
+		packet->message = estrndup((char *)p, MIN(i, sizeof(buf) - (p - buf)));
 		packet->message_len = i;
 	} else {
 		packet->message = NULL;
@@ -1032,7 +1032,7 @@
 			  Thus, the name is size - 1. And we add 1 for a trailing \0.
 			*/
 			len = packet->header.size - 1;
-			packet->info_or_local_file = mnd_pemalloc(len + 1, conn->persistent);
+			packet->info_or_local_file = mnd_emalloc(len + 1);
 			memcpy(packet->info_or_local_file, p, len);
 			packet->info_or_local_file[len] = '\0';
 			packet->info_or_local_file_len = len;
@@ -1867,7 +1867,7 @@

 	PACKET_READ_HEADER_AND_BODY(packet, conn, buf, sizeof(buf), "statistics", PROT_STATS_PACKET);

-	packet->message = mnd_pemalloc(packet->header.size + 1, conn->persistent);
+	packet->message = mnd_emalloc(packet->header.size + 1);
 	memcpy(packet->message, buf, packet->header.size);
 	packet->message[packet->header.size] = '\0';
 	packet->message_len = packet->header.size;

Added: php/php-src/trunk/ext/mysqli/tests/bug49442.phpt
===================================================================
--- php/php-src/trunk/ext/mysqli/tests/bug49442.phpt	                        (rev 0)
+++ php/php-src/trunk/ext/mysqli/tests/bug49442.phpt	2009-09-16 17:03:44 UTC (rev 288379)
@@ -0,0 +1,119 @@
+--TEST--
+Bug #49422 (mysqlnd: mysqli_real_connect() and LOAD DATA INFILE crash)
+--SKIPIF--
+<?php
+require_once('skipif.inc');
+require_once('skipifconnectfailure.inc');
+?>
+--INI--
+mysqli.allow_local_infile=1
+mysqli.allow_persistent=1
+mysqli.max_persistent=1
+--FILE--
+<?php
+	include ("connect.inc");
+
+	$link = mysqli_init();
+	if (!mysqli_real_connect($link, $host, $user, $passwd, $db, $port, $socket)) {
+		printf("[001] Connect failed, [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error());
+	}
+
+	if (!mysqli_query($link, 'DROP TABLE IF EXISTS test')) {
+		printf("[002] Failed to drop old test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link));
+	}
+
+	if (!mysqli_query($link, 'CREATE TABLE test(id INT, label CHAR(1), PRIMARY KEY(id)) ENGINE=' . $engine)) {
+		printf("[003] Failed to create test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link));
+	}
+
+	include("local_infile_tools.inc");
+	$file = create_standard_csv(4);
+
+	if (!...@mysqli_query($link, sprintf("LOAD DATA LOCAL INFILE '%s'
+			INTO TABLE test
+			FIELDS TERMINATED BY ';' OPTIONALLY ENCLOSED BY '\''
+			LINES TERMINATED BY '\n'",
+			mysqli_real_escape_string($link, $file)))) {
+			printf("[005] [%d] %s\n",  mysqli_errno($link), mysqli_error($link));
+	}
+
+	if (!$res = mysqli_query($link, "SELECT * FROM test ORDER BY id"))
+		printf("[006] [%d] %s\n",  mysqli_errno($link), mysqli_error($link));
+
+	$rows = array();
+	while ($row = mysqli_fetch_assoc($res)) {
+		var_dump($row);
+		$rows[] = $row;
+	}
+
+	mysqli_free_result($res);
+
+	mysqli_query($link, "DELETE FROM test");
+	mysqli_close($link);
+
+	if ($IS_MYSQLND) {
+		/*
+			mysqlnd makes a connection created through mysql_init()/mysqli_real_connect() always a 'persistent' one.
+			At this point 'persistent' is not to be confused with what a user calls a 'persistent' - in this case
+			'persistent' means that mysqlnd uses malloc() instead of emalloc(). nothing else. ext/mysqli will
+			not consider it as a 'persistent' connection in a user sense, ext/mysqli will not appy max_persistent etc.
+			Its only about malloc() vs. emalloc().
+
+			However, the bug is about malloc() and efree(). You can make make mysqlnd use malloc() by either using
+			pconnect or mysql_init() - so we should test pconnect as well..
+		*/
+		$host = 'p:' . $host;
+		if (!$link = mysqli_connect($host, $user, $passwd, $db, $port, $socket)) {
+			printf("[007] Connect failed, [%d] %s\n", mysqli_connect_errno(), mysqli_connect_error());
+		}
+
+		/* bug happened during query processing */
+		if (!...@mysqli_query($link, sprintf("LOAD DATA LOCAL INFILE '%s'
+			INTO TABLE test
+			FIELDS TERMINATED BY ';' OPTIONALLY ENCLOSED BY '\''
+			LINES TERMINATED BY '\n'",
+			mysqli_real_escape_string($link, $file)))) {
+			printf("[008] [%d] %s\n",  mysqli_errno($link), mysqli_error($link));
+		}
+
+		/* we survived? that's good enough... */
+
+		if (!$res = mysqli_query($link, "SELECT * FROM test ORDER BY id"))
+			printf("[009] [%d] %s\n",  mysqli_errno($link), mysqli_error($link));
+
+		$i = 0;
+		while ($row = mysqli_fetch_assoc($res)) {
+			if (($row['id'] != $rows[$i]['id']) || ($row['label'] != $rows[$i]['label'])) {
+				printf("[010] Wrong values, check manually!\n");
+			}
+			$i++;
+		}
+		mysqli_close($link);
+	}
+
+	print "done!";
+?>
+--CLEAN--
+<?php
+	require_once("clean_table.inc");
+?>
+--EXPECTF--
+array(2) {
+  [%u|b%"id"]=>
+  %unicode|string%(2) "97"
+  [%u|b%"label"]=>
+  %unicode|string%(1) "x"
+}
+array(2) {
+  [%u|b%"id"]=>
+  %unicode|string%(2) "98"
+  [%u|b%"label"]=>
+  %unicode|string%(1) "y"
+}
+array(2) {
+  [%u|b%"id"]=>
+  %unicode|string%(2) "99"
+  [%u|b%"label"]=>
+  %unicode|string%(1) "z"
+}
+done!
\ No newline at end of file

Modified: php/php-src/trunk/ext/mysqlnd/mysqlnd.h
===================================================================
--- php/php-src/trunk/ext/mysqlnd/mysqlnd.h	2009-09-16 15:00:54 UTC (rev 288378)
+++ php/php-src/trunk/ext/mysqlnd/mysqlnd.h	2009-09-16 17:03:44 UTC (rev 288379)
@@ -26,7 +26,7 @@
 #define MYSQLND_VERSION_ID 50005

 /* This forces inlining of some accessor functions */
-#define MYSQLND_USE_OPTIMISATIONS 1
+#define MYSQLND_USE_OPTIMISATIONS 0

 #define MYSQLND_STRING_TO_INT_CONVERSION
 /*

Modified: php/php-src/trunk/ext/mysqlnd/mysqlnd_priv.h
===================================================================
--- php/php-src/trunk/ext/mysqlnd/mysqlnd_priv.h	2009-09-16 15:00:54 UTC (rev 288378)
+++ php/php-src/trunk/ext/mysqlnd/mysqlnd_priv.h	2009-09-16 17:03:44 UTC (rev 288379)
@@ -104,10 +104,12 @@
 		if ((buf)) { \
 			pefree((buf), (persistent)); \
 		} \
-		(buf) = (message); \
+		if ((message)) { \
+			(buf) = pestrndup((message), (len), (persistent)); \
+		} else { \
+			buf = NULL; \
+		} \
 		(buf_len) = (len); \
-		/* Transfer ownership*/ \
-		(message) = NULL; \
 	}

 #define SET_EMPTY_MESSAGE(buf, buf_len, persistent) \

Modified: php/php-src/trunk/ext/mysqlnd/mysqlnd_wireprotocol.c
===================================================================
--- php/php-src/trunk/ext/mysqlnd/mysqlnd_wireprotocol.c	2009-09-16 15:00:54 UTC (rev 288378)
+++ php/php-src/trunk/ext/mysqlnd/mysqlnd_wireprotocol.c	2009-09-16 17:03:44 UTC (rev 288379)
@@ -813,7 +813,7 @@

 	/* There is a message */
 	if (packet->header.size > p - buf && (i = php_mysqlnd_net_field_length(&p))) {
-		packet->message = pestrndup((char *)p, MIN(i, sizeof(buf) - (p - buf)), conn->persistent);
+		packet->message = estrndup((char *)p, MIN(i, sizeof(buf) - (p - buf)));
 		packet->message_len = i;
 	} else {
 		packet->message = NULL;
@@ -1032,7 +1032,7 @@
 			  Thus, the name is size - 1. And we add 1 for a trailing \0.
 			*/
 			len = packet->header.size - 1;
-			packet->info_or_local_file = mnd_pemalloc(len + 1, conn->persistent);
+			packet->info_or_local_file = mnd_emalloc(len + 1);
 			memcpy(packet->info_or_local_file, p, len);
 			packet->info_or_local_file[len] = '\0';
 			packet->info_or_local_file_len = len;
@@ -1867,7 +1867,7 @@

 	PACKET_READ_HEADER_AND_BODY(packet, conn, buf, sizeof(buf), "statistics", PROT_STATS_PACKET);

-	packet->message = mnd_pemalloc(packet->header.size + 1, conn->persistent);
+	packet->message = mnd_emalloc(packet->header.size + 1);
 	memcpy(packet->message, buf, packet->header.size);
 	packet->message[packet->header.size] = '\0';
 	packet->message_len = packet->header.size;
-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to