Attachment 298449 Details for Bug 428837 – This looks like it will address this bug...have not verified yet

[patch] This looks like it will address this bug...have not verified yet

nss_ldap-leaky-fds.patch (text/plain), 4.02 KB, created by Jack Neely on 2008-03-18 20:19:01 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jack Neely

Created: 2008-03-18 20:19:01 UTC

Size: 4.02 KB

patch

obsolete

>From http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=401758
>
>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 */
>   {
>
>
>

Actions: View | Diff

Attachments on bug 428837: 291718 | 298449