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 */
   {


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to