Fix nasty bug that would come up only if a client connection to a remote

ldap server suddenly dies.
We were creating a wrong talloc hierarchy, so the event.fde was not
freed automatically as expected. This in turn made the event system call
the ldap io handlers with a null packet structure, causing a segfault.
Fix also the ordering in ldap_connection_dead()
Thanks to Metze for the huge help in tracking down this one.

diff --git a/source4/auth/gensec/gensec.h b/source4/auth/gensec/gensec.h
index 2830297ffe4043fc31ae661aac1f5d90b27279c0..84fc26d1271fc778a62efb96b79cba91d55a3125 100644 (file)
--- a/source4/auth/gensec/gensec.h
+++ b/source4/auth/gensec/gensec.h
@@ -174,6 +174,7 @@ struct gensec_security;
 struct socket_context;
 
 NTSTATUS gensec_socket_init(struct gensec_security *gensec_security,
+                           TALLOC_CTX *mem_ctx, 
                            struct socket_context *current_socket,
                            struct event_context *ev,
                            void (*recv_handler)(void *, uint16_t),

diff --git a/source4/auth/gensec/socket.c b/source4/auth/gensec/socket.c
index 27449bf610294b334e03fbee393c911b225092d2..319730e2cabf0ee6228253b445514ef1a1355acc 100644 (file)
--- a/source4/auth/gensec/socket.c
+++ b/source4/auth/gensec/socket.c
@@ -408,8 +408,10 @@ static NTSTATUS gensec_socket_send(struct socket_context *sock,
 }
 
 /* Turn a normal socket into a potentially GENSEC wrapped socket */
+/* CAREFUL: this function will steal 'current_socket' */
 
 NTSTATUS gensec_socket_init(struct gensec_security *gensec_security,
+                           TALLOC_CTX *mem_ctx,
                            struct socket_context *current_socket,
                            struct event_context *ev,
                            void (*recv_handler)(void *, uint16_t),
@@ -420,7 +422,7 @@ NTSTATUS gensec_socket_init(struct gensec_security *gensec_security,
        struct socket_context *new_sock;
        NTSTATUS nt_status;
 
-       nt_status = socket_create_with_ops(current_socket, &gensec_socket_ops, &new_sock, 
+       nt_status = socket_create_with_ops(mem_ctx, &gensec_socket_ops, &new_sock, 
                                           SOCKET_TYPE_STREAM, current_socket->flags | SOCKET_FLAG_ENCRYPT);
        if (!NT_STATUS_IS_OK(nt_status)) {
                *new_socket = NULL;
@@ -432,22 +434,19 @@ NTSTATUS gensec_socket_init(struct gensec_security *gensec_security,
        gensec_socket = talloc(new_sock, struct gensec_socket);
        if (gensec_socket == NULL) {
                *new_socket = NULL;
+               talloc_free(new_sock);
                return NT_STATUS_NO_MEMORY;
        }
 
        new_sock->private_data       = gensec_socket;
        gensec_socket->socket        = current_socket;
 
-       if (talloc_reference(gensec_socket, current_socket) == NULL) {
-               *new_socket = NULL;
-               return NT_STATUS_NO_MEMORY;
-       }
-
        /* Nothing to do here, if we are not actually wrapping on this socket */
        if (!gensec_have_feature(gensec_security, GENSEC_FEATURE_SEAL) &&
            !gensec_have_feature(gensec_security, GENSEC_FEATURE_SIGN)) {
                
                gensec_socket->wrap = false;
+               talloc_steal(gensec_socket, current_socket);
                *new_socket = new_sock;
                return NT_STATUS_OK;
        }
@@ -469,6 +468,7 @@ NTSTATUS gensec_socket_init(struct gensec_security *gensec_security,
        gensec_socket->packet = packet_init(gensec_socket);
        if (gensec_socket->packet == NULL) {
                *new_socket = NULL;
+               talloc_free(new_sock);
                return NT_STATUS_NO_MEMORY;
        }
 
@@ -481,6 +481,7 @@ NTSTATUS gensec_socket_init(struct gensec_security *gensec_security,
 
        /* TODO: full-request that knows about maximum packet size */
 
+       talloc_steal(gensec_socket, current_socket);
        *new_socket = new_sock;
        return NT_STATUS_OK;
 }

diff --git a/source4/ldap_server/ldap_bind.c b/source4/ldap_server/ldap_bind.c
index 8357251a8f2f3b0a49e56d89d7e6efb752000d3d..20777e526195b353586308d58af78e1185e0a40d 100644 (file)
--- a/source4/ldap_server/ldap_bind.c
+++ b/source4/ldap_server/ldap_bind.c
@@ -208,6 +208,7 @@ static NTSTATUS ldapsrv_BindSASL(struct ldapsrv_call *call)
                } else {
                        ctx->conn = conn;
                        status = gensec_socket_init(conn->gensec, 
+                                                   conn->connection,
                                                    conn->connection->socket,
                                                    conn->connection->event.ctx, 
                                                    stream_io_handler_callback,

diff --git a/source4/libcli/ldap/ldap_bind.c b/source4/libcli/ldap/ldap_bind.c
index 65673116be1232b6e8c99e6024a5dc29050cf66a..b66232c02e55356cdad32dcba285516e6e7ae5d4 100644 (file)
--- a/source4/libcli/ldap/ldap_bind.c
+++ b/source4/libcli/ldap/ldap_bind.c
@@ -387,6 +387,7 @@ _PUBLIC_ NTSTATUS ldap_bind_sasl(struct ldap_connection *conn,
        if (NT_STATUS_IS_OK(status)) {
                struct socket_context *sasl_socket;
                status = gensec_socket_init(conn->gensec, 
+                                           conn,
                                            conn->sock,
                                            conn->event.event_ctx, 
                                            ldap_read_io_handler,

diff --git a/source4/libcli/ldap/ldap_client.c b/source4/libcli/ldap/ldap_client.c
index 844238afdb51d4bd24e825f03c06c3e5e918f0b6..d7960f901ab707eddfa550eadac7f6151a546938 100644 (file)
--- a/source4/libcli/ldap/ldap_client.c
+++ b/source4/libcli/ldap/ldap_client.c
@@ -77,6 +77,12 @@ static void ldap_connection_dead(struct ldap_connection *conn)
 {
        struct ldap_request *req;
 
+       talloc_free(conn->sock);  /* this will also free event.fde */
+       talloc_free(conn->packet);
+       conn->sock = NULL;
+       conn->event.fde = NULL;
+       conn->packet = NULL;
+
        /* return an error for any pending request ... */
        while (conn->pending) {
                req = conn->pending;
@@ -87,12 +93,6 @@ static void ldap_connection_dead(struct ldap_connection *conn)
                        req->async.fn(req);
                }
        }
-
-       talloc_free(conn->sock);  /* this will also free event.fde */
-       talloc_free(conn->packet);
-       conn->sock = NULL;
-       conn->event.fde = NULL;
-       conn->packet = NULL;
 }
 
 static void ldap_reconnect(struct ldap_connection *conn);
@@ -400,6 +400,7 @@ static void ldap_connect_got_sock(struct composite_context *ctx,
        talloc_steal(conn, conn->sock);
        if (conn->ldaps) {
                struct socket_context *tls_socket;
+               struct socket_context *tmp_socket;
                char *cafile = private_path(conn->sock, conn->lp_ctx, lp_tls_cafile(conn->lp_ctx));
 
                if (!cafile || !*cafile) {
@@ -414,9 +415,11 @@ static void ldap_connect_got_sock(struct composite_context *ctx,
                        talloc_free(conn->sock);
                        return;
                }
-               talloc_unlink(conn, conn->sock);
-               conn->sock = tls_socket;
-               talloc_steal(conn, conn->sock);
+
+               /* the original socket, must become a child of the tls socket */
+               tmp_socket = conn->sock;
+               conn->sock = talloc_steal(conn, tls_socket);
+               talloc_steal(conn->sock, tmp_socket);
        }
 
        conn->packet = packet_init(conn);