Fix subtle but important bug in ip_connect(); it shouldn't close the

socket on a connection error. Also ensure that socket is closed in
iplookup.c after ip_connect() failure.

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index ddc8b47abfddfd71e10576e91989caf07cb7abf7..fcf835025a706d28085000b6987101977237fd0e 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -1,4 +1,4 @@
-$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.336 2006/04/04 08:35:39 ph10 Exp $
+$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.337 2006/04/04 09:09:44 ph10 Exp $

 
 Change log file for Exim from version 4.21
 -------------------------------------------
 
 Change log file for Exim from version 4.21
 -------------------------------------------


@@ -340,6 +340,18 @@ PH/69 The HTML version of the specification is now built in a directory called
 
 PH/70 Catch two compiler warnings in sieve.c.
 
 
 PH/70 Catch two compiler warnings in sieve.c.
 

+PH/71 Fixed an obscure and subtle bug (thanks Alexander & Matthias). The
+      function verify_get_ident() calls ip_connect() to connect a socket, but
+      if the "connect()" function timed out, ip_connect() used to close the
+      socket. However, verify_get_ident() also closes the socket later, and in
+      between Exim writes to the log, which may get opened at this point. When
+      the socket was closed in ip_connect(), the log could get the same file
+      descriptor number as the socket. This naturally causes chaos. The fix is
+      not to close the socket in ip_connect(); the socket should be closed by
+      the function that creates it. There was only one place in the code where
+      this was missing, in the iplookup router, which I don't think anybody now
+      uses, but I've fixed it anyway.
+

 
 Exim version 4.60
 -----------------
 
 Exim version 4.60
 -----------------

diff --git a/src/ACKNOWLEDGMENTS b/src/ACKNOWLEDGMENTS
index fe9ac9d35dc156d4878a54814a6222d4d0ed9c65..4fb6c922d91588435afffa07edea7eb91b74308b 100644 (file)
--- a/src/ACKNOWLEDGMENTS
+++ b/src/ACKNOWLEDGMENTS
@@ -1,4 +1,4 @@
-$Cambridge: exim/src/ACKNOWLEDGMENTS,v 1.44 2006/03/16 12:07:55 ph10 Exp $
+$Cambridge: exim/src/ACKNOWLEDGMENTS,v 1.45 2006/04/04 09:09:45 ph10 Exp $

 
 EXIM ACKNOWLEDGEMENTS
 
 
 EXIM ACKNOWLEDGEMENTS
 


@@ -20,7 +20,7 @@ relatively small patches.
 Philip Hazel
 
 Lists created: 20 November 2002
 Philip Hazel
 
 Lists created: 20 November 2002

-Last updated:  16 March 2006
+Last updated:  04 April 2006

 
 
 THE OLD LIST
 
 
 THE OLD LIST


@@ -173,6 +173,7 @@ Tom Kistner               SPA server code
                             extension (exiscan)
 Jürgen Kreileder          Fix for cyrus_sasl advertisement problem
 Friso Kuipers             Patch for GDBM problem
                             extension (exiscan)
 Jürgen Kreileder          Fix for cyrus_sasl advertisement problem
 Friso Kuipers             Patch for GDBM problem

+Matthias Lederhofer       Diagnosing and patching obscure and subtle socket bug

 Chris Liddiard            Fix for bug in exiqsumm
 Chris Lightfoot           Patch for -restore-times in exim_lock
 Edgar Lovecraft           Patch for ${str2b64:
 Chris Liddiard            Fix for bug in exiqsumm
 Chris Lightfoot           Patch for -restore-times in exim_lock
 Edgar Lovecraft           Patch for ${str2b64:


@@ -199,6 +200,7 @@ Alex Miller               Suggested readline() patch
                           Support for the DrWeb content scanner
 Andreas Mueller           Patch for logging uncompleted SMTP transactions
 Pete Naylor               Patch for LDAP TCP connect timeout setting
                           Support for the DrWeb content scanner
 Andreas Mueller           Patch for logging uncompleted SMTP transactions
 Pete Naylor               Patch for LDAP TCP connect timeout setting

+Alexander Newmann         Diagnosing and patching obscure and subtle socket bug

 Matthew Newton            Patch for exicyclog log location problem
 Marcin Owsiany            Diagnosis of a tricky timeout failure bug
 Eric Parusel              Patch for tls_remember_esmtp
 Matthew Newton            Patch for exicyclog log location problem
 Marcin Owsiany            Diagnosis of a tricky timeout failure bug
 Eric Parusel              Patch for tls_remember_esmtp

diff --git a/src/src/ip.c b/src/src/ip.c
index 0e4eab6503b4ad3f6ab9076597f562020f96d817..c15e501477792ca3a9e82b214fc69b53d591aa54 100644 (file)
--- a/src/src/ip.c
+++ b/src/src/ip.c
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/ip.c,v 1.5 2006/02/16 10:05:33 ph10 Exp $ */
+/* $Cambridge: exim/src/src/ip.c,v 1.6 2006/04/04 09:09:45 ph10 Exp $ */

 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *


@@ -165,7 +165,9 @@ return bind(sock, (struct sockaddr *)&sin, s_len);
 *************************************************/
 
 /* This function connects a socket to a remote address and port. The socket may
 *************************************************/
 
 /* This function connects a socket to a remote address and port. The socket may

-or may not have previously been bound to a local interface.
+or may not have previously been bound to a local interface. The socket is not
+closed, even in cases of error. It is expected that the calling function, which
+created the socket, will be the one that closes it.

 
 Arguments:
   sock        the socket
 
 Arguments:
   sock        the socket


@@ -243,7 +245,6 @@ if (rc >= 0) return 0;
 /* A failure whose error code is "Interrupted system call" is in fact
 an externally applied timeout if the signal handler has been run. */
 
 /* A failure whose error code is "Interrupted system call" is in fact
 an externally applied timeout if the signal handler has been run. */
 

-(void)close(sock);

 errno = (save_errno == EINTR && sigalrm_seen)? ETIMEDOUT : save_errno;
 return -1;
 }
 errno = (save_errno == EINTR && sigalrm_seen)? ETIMEDOUT : save_errno;
 return -1;
 }

diff --git a/src/src/routers/iplookup.c b/src/src/routers/iplookup.c
index 8c17c62faa3a4852d1cacbbf13489267894d5354..9dafd6fa536979afd02a90da23ec496b02638cc6 100644 (file)
--- a/src/src/routers/iplookup.c
+++ b/src/src/routers/iplookup.c
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/routers/iplookup.c,v 1.6 2006/02/07 11:19:02 ph10 Exp $ */
+/* $Cambridge: exim/src/src/routers/iplookup.c,v 1.7 2006/04/04 09:09:45 ph10 Exp $ */

 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *


@@ -240,6 +240,7 @@ while ((hostname = string_nextinlist(&listptr, &sep, host_buffer,
 
     if (ip_connect(query_socket, host_af, h->address,ob->port, ob->timeout) < 0)
       {
 
     if (ip_connect(query_socket, host_af, h->address,ob->port, ob->timeout) < 0)
       {

+      close(query_socket);

       DEBUG(D_route)
         debug_printf("connection to %s failed: %s\n", h->address,
           strerror(errno));
       DEBUG(D_route)
         debug_printf("connection to %s failed: %s\n", h->address,
           strerror(errno));