From 83b0c5d43f568222d97f9b8de985f1e39a375fb9 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Wed, 24 Sep 2008 01:37:16 -0400 Subject: [PATCH] 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. --- source4/auth/gensec/gensec.h | 1 + source4/auth/gensec/socket.c | 13 +++++++------ source4/ldap_server/ldap_bind.c | 1 + source4/libcli/ldap/ldap_bind.c | 1 + source4/libcli/ldap/ldap_client.c | 21 ++++++++++++--------- 5 files changed, 22 insertions(+), 15 deletions(-) diff --git a/source4/auth/gensec/gensec.h b/source4/auth/gensec/gensec.h index 2830297ffe4..84fc26d1271 100644 --- 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 27449bf6102..319730e2cab 100644 --- 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 8357251a8f2..20777e52619 100644 --- 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 65673116be1..b66232c02e5 100644 --- 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 844238afdb5..d7960f901ab 100644 --- 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); -- 2.11.4.GIT