From c22e3b0800b36f913bb8fb15cfea9830ddb78493 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 27 Jun 2013 11:27:03 +1000 Subject: [PATCH 1/4] service_stream: Log if the connection termination is deferred or not (bug #9820) Signed-off-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit df929d6feb857668ad9da277213e9fae1480ff63) --- 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.10.4 From 52d3a4bb440f8cf7b66db05dcd99352497b25f98 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Thu, 27 Jun 2013 11:28:03 +1000 Subject: [PATCH 2/4] s4-winbindd: Do not terminate a connection that is still pending (bug #9820) 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 Pair-Programmed-With: Stefan Metzmacher Signed-off-by: Andrew Bartlett Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett Reviewed-by: Stefan Metzmacher (cherry picked from commit 2505d48e4fbcd8a805a88ad0b05fb1a16a588197) --- source4/winbind/wb_samba3_protocol.c | 5 ++++ source4/winbind/wb_server.c | 51 +++++++++++++++++++++++++++++++++- source4/winbind/wb_server.h | 10 ++++++- 3 files changed, 64 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..29ed5a6 100644 --- a/source4/winbind/wb_server.c +++ b/source4/winbind/wb_server.c @@ -28,19 +28,66 @@ #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) { - stream_terminate_connection(wbconn->conn, 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); + 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); + } } 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: " @@ -112,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 9b03004..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; @@ -94,9 +99,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.10.4 From ee5518b0bf33d34e91a50a302d25d1c87e20ff50 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 9 Jul 2013 16:38:59 +0200 Subject: [PATCH 3/4] s4:rpc_server: make sure we don't terminate a connection with pending requests (bug #9820) 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 Reviewed-by: Andrew Bartlett Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Wed Jul 10 08:47:38 CEST 2013 on sn-devel-104 (cherry picked from commit e6a58d370403e818bc2cfb8389751b78adcc14fd) --- source4/rpc_server/dcerpc_server.c | 55 ++++++++++++++++++++++++++++++++++-- source4/rpc_server/dcerpc_server.h | 8 +++++- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/source4/rpc_server/dcerpc_server.c b/source4/rpc_server/dcerpc_server.c index 389cbe3..10e711b 100644 --- a/source4/rpc_server/dcerpc_server.c +++ b/source4/rpc_server/dcerpc_server.c @@ -386,6 +386,8 @@ _PUBLIC_ NTSTATUS dcesrv_endpoint_connect(struct dcesrv_context *dce_ctx, return NT_STATUS_NO_MEMORY; } + p->prev = NULL; + p->next = NULL; p->dce_ctx = dce_ctx; p->endpoint = ep; p->contexts = NULL; @@ -402,7 +404,7 @@ _PUBLIC_ NTSTATUS dcesrv_endpoint_connect(struct dcesrv_context *dce_ctx, p->event_ctx = event_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 +1145,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 +1272,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 +1422,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, @@ -1473,10 +1511,23 @@ static void dcesrv_read_fragment_done(struct tevent_req *subreq) { struct dcesrv_connection *dce_conn = tevent_req_callback_data(subreq, struct dcesrv_connection); + struct dcesrv_context *dce_ctx = dce_conn->dce_ctx; struct ncacn_packet *pkt; 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_ctx); + return; + } + + dcesrv_cleanup_broken_connections(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..66fe51e 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; @@ -208,7 +211,8 @@ struct dcesrv_connection { /* the transport level session key */ DATA_BLOB transport_session_key; - bool processing; + /* is this connection pending termination? If so, why? */ + 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.10.4 From a1db2aec46ec16a7aab51be55fc53752edddfdac Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 10 Jul 2013 14:48:18 +0200 Subject: [PATCH 4/4] s4:server: avoid calling into nss_winbind from within 'samba' The most important part is that the 'winbind_server' doesn't recurse into itself. This could happen if the krb5 libraries call getlogin(). As we may run in single process mode, we need to set _NO_WINBINDD=1 everywhere, the only exception is the forked 'smbd'. Signed-off-by: Stefan Metzmacher Reviewed-by: Andrew Bartlett Autobuild-User(master): Stefan Metzmacher Autobuild-Date(master): Wed Jul 10 23:18:06 CEST 2013 on sn-devel-104 (cherry picked from commit 596b51c666e549fb518d92931d8837922154a2fe) --- file_server/file_server.c | 9 +++++++++ source4/smbd/server.c | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/file_server/file_server.c b/file_server/file_server.c index 5d44d5a..aab5f39 100644 --- a/file_server/file_server.c +++ b/file_server/file_server.c @@ -28,6 +28,7 @@ #include "source4/smbd/process_model.h" #include "file_server/file_server.h" #include "dynconfig.h" +#include "nsswitch/winbind_client.h" /* called if smbd exits @@ -64,6 +65,8 @@ static void s3fs_task_init(struct task_server *task) smbd_path = talloc_asprintf(task, "%s/smbd", dyn_SBINDIR); smbd_cmd[0] = smbd_path; + /* the child should be able to call through nss_winbind */ + (void)winbind_on(); /* start it as a child process */ subreq = samba_runcmd_send(task, task->event_ctx, timeval_zero(), 1, 0, smbd_cmd, @@ -72,6 +75,12 @@ static void s3fs_task_init(struct task_server *task) "--foreground", debug_get_output_is_stdout()?"--log-stdout":NULL, NULL); + /* the parent should not be able to call through nss_winbind */ + if (!winbind_off()) { + DEBUG(0,("Failed to re-disable recursive winbindd calls after forking smbd\n")); + task_server_terminate(task, "Failed to re-disable recursive winbindd calls", true); + return; + } if (subreq == NULL) { DEBUG(0, ("Failed to start smbd as child daemon\n")); task_server_terminate(task, "Failed to startup s3fs smb task", true); diff --git a/source4/smbd/server.c b/source4/smbd/server.c index 0ad3e6b..37aac62 100644 --- a/source4/smbd/server.c +++ b/source4/smbd/server.c @@ -43,6 +43,7 @@ #include "cluster/cluster.h" #include "dynconfig/dynconfig.h" #include "lib/util/samba_modules.h" +#include "nsswitch/winbind_client.h" /* recursively delete a directory tree @@ -402,6 +403,12 @@ static int binary_smbd_main(const char *binary_name, int argc, const char *argv[ } } + /* make sure we won't go through nss_winbind */ + if (!winbind_off()) { + DEBUG(0,("Failed to disable recusive winbindd calls. Exiting.\n")); + exit(1); + } + gensec_init(); /* FIXME: */ ntptr_init(); /* FIXME: maybe run this in the initialization function -- 1.7.10.4