Date: Fri, 26 Aug 2011 07:25:31 -0000 (UTC)
From: Juha ErkkilÃ¤ <juha.erkkila@opinsys.fi>
To: krb5-bugs@mit.edu
Subject: TCP connection leak with 1.9.1, with connect_to_server()
Message-ID: <b7940fff-e7e7-4771-a64f-f6b12f60b500@mail>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
Precedence: list
Sender: krb5-bugs-incoming-bounces@PCH.mit.edu
Errors-To: krb5-bugs-incoming-bounces@PCH.mit.edu
Content-Length: 4299

TCP connection leak with 1.9.1, with connect_to_server()

Hi,

It seems I have run into a problem with MIT Kerberos version 1.9.1,
that did not occur in some previous versions.  The addition of
connect_to_server() in src/lib/kadm5/clnt/client_init.c appears to cause
TCP socket leak.  For every new connection, connect_to_server() is used
and it provides the socket to clnttcp_create(), but clnttcp_*-functions
leave the responsibility of closing the socket to the layer that created
the socket.  Thus, kadm5_destroy() and clnt_destroy() will not close
the socket created in connect_to_server().

If I understand the API correctly, calling:

kadm5_init_krb5_context(&context)
kadm5_init_with_skey(context, ..., &kadm5_handle)
  ...
kadm5_destroy(kadm5_handle)

should not produce such a leak.

Here's a patch that fixes the problem by making the clnt_destroy()
function take care of closing the socket.  I don't know if this
is a proper way to solve the issue, though.

Juha

diff -ruN krb5-1.9.1+dfsg.debpatched/src/include/gssrpc/clnt.h krb5-1.9.1+dfsg/src/include/gssrpc/clnt.h
--- krb5-1.9.1+dfsg.debpatched/src/include/gssrpc/clnt.h        2011-06-02 16:24:25.000000000 +0300
+++ krb5-1.9.1+dfsg/src/include/gssrpc/clnt.h   2011-08-25 17:04:46.000000000 +0300
@@ -273,9 +273,10 @@
  *     register int *sockp;
  *     u_int sendsz;
  *     u_int recvsz;
+ *     int always_closesocket;
  */
 extern CLIENT *clnttcp_create(struct sockaddr_in *, rpcprog_t, rpcvers_t,
-                             int *, u_int, u_int);
+                             int *, u_int, u_int, int);

 /*
  * UDP based rpc.
diff -ruN krb5-1.9.1+dfsg.debpatched/src/lib/kadm5/clnt/client_init.c krb5-1.9.1+dfsg/src/lib/kadm5/clnt/client_init.c
--- krb5-1.9.1+dfsg.debpatched/src/lib/kadm5/clnt/client_init.c 2011-06-02 16:24:25.000000000 +0300
+++ krb5-1.9.1+dfsg/src/lib/kadm5/clnt/client_init.c    2011-08-25 17:04:46.000000000 +0300
@@ -293,7 +293,7 @@
     if (code)
         goto error;

-    handle->clnt = clnttcp_create(NULL, rpc_prog, rpc_vers, &fd, 0, 0);
+    handle->clnt = clnttcp_create(NULL, rpc_prog, rpc_vers, &fd, 0, 0, 1);
     if (handle->clnt == NULL) {
         code = KADM5_RPC_ERROR;
 #ifdef DEBUG
diff -ruN krb5-1.9.1+dfsg.debpatched/src/lib/rpc/clnt_generic.c krb5-1.9.1+dfsg/src/lib/rpc/clnt_generic.c
--- krb5-1.9.1+dfsg.debpatched/src/lib/rpc/clnt_generic.c       2011-06-02 16:24:25.000000000 +0300
+++ krb5-1.9.1+dfsg/src/lib/rpc/clnt_generic.c  2011-08-25 17:04:46.000000000 +0300
@@ -101,7 +101,7 @@
                clnt_control(client, CLSET_TIMEOUT, &tv);
                break;
        case IPPROTO_TCP:
-               client = clnttcp_create(&sockin, prog, vers, &sock, 0, 0);
+               client = clnttcp_create(&sockin, prog, vers, &sock, 0, 0, 0);
                if (client == NULL) {
                        return (NULL);
                }
diff -ruN krb5-1.9.1+dfsg.debpatched/src/lib/rpc/clnt_tcp.c krb5-1.9.1+dfsg/src/lib/rpc/clnt_tcp.c
--- krb5-1.9.1+dfsg.debpatched/src/lib/rpc/clnt_tcp.c   2011-06-02 16:24:25.000000000 +0300
+++ krb5-1.9.1+dfsg/src/lib/rpc/clnt_tcp.c      2011-08-25 17:04:46.000000000 +0300
@@ -127,7 +127,8 @@
        rpcvers_t vers,
         SOCKET *sockp,
        u_int sendsz,
-       u_int recvsz)
+       u_int recvsz,
+       int always_closesocket)
 {
        CLIENT *h;
        register struct ct_data *ct = 0;
@@ -178,7 +179,7 @@
                }
                ct->ct_closeit = TRUE;
        } else {
-               ct->ct_closeit = FALSE;
+               ct->ct_closeit = always_closesocket ? TRUE : FALSE;
        }

        /*
diff -ruN krb5-1.9.1+dfsg.debpatched/src/lib/rpc/pmap_getmaps.c krb5-1.9.1+dfsg/src/lib/rpc/pmap_getmaps.c
--- krb5-1.9.1+dfsg.debpatched/src/lib/rpc/pmap_getmaps.c       2011-06-02 16:24:25.000000000 +0300
+++ krb5-1.9.1+dfsg/src/lib/rpc/pmap_getmaps.c  2011-08-25 17:04:46.000000000 +0300
@@ -75,7 +75,7 @@
        minutetimeout.tv_usec = 0;
        address->sin_port = htons(PMAPPORT);
        client = clnttcp_create(address, PMAPPROG,
-           PMAPVERS, &sock, 50, 500);
+           PMAPVERS, &sock, 50, 500, 0);
        if (client != (CLIENT *)NULL) {
                if (CLNT_CALL(client, PMAPPROC_DUMP, xdr_void, NULL, xdr_pmaplist,
                    &head, minutetimeout) != RPC_SUCCESS) {