Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:13319
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Wed, 13 Oct 2004 15:50:28 +0200
To: PHP Developers Mailing List <internals@lists.php.net>
Message-ID: <20041013135028.GA29756@deejai2.mch.fsc.net>
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="2fHTh5uZTiUOsy+g"
Content-Disposition: inline
User-Agent: Mutt/1.4.2.1i
Subject: [PHP-DEV] [PATCH PHP_4_3] Core dumps in cyrus
From: Martin.Kraemer@Fujitsu-Siemens.com (Martin Kraemer)

--2fHTh5uZTiUOsy+g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

In the cyrus_connect() php function, the Cyrus function
imclient_connect() is called, and its return value is checked for
0, -1 or -2. There is a problem with that however:

a) imclient_connect() returns in fact 0, -1, -2 OR the value of
   errno (if something outside of imclient_connect()'s scope failed).
   This can and will happen if you pass it a valid hostname where
   no IMAP server is listening.
   From the man page:
     imclient_connect()
          Connects the client server to the host. If successful,
          it returns 0 and sets the imclient argument to a
          pointer to an imclient struct. The imclient struct
          represents the current connection,  flags, and
          callbacks. On failure, the current errno is returned if
---------------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
          a system call failed,  -1 is returned if the host name
          was not found, and  -2 is returned if the service name
          was not found.

b) conn can be uninitialized if the return value is not 0, -1 or -2,
   but it WILL be used in ZEND_REGISTER_RESOURCE, and will be
   dereferenced at the end of PHP_FUNCTION(cyrus_connect).

Find a patch which tries to be more conservative about pointers, plus
it checks the errno-branch of imclient_connect()'s return.

   Martin
-- 
<Martin.Kraemer@Fujitsu-Siemens.com>         |     Fujitsu Siemens
Fon: +49-89-636-46021, FAX: +49-89-636-47655 | 81730  Munich,  Germany

--2fHTh5uZTiUOsy+g
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="cyrus.c.diff"

Index: ext/cyrus/cyrus.c
===================================================================
RCS file: /repository/php-src/ext/cyrus/Attic/cyrus.c,v
retrieving revision 1.12.2.3
diff -u -r1.12.2.3 cyrus.c
--- ext/cyrus/cyrus.c	13 Jun 2003 14:40:01 -0000	1.12.2.3
+++ ext/cyrus/cyrus.c	13 Oct 2004 13:39:48 -0000
@@ -68,16 +68,16 @@
 {
 	php_cyrus *conn = (php_cyrus *) rsrc->ptr;
 
-	if (conn->client)
+	if (conn && conn->client)
 		imclient_close(conn->client);
 
-	if (conn->host)
+	if (conn && conn->host)
 		efree(conn->host);
 
-	if (conn->port)
+	if (conn && conn->port)
 		efree(conn->port);
 
-	efree(conn);
+	if (conn) efree(conn);
 }
 
 PHP_MINIT_FUNCTION(cyrus)
@@ -118,8 +118,8 @@
 	zval            **z_host;
 	zval            **z_port;
 	zval            **z_flags;
-	php_cyrus        *conn;
-	struct imclient  *client;
+	php_cyrus        *conn = NULL;
+	struct imclient  *client = NULL;
 	char             *host;
 	char             *port = NULL;
 	int               flags = 0;
@@ -173,9 +173,14 @@
 	case -2:
 		php_error(E_WARNING, "%s(): Invalid port: %d", get_active_function_name(TSRMLS_C), port);
 		RETURN_FALSE;
+
+	default:
+		php_error(E_WARNING, "%s(): %s", get_active_function_name(TSRMLS_C), strerror(errno));
+		RETURN_FALSE;
 	}
 
 	ZEND_REGISTER_RESOURCE(return_value, conn, le_cyrus);
+	if (conn)
 	conn->id = Z_LVAL_P(return_value);
 }
 /* }}} */

--2fHTh5uZTiUOsy+g--