From 8c021cec14de135bc051950e4e70f6bcd03c639f Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 27 Jun 2013 11:27:03 +1000 Subject: [PATCH 1/4] TODO service_stream: Log if the connection termination is deferred or not --- source4/smbd/service_stream.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source4/smbd/service_stream.c b/source4/smbd/service_stream.c index 22c4c04..74bb477 100644 --- a/source4/smbd/service_stream.c +++ b/source4/smbd/service_stream.c @@ -60,7 +60,11 @@ void stream_terminate_connection(struct stream_connection *srv_conn, const char if (!reason) reason = "unknown reason"; - DEBUG(3,("Terminating connection - '%s'\n", reason)); + if (srv_conn->processing) { + DEBUG(3,("Terminating connection deferred - '%s'\n", reason)); + } else { + DEBUG(3,("Terminating connection - '%s'\n", reason)); + } srv_conn->terminate = reason; -- 1.7.9.5 From 7cc83acc4de163a91fa9665dfdd3cc5fcb751193 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 27 Jun 2013 11:28:03 +1000 Subject: [PATCH 2/4] TODO s4-winbindd: Do not terminate a connection that is still pending Instead, wait until the call attempts to reply, and let it terminate then (often this happens in the attempt to then write to the broken pipe). Andrew Bartlett --- source4/winbind/wb_samba3_protocol.c | 5 +++++ source4/winbind/wb_server.c | 14 +++++++++++++- source4/winbind/wb_server.h | 5 ++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/source4/winbind/wb_samba3_protocol.c b/source4/winbind/wb_samba3_protocol.c index 2846e9c..1b78c99 100644 --- a/source4/winbind/wb_samba3_protocol.c +++ b/source4/winbind/wb_samba3_protocol.c @@ -297,6 +297,8 @@ NTSTATUS wbsrv_samba3_send_reply(struct wbsrv_samba3_call *call) struct tevent_req *subreq; NTSTATUS status; + call->wbconn->pending_calls--; + status = wbsrv_samba3_push_reply(call); NT_STATUS_NOT_OK_RETURN(status); @@ -355,9 +357,12 @@ NTSTATUS wbsrv_samba3_process(struct wbsrv_samba3_call *call) return status; } + call->wbconn->pending_calls++; + status = wbsrv_samba3_handle_call(call); if (!NT_STATUS_IS_OK(status)) { + call->wbconn->pending_calls--; talloc_free(call); return status; } diff --git a/source4/winbind/wb_server.c b/source4/winbind/wb_server.c index 983f9f5..c215630 100644 --- a/source4/winbind/wb_server.c +++ b/source4/winbind/wb_server.c @@ -31,7 +31,14 @@ void wbsrv_terminate_connection(struct wbsrv_connection *wbconn, const char *reason) { - stream_terminate_connection(wbconn->conn, reason); + if (wbconn->pending_calls == 0) { + char *full_reason = talloc_asprintf(wbconn, "wbsrv: %s", reason); + stream_terminate_connection(wbconn->conn, full_reason ? full_reason : reason); + } else { + DEBUG(3,("wbsrv: terminating connection due to '%s' defered due to %d pending calls\n", + reason, wbconn->pending_calls)); + wbconn->terminate = reason; + } } static void wbsrv_call_loop(struct tevent_req *subreq) @@ -41,6 +48,11 @@ static void wbsrv_call_loop(struct tevent_req *subreq) struct wbsrv_samba3_call *call; NTSTATUS status; + if (wbsrv_conn->terminate) { + wbsrv_terminate_connection(wbsrv_conn, wbsrv_conn->terminate); + return; + } + call = talloc_zero(wbsrv_conn, struct wbsrv_samba3_call); if (call == NULL) { wbsrv_terminate_connection(wbsrv_conn, "wbsrv_call_loop: " diff --git a/source4/winbind/wb_server.h b/source4/winbind/wb_server.h index 9b03004..c03bfa1 100644 --- a/source4/winbind/wb_server.h +++ b/source4/winbind/wb_server.h @@ -94,9 +94,12 @@ struct wbsrv_connection { /* storage for protocol specific data */ void *protocol_private_data; - /* how many calls are pending */ + /* how many calls are pending (do not terminate the connection with calls pending a reply) */ uint32_t pending_calls; + /* is this connection pending termination? If so, why? */ + const char *terminate; + struct tstream_context *tstream; struct tevent_queue *send_queue; -- 1.7.9.5 From 7bd565c6774c4919e4259328e74d54da877d2610 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 9 Jul 2013 13:57:33 +0200 Subject: [PATCH 3/4] sq wbsrv_terminate_connection --- source4/winbind/wb_server.c | 45 +++++++++++++++++++++++++++++++++++++++---- source4/winbind/wb_server.h | 5 +++++ 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/source4/winbind/wb_server.c b/source4/winbind/wb_server.c index c215630..29ed5a6 100644 --- a/source4/winbind/wb_server.c +++ b/source4/winbind/wb_server.c @@ -28,16 +28,43 @@ #include "libcli/util/tstream.h" #include "param/param.h" #include "param/secrets.h" +#include "lib/util/dlinklist.h" void wbsrv_terminate_connection(struct wbsrv_connection *wbconn, const char *reason) { + struct wbsrv_service *service = wbconn->listen_socket->service; + if (wbconn->pending_calls == 0) { char *full_reason = talloc_asprintf(wbconn, "wbsrv: %s", reason); + + DLIST_REMOVE(service->broken_connections, wbconn); stream_terminate_connection(wbconn->conn, full_reason ? full_reason : reason); - } else { - DEBUG(3,("wbsrv: terminating connection due to '%s' defered due to %d pending calls\n", - reason, wbconn->pending_calls)); - wbconn->terminate = reason; + return; + } + + if (wbconn->terminate != NULL) { + return; + } + + DEBUG(3,("wbsrv: terminating connection due to '%s' defered due to %d pending calls\n", + reason, wbconn->pending_calls)); + wbconn->terminate = talloc_strdup(wbconn, reason); + if (wbconn->terminate == NULL) { + wbconn->terminate = "wbsrv: defered terminating connection - no memory"; + } + DLIST_ADD_END(service->broken_connections, wbconn, NULL); +} + +static void wbsrv_cleanup_broken_connections(struct wbsrv_service *s) +{ + struct wbsrv_connection *cur, *next; + + next = s->broken_connections; + while (next != NULL) { + cur = next; + next = cur->next; + + wbsrv_terminate_connection(cur, cur->terminate); } } @@ -45,14 +72,22 @@ static void wbsrv_call_loop(struct tevent_req *subreq) { struct wbsrv_connection *wbsrv_conn = tevent_req_callback_data(subreq, struct wbsrv_connection); + struct wbsrv_service *service = wbsrv_conn->listen_socket->service; struct wbsrv_samba3_call *call; NTSTATUS status; if (wbsrv_conn->terminate) { + /* + * if the current connection is broken + * we need to clean it up before any other connection + */ wbsrv_terminate_connection(wbsrv_conn, wbsrv_conn->terminate); + wbsrv_cleanup_broken_connections(service); return; } + wbsrv_cleanup_broken_connections(service); + call = talloc_zero(wbsrv_conn, struct wbsrv_samba3_call); if (call == NULL) { wbsrv_terminate_connection(wbsrv_conn, "wbsrv_call_loop: " @@ -124,6 +159,8 @@ static void wbsrv_accept(struct stream_connection *conn) struct tevent_req *subreq; int rc; + wbsrv_cleanup_broken_connections(wbsrv_socket->service); + wbsrv_conn = talloc_zero(conn, struct wbsrv_connection); if (wbsrv_conn == NULL) { stream_terminate_connection(conn, "wbsrv_accept: out of memory"); diff --git a/source4/winbind/wb_server.h b/source4/winbind/wb_server.h index c03bfa1..26c404d 100644 --- a/source4/winbind/wb_server.h +++ b/source4/winbind/wb_server.h @@ -34,6 +34,8 @@ struct wbsrv_service { struct idmap_context *idmap_ctx; const char *priv_pipe_dir; const char *pipe_dir; + + struct wbsrv_connection *broken_connections; }; struct wbsrv_samconn { @@ -85,6 +87,9 @@ struct wbsrv_listen_socket { state of an open winbind connection */ struct wbsrv_connection { + /* for the broken_connections DLIST */ + struct wbsrv_connection *prev, *next; + /* stream connection we belong to */ struct stream_connection *conn; -- 1.7.9.5 From 9fd26f145aaf9c876db035d9db5c916007d23861 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 9 Jul 2013 16:38:59 +0200 Subject: [PATCH 4/4] s4:rpc_server: make sure we don't terminate a connection with pending requests Sadly we may have nested event loops, which won't work correctly with broken connections, that's why we have to do this... Signed-off-by: Stefan Metzmacher --- source4/rpc_server/dcerpc_server.c | 51 +++++++++++++++++++++++++++++++++++- source4/rpc_server/dcerpc_server.h | 6 +++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/source4/rpc_server/dcerpc_server.c b/source4/rpc_server/dcerpc_server.c index 389cbe3..9965e43 100644 --- a/source4/rpc_server/dcerpc_server.c +++ b/source4/rpc_server/dcerpc_server.c @@ -403,6 +403,7 @@ _PUBLIC_ NTSTATUS dcesrv_endpoint_connect(struct dcesrv_context *dce_ctx, p->msg_ctx = msg_ctx; p->server_id = server_id; p->processing = false; + p->terminate = NULL; p->state_flags = state_flags; ZERO_STRUCT(p->transport); @@ -1143,6 +1144,7 @@ _PUBLIC_ NTSTATUS dcesrv_init_context(TALLOC_CTX *mem_ctx, dce_ctx->lp_ctx = lp_ctx; dce_ctx->assoc_groups_idr = idr_init(dce_ctx); NT_STATUS_HAVE_NO_MEMORY(dce_ctx->assoc_groups_idr); + dce_ctx->broken_connections = NULL; for (i=0;endpoint_servers[i];i++) { const struct dcesrv_endpoint_server *ep_server; @@ -1269,12 +1271,45 @@ const struct dcesrv_critical_sizes *dcerpc_module_version(void) static void dcesrv_terminate_connection(struct dcesrv_connection *dce_conn, const char *reason) { + struct dcesrv_context *dce_ctx = dce_conn->dce_ctx; struct stream_connection *srv_conn; srv_conn = talloc_get_type(dce_conn->transport.private_data, struct stream_connection); - stream_terminate_connection(srv_conn, reason); + if (dce_conn->pending_call_list == NULL) { + char *full_reason = talloc_asprintf(dce_conn, "dcesrv: %s", reason); + + DLIST_REMOVE(dce_ctx->broken_connections, dce_conn); + stream_terminate_connection(srv_conn, full_reason ? full_reason : reason); + return; + } + + if (dce_conn->terminate != NULL) { + return; + } + + DEBUG(3,("dcesrv: terminating connection due to '%s' defered due to pending calls\n", + reason)); + dce_conn->terminate = talloc_strdup(dce_conn, reason); + if (dce_conn->terminate == NULL) { + dce_conn->terminate = "dcesrv: defered terminating connection - no memory"; + } + DLIST_ADD_END(dce_ctx->broken_connections, dce_conn, NULL); } + +static void dcesrv_cleanup_broken_connections(struct dcesrv_context *dce_ctx) +{ + struct dcesrv_connection *cur, *next; + + next = dce_ctx->broken_connections; + while (next != NULL) { + cur = next; + next = cur->next; + + dcesrv_terminate_connection(cur, cur->terminate); + } +} + /* We need this include to be able to compile on some plateforms * (ie. freebsd 7.2) as it seems that is not included * correctly. @@ -1386,6 +1421,8 @@ static void dcesrv_sock_accept(struct stream_connection *srv_conn) struct tevent_req *subreq; struct loadparm_context *lp_ctx = dcesrv_sock->dcesrv_ctx->lp_ctx; + dcesrv_cleanup_broken_connections(dcesrv_sock->dcesrv_ctx); + if (!srv_conn->session_info) { status = auth_anonymous_session_info(srv_conn, lp_ctx, @@ -1477,6 +1514,18 @@ static void dcesrv_read_fragment_done(struct tevent_req *subreq) DATA_BLOB buffer; NTSTATUS status; + if (dce_conn->terminate) { + /* + * if the current connection is broken + * we need to clean it up before any other connection + */ + dcesrv_terminate_connection(dce_conn, dce_conn->terminate); + dcesrv_cleanup_broken_connections(dce_conn->dce_ctx); + return; + } + + dcesrv_cleanup_broken_connections(dce_conn->dce_ctx); + status = dcerpc_read_ncacn_packet_recv(subreq, dce_conn, &pkt, &buffer); TALLOC_FREE(subreq); diff --git a/source4/rpc_server/dcerpc_server.h b/source4/rpc_server/dcerpc_server.h index 4fcb5c5..501cdbe 100644 --- a/source4/rpc_server/dcerpc_server.h +++ b/source4/rpc_server/dcerpc_server.h @@ -170,6 +170,9 @@ struct dcesrv_connection_context { /* the state associated with a dcerpc server connection */ struct dcesrv_connection { + /* for the broken_connections DLIST */ + struct dcesrv_connection *prev, *next; + /* the top level context for this server */ struct dcesrv_context *dce_ctx; @@ -209,6 +212,7 @@ struct dcesrv_connection { DATA_BLOB transport_session_key; bool processing; + const char *terminate; const char *packet_log_dir; @@ -288,6 +292,8 @@ struct dcesrv_context { struct loadparm_context *lp_ctx; struct idr_context *assoc_groups_idr; + + struct dcesrv_connection *broken_connections; }; /* this structure is used by modules to determine the size of some critical types */ -- 1.7.9.5