Bug#315547: [patch] stop fd leak in libnss-ldap
i believe these 3 bugs are the same problem.
when the ldap server closes the connection during a response, libnss-ldap
doesn't really notice at all... it returns an error code to the caller but
doesn't pay attention to the error code itself. then nscd (or whatever
other caller is involved) tries to re-open a new connection and
libnss-ldap decides that someone has stolen its old socket and proceeds to
leak the socket.
the do_get_our_socket portion of the patch below fixes the leak.
the do_drop_connection portion of this patch which is not technically
required to fix the leak -- it fixes another bug: libnss-ldap is totally
broken in multithreaded programs (such as nscd) because you can't do
"close(10); dup2(14,10);" and guarantee another thread didn't re-open fd
10 in the meanwhile. the patch as included fixes this problem but only
when non-ssl connections are in use... in the case ssl connections are in
use it's just totally broken and can't be fixed. yay. (however thanks to
fixing the do_get_our_socket code the drop code is rarely called in the
dangerous manner.)
-dean
Index: libnss-ldap-250/ldap-nss.c
===================================================================
--- libnss-ldap-250.orig/ldap-nss.c 2006-04-26 18:19:00.000000000 -0700
+++ libnss-ldap-250/ldap-nss.c 2007-01-08 21:40:41.000000000 -0800
@@ -793,23 +793,31 @@ do_get_our_socket(int *sd)
NSS_LDAP_SOCKLEN_T peernamelen = sizeof (peername);
if (getsockname (*sd, (struct sockaddr *) &sockname, &socknamelen) != 0 ||
- getpeername (*sd, (struct sockaddr *) &peername, &peernamelen) != 0)
+ !do_sockaddr_isequal (&__session.ls_sockname,
+ socknamelen,
+ &sockname,
+ socknamelen))
+ {
+ isOurSocket = 0;
+ }
+ /*
+ * XXX: We don't pay any attention to return codes in places such as
+ * do_search_s so we never observe when the other end has disconnected
+ * our socket. In that case we'll get an ENOTCONN error here... and
+ * it's best we ignore the error -- otherwise we'll leak a filedescriptor.
+ * The correct fix would be to test error codes in many places.
+ */
+ else if (getpeername (*sd, (struct sockaddr *) &peername, &peernamelen) != 0)
{
- isOurSocket = 0;
+ if (errno != ENOTCONN)
+ isOurSocket = 0;
}
else
{
- isOurSocket = do_sockaddr_isequal (&__session.ls_sockname,
- socknamelen,
- &sockname,
- socknamelen);
- if (isOurSocket)
- {
- isOurSocket = do_sockaddr_isequal (&__session.ls_peername,
- peernamelen,
- &peername,
- peernamelen);
- }
+ isOurSocket = do_sockaddr_isequal (&__session.ls_peername,
+ peernamelen,
+ &peername,
+ peernamelen);
}
}
#endif /* HAVE_LDAPSSL_CLIENT_INIT */
@@ -876,13 +884,16 @@ do_drop_connection(int sd, int closeSd)
dummyfd = socket (AF_INET, SOCK_STREAM, 0);
if (dummyfd > -1 && dummyfd != sd)
{
- do_closefd (sd);
+ /* we must let dup2 close sd for us to avoid race conditions
+ * in multithreaded code.
+ */
do_dupfd (dummyfd, sd);
do_closefd (dummyfd);
}
#ifdef HAVE_LDAP_LD_FREE
#if defined(LDAP_API_FEATURE_X_OPENLDAP) && (LDAP_API_VERSION > 2000)
+ /* XXX: when using openssl this will *ALWAYS* close the fd */
(void) ldap_ld_free (__session.ls_conn, 0, NULL, NULL);
#else
(void) ldap_ld_free (__session.ls_conn, 0);
@@ -892,13 +903,18 @@ do_drop_connection(int sd, int closeSd)
#endif /* HAVE_LDAP_LD_FREE */
/* Do we want our original sd back? */
- do_closefd (sd);
if (savedfd > -1)
{
if (closeSd == 0)
do_dupfd (savedfd, sd);
+ else
+ do_closefd (sd);
do_closefd (savedfd);
- }
+ }
+ else
+ {
+ do_closefd (sd);
+ }
}
#else /* No sd available */
{
Reply to: