From d515a9174a5ea517bc3d27bc4d40223b24d7a47f Mon Sep 17 00:00:00 2001 From: Philip Hazel Date: Tue, 4 Apr 2006 09:09:44 +0000 Subject: [PATCH] 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. --- doc/doc-txt/ChangeLog | 14 +++++++++++++- src/ACKNOWLEDGMENTS | 6 ++++-- src/src/ip.c | 7 ++++--- src/src/routers/iplookup.c | 3 ++- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index ddc8b47ab..fcf835025 100644 --- 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 ------------------------------------------- @@ -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/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 ----------------- diff --git a/src/ACKNOWLEDGMENTS b/src/ACKNOWLEDGMENTS index fe9ac9d35..4fb6c922d 100644 --- 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 @@ -20,7 +20,7 @@ relatively small patches. Philip Hazel Lists created: 20 November 2002 -Last updated: 16 March 2006 +Last updated: 04 April 2006 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 +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: @@ -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 +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 diff --git a/src/src/ip.c b/src/src/ip.c index 0e4eab650..c15e50147 100644 --- 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 * @@ -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 -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 @@ -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. */ -(void)close(sock); 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 8c17c62fa..9dafd6fa5 100644 --- 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 * @@ -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) { + close(query_socket); DEBUG(D_route) debug_printf("connection to %s failed: %s\n", h->address, strerror(errno)); -- 2.30.2