From e52502b38d2ffff8103006cca1f86ee7a55e6b45 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Mar 2020 10:55:22 -0700 Subject: [PATCH 1/6] s3: smbd: In aio_del_req_from_fsp() talloc_free(fsp->aio_requests[]) when fsp->num_aio_requests reaches zero. The add code in aio_add_req_to_fsp() re-tallocs this array on demand, and talloc freeing it here allows it to be used as the parent for a tevent wait queue, so callers can get notified when all outstanding aio on an fsp is finished. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/aio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source3/smbd/aio.c b/source3/smbd/aio.c index 0f824f5aa1f..afe76608cd3 100644 --- a/source3/smbd/aio.c +++ b/source3/smbd/aio.c @@ -103,6 +103,7 @@ static int aio_del_req_from_fsp(struct aio_req_fsp_link *lnk) if (fsp->num_aio_requests == 0) { tevent_wait_done(fsp->deferred_close); + TALLOC_FREE(fsp->aio_requests); } return 0; } -- 2.25.1.481.gfbce0eb801-goog From f83d008cfbddc485948c6d22b9590c6791152e1a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Mar 2020 12:25:58 -0700 Subject: [PATCH 2/6] s3: smbd: Don't allow force disconnect of a connection already being disconnected. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/conn_idle.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/source3/smbd/conn_idle.c b/source3/smbd/conn_idle.c index cd12e3f1266..9de2ae91d94 100644 --- a/source3/smbd/conn_idle.c +++ b/source3/smbd/conn_idle.c @@ -103,6 +103,12 @@ void conn_force_tdis( } tcon = conn->tcon; + if (NT_STATUS_EQUAL(tcon->status, + NT_STATUS_NETWORK_NAME_DELETED)) { + /* In the process of already being disconnected. */ + continue; + } + do_close = check_fn(conn, private_data); if (!do_close) { continue; -- 2.25.1.481.gfbce0eb801-goog From 814ade554e68e65ef3d6b22b760c354dd2255d4b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Mar 2020 13:37:19 -0700 Subject: [PATCH 3/6] s3: smbd: Add async version of conn_force_tdis(). Commented out so it can be seen complete as a diff. The next commit will replace the old synchronous conn_force_tdis() code with the new async code. Uses a wait_queue to cause the force close requests to stay pending until all outstanding aio is finished on all file handles opened on the connection. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/conn_idle.c | 191 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 191 insertions(+) diff --git a/source3/smbd/conn_idle.c b/source3/smbd/conn_idle.c index 9de2ae91d94..1c47bffe2cd 100644 --- a/source3/smbd/conn_idle.c +++ b/source3/smbd/conn_idle.c @@ -23,6 +23,7 @@ #include "smbd/smbd.h" #include "smbd/globals.h" #include "rpc_server/rpc_pipes.h" +#include "lib/util/tevent_ntstatus.h" /**************************************************************************** Update last used timestamps. @@ -75,6 +76,196 @@ bool conn_idle_all(struct smbd_server_connection *sconn, time_t t) return true; } +#if 0 +struct conn_force_tdis_state { + struct tevent_queue *wait_queue; +}; + +static void conn_force_tdis_wait_done(struct tevent_req *subreq); + +static struct tevent_req *conn_force_tdis_send(connection_struct *conn) +{ + struct tevent_req *req; + struct conn_force_tdis_state *state; + struct tevent_req *subreq; + files_struct *fsp; + + /* Create this off the NULL context. We must clean up on return. */ + req = tevent_req_create(NULL, &state, + struct conn_force_tdis_state); + if (req == NULL) { + return NULL; + } + state->wait_queue = tevent_queue_create(state, + "conn_force_tdis_wait_queue"); + if (tevent_req_nomem(state->wait_queue, req)) { + TALLOC_FREE(req); + return NULL; + } + + /* + * Make sure that no new request will be able to use this tcon. + * This ensures that once all outstanding fsp->aio_requests + * on this tcon are done, we are safe to close it. + */ + conn->tcon->status = NT_STATUS_NETWORK_NAME_DELETED; + + for (fsp = conn->sconn->files; fsp; fsp = fsp->next) { + if (fsp->conn != conn) { + continue; + } + if (fsp->num_aio_requests > 0) { + /* + * Now wait until all aio requests on this fsp are + * finished. + * + * We don't set a callback, as we just want to block the + * wait queue and the talloc_free() of fsp->aio_request + * will remove the item from the wait queue. + */ + subreq = tevent_queue_wait_send(fsp->aio_requests, + conn->sconn->ev_ctx, + state->wait_queue); + if (tevent_req_nomem(subreq, req)) { + TALLOC_FREE(req); + return NULL; + } + } + } + /* + * Now we add our own waiter to the end of the queue, + * this way we get notified when all pending requests are finished + * and reply to the outstanding SMB1 request. + */ + subreq = tevent_queue_wait_send(state, + conn->sconn->ev_ctx, + state->wait_queue); + if (tevent_req_nomem(subreq, req)) { + TALLOC_FREE(req); + return NULL; + } + + tevent_req_set_callback(subreq, conn_force_tdis_wait_done, req); + return req; +} + +static void conn_force_tdis_wait_done(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + + tevent_queue_wait_recv(subreq); + TALLOC_FREE(subreq); + tevent_req_done(req); +} + +static NTSTATUS conn_force_tdis_recv(struct tevent_req *req) +{ + return tevent_req_simple_recv_ntstatus(req); +} + +static void conn_force_tdis_done(struct tevent_req *req) +{ + connection_struct *conn = tevent_req_callback_data( + req, connection_struct); + NTSTATUS status; + uint64_t vuid = UID_FIELD_INVALID; + struct smbXsrv_tcon *tcon = conn->tcon; + struct smbd_server_connection *sconn = conn->sconn; + + status = conn_force_tdis_recv(req); + TALLOC_FREE(req); + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("conn_force_tdis_recv of share '%s' " + "(wire_id=0x%08x) failed: %s\n", + tcon->global->share_name, + tcon->global->tcon_wire_id, + nt_errstr(status)); + return; + } + + if (conn->sconn->using_smb2) { + vuid = conn->vuid; + } + + DBG_WARNING("Closing " + "share '%s' (wire_id=0x%08x)\n", + tcon->global->share_name, + tcon->global->tcon_wire_id); + + conn = NULL; + status = smbXsrv_tcon_disconnect(tcon, vuid); + if (!NT_STATUS_IS_OK(status)) { + DBG_ERR("smbXsrv_tcon_disconnect() of share '%s' " + "(wire_id=0x%08x) failed: %s\n", + tcon->global->share_name, + tcon->global->tcon_wire_id, + nt_errstr(status)); + return; + } + + TALLOC_FREE(tcon); + + /* + * As we've been awoken, we may have changed + * uid in the meantime. Ensure we're still root. + */ + change_to_root_user(); + reload_services(sconn, conn_snum_used, true); +} + +/**************************************************************************** + Forcibly unmount a share - async. + All instances of the parameter 'sharename' share are unmounted. + The special sharename '*' forces unmount of all shares. +****************************************************************************/ + +void conn_force_tdis( + struct smbd_server_connection *sconn, + bool (*check_fn)(struct connection_struct *conn, + void *private_data), + void *private_data) +{ + connection_struct *conn; + + /* SMB1 and SMB 2*/ + for (conn = sconn->connections; conn; conn = conn->next) { + bool do_close = false; + struct tevent_req *req; + + if (conn->tcon == NULL) { + continue; + } + + if (NT_STATUS_EQUAL(conn->tcon->status, + NT_STATUS_NETWORK_NAME_DELETED)) { + /* In the process of already being disconnected. */ + continue; + } + + do_close = check_fn(conn, private_data); + if (!do_close) { + continue; + } + + req = conn_force_tdis_send(conn); + if (req == NULL) { + DBG_WARNING("talloc_fail forcing async close of " + "share '%s'\n", + conn->tcon->global->share_name); + continue; + } + + DBG_WARNING("Forcing async close of " + "share '%s' (wire_id=0x%08x)\n", + conn->tcon->global->share_name, + conn->tcon->global->tcon_wire_id); + + tevent_req_set_callback(req, conn_force_tdis_done, conn); + } +} +#endif + /**************************************************************************** Forcibly unmount a share. All instances of the parameter 'sharename' share are unmounted. -- 2.25.1.481.gfbce0eb801-goog From 7a455decd182593ae0cdaeaebf2d724ea4e8a61f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Mar 2020 13:39:21 -0700 Subject: [PATCH 4/6] s3: smbd: Replace synchronous conn_force_tdis() with the async version. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/conn_idle.c | 68 ---------------------------------------- 1 file changed, 68 deletions(-) diff --git a/source3/smbd/conn_idle.c b/source3/smbd/conn_idle.c index 1c47bffe2cd..d41d0265904 100644 --- a/source3/smbd/conn_idle.c +++ b/source3/smbd/conn_idle.c @@ -76,7 +76,6 @@ bool conn_idle_all(struct smbd_server_connection *sconn, time_t t) return true; } -#if 0 struct conn_force_tdis_state { struct tevent_queue *wait_queue; }; @@ -264,70 +263,3 @@ void conn_force_tdis( tevent_req_set_callback(req, conn_force_tdis_done, conn); } } -#endif - -/**************************************************************************** - Forcibly unmount a share. - All instances of the parameter 'sharename' share are unmounted. - The special sharename '*' forces unmount of all shares. -****************************************************************************/ - -void conn_force_tdis( - struct smbd_server_connection *sconn, - bool (*check_fn)(struct connection_struct *conn, - void *private_data), - void *private_data) -{ - connection_struct *conn, *next; - - /* SMB1 and SMB 2*/ - for (conn = sconn->connections; conn; conn = next) { - struct smbXsrv_tcon *tcon; - bool do_close = false; - NTSTATUS status; - uint64_t vuid = UID_FIELD_INVALID; - - next = conn->next; - - if (conn->tcon == NULL) { - continue; - } - tcon = conn->tcon; - - if (NT_STATUS_EQUAL(tcon->status, - NT_STATUS_NETWORK_NAME_DELETED)) { - /* In the process of already being disconnected. */ - continue; - } - - do_close = check_fn(conn, private_data); - if (!do_close) { - continue; - } - - DBG_WARNING("Forcing close of " - "share '%s' (wire_id=0x%08x)\n", - tcon->global->share_name, - tcon->global->tcon_wire_id); - - if (sconn->using_smb2) { - vuid = conn->vuid; - } - - conn = NULL; - status = smbXsrv_tcon_disconnect(tcon, vuid); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(0, ("conn_force_tdis: " - "smbXsrv_tcon_disconnect() of share '%s' " - "(wire_id=0x%08x) failed: %s\n", - tcon->global->share_name, - tcon->global->tcon_wire_id, - nt_errstr(status))); - } - - TALLOC_FREE(tcon); - } - - change_to_root_user(); - reload_services(sconn, conn_snum_used, true); -} -- 2.25.1.481.gfbce0eb801-goog From 212622bbba91ac2070ef7cd540128b5a5f8a69fc Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Mar 2020 11:15:19 -0700 Subject: [PATCH 5/6] s3: smbd: Add async version of SMB1tdis. Waits until all aio requests on all fsp's under this conn struct are finished before returning to the client. Done this way (commented out) so it is a clean diff and it's clear what is being added. The next commit will remove the old synchronous version. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 180 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index aef34d9ede8..563df0c3e31 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -42,6 +42,7 @@ #include "smbprofile.h" #include "../lib/tsocket/tsocket.h" #include "lib/tevent_wait.h" +#include "lib/util/tevent_ntstatus.h" #include "libcli/smb/smb_signing.h" #include "lib/util/sys_rw_data.h" #include "librpc/gen_ndr/open_files.h" @@ -6026,6 +6027,185 @@ void reply_unlock(struct smb_request *req) #undef DBGC_CLASS #define DBGC_CLASS DBGC_ALL +#if 0 +struct smbd_smb1_tdis_state { + struct tevent_queue *wait_queue; +}; + +static void smbd_smb1_tdis_wait_done(struct tevent_req *subreq); + +/**************************************************************************** + Async SMB1 tdis. + Note, on failure here we deallocate and return NULL to allow the caller to + SMB1 return an error of ERRnomem immediately. +****************************************************************************/ + +static struct tevent_req *smbd_smb1_tdis_send(struct smb_request *smb1req) +{ + struct tevent_req *req; + struct smbd_smb1_tdis_state *state; + struct tevent_req *subreq; + connection_struct *conn = smb1req->conn; + files_struct *fsp; + + /* Create this off the NULL context. We must clean up on return. */ + req = tevent_req_create(NULL, &state, + struct smbd_smb1_tdis_state); + if (req == NULL) { + return NULL; + } + state->wait_queue = tevent_queue_create(state, "smb1_tdis_wait_queue"); + if (tevent_req_nomem(state->wait_queue, req)) { + TALLOC_FREE(req); + return NULL; + } + + /* + * Make sure that no new request will be able to use this tcon. + * This ensures that once all outstanding fsp->aio_requests + * on this tcon are done, we are safe to close it. + */ + conn->tcon->status = NT_STATUS_NETWORK_NAME_DELETED; + + for (fsp = conn->sconn->files; fsp; fsp = fsp->next) { + if (fsp->conn != conn) { + continue; + } + if (fsp->num_aio_requests > 0) { + /* + * Now wait until all aio requests on this fsp are + * finished. + * + * We don't set a callback, as we just want to block the + * wait queue and the talloc_free() of fsp->aio_request + * will remove the item from the wait queue. + */ + subreq = tevent_queue_wait_send(fsp->aio_requests, + conn->sconn->ev_ctx, + state->wait_queue); + if (tevent_req_nomem(subreq, req)) { + TALLOC_FREE(req); + return NULL; + } + } + } + + /* + * Now we add our own waiter to the end of the queue, + * this way we get notified when all pending requests are finished + * and reply to the outstanding SMB1 request. + */ + subreq = tevent_queue_wait_send(state, + conn->sconn->ev_ctx, + state->wait_queue); + if (tevent_req_nomem(subreq, req)) { + TALLOC_FREE(req); + return NULL; + } + + /* + * We're really going async - move the SMB1 request from + * a talloc stackframe above us to the NULL context. + * We need this to stick around until the wait_done + * callback is invoked. + */ + smb1req = talloc_move(NULL, &smb1req); + + tevent_req_set_callback(subreq, smbd_smb1_tdis_wait_done, req); + + return req; +} + +static void smbd_smb1_tdis_wait_done(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + + tevent_queue_wait_recv(subreq); + TALLOC_FREE(subreq); + tevent_req_done(req); +} + +static NTSTATUS smbd_smb1_tdis_recv(struct tevent_req *req) +{ + return tevent_req_simple_recv_ntstatus(req); +} + +static void smbd_smb1_request_tdis_done(struct tevent_req *req) +{ + struct smb_request *smb1req = tevent_req_callback_data( + req, struct smb_request); + NTSTATUS status; + struct smbXsrv_tcon *tcon = smb1req->conn->tcon; + + /* + * As we've been awoken, we may have changed + * uid in the meantime. Ensure we're still root. + */ + change_to_root_user(); + + status = smbd_smb1_tdis_recv(req); + TALLOC_FREE(req); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb1req); + END_PROFILE(SMBtdis); + exit_server(__location__ ": smbd_smb1_tdis_recv failed"); + return; + } + + status = smbXsrv_tcon_disconnect(tcon, + smb1req->vuid); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb1req); + END_PROFILE(SMBtdis); + exit_server(__location__ ": smbXsrv_tcon_disconnect failed"); + return; + } + + /* smbXsrv_tcon_disconnect frees smb1req->conn. */ + smb1req->conn = NULL; + + TALLOC_FREE(tcon); + + reply_outbuf(smb1req, 0, 0); + /* + * The following call is needed to push the + * reply data back out the socket after async + * return. Plus it frees smb1req. + */ + smb_request_done(smb1req); + END_PROFILE(SMBtdis); +} + +/**************************************************************************** + Reply to a tdis. + conn POINTER CAN BE NULL HERE ! +****************************************************************************/ + +void reply_tdis(struct smb_request *smb1req) +{ + struct tevent_req *req; + + START_PROFILE(SMBtdis); + + if (smb1req->conn == NULL) { + DEBUG(4,("Invalid connection in tdis\n")); + reply_force_doserror(smb1req, ERRSRV, ERRinvnid); + END_PROFILE(SMBtdis); + return; + } + + req = smbd_smb1_tdis_send(smb1req); + if (req == NULL) { + reply_force_doserror(smb1req, ERRDOS, ERRnomem); + END_PROFILE(SMBtdis); + return; + } + /* We're async. This will complete later. */ + tevent_req_set_callback(req, smbd_smb1_request_tdis_done, smb1req); + return; +} +#endif /**************************************************************************** Reply to a tdis. conn POINTER CAN BE NULL HERE ! -- 2.25.1.481.gfbce0eb801-goog From 6dc6770f49593ebb9b99807a9153beb2c0e4fbb2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Mar 2020 11:17:41 -0700 Subject: [PATCH 6/6] s3: smbd: Remove old synchronous SMB1 reply_tdis(). SMB1 tree disconnect is now fully async. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 48 -------------------------------------------- 1 file changed, 48 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 563df0c3e31..58987e6af1a 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -6027,7 +6027,6 @@ void reply_unlock(struct smb_request *req) #undef DBGC_CLASS #define DBGC_CLASS DBGC_ALL -#if 0 struct smbd_smb1_tdis_state { struct tevent_queue *wait_queue; }; @@ -6205,53 +6204,6 @@ void reply_tdis(struct smb_request *smb1req) tevent_req_set_callback(req, smbd_smb1_request_tdis_done, smb1req); return; } -#endif -/**************************************************************************** - Reply to a tdis. - conn POINTER CAN BE NULL HERE ! -****************************************************************************/ - -void reply_tdis(struct smb_request *req) -{ - NTSTATUS status; - connection_struct *conn = req->conn; - struct smbXsrv_tcon *tcon; - - START_PROFILE(SMBtdis); - - if (!conn) { - DEBUG(4,("Invalid connection in tdis\n")); - reply_force_doserror(req, ERRSRV, ERRinvnid); - END_PROFILE(SMBtdis); - return; - } - - tcon = conn->tcon; - req->conn = NULL; - - /* - * TODO: cancel all outstanding requests on the tcon - */ - status = smbXsrv_tcon_disconnect(tcon, req->vuid); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(0, ("reply_tdis: " - "smbXsrv_tcon_disconnect() failed: %s\n", - nt_errstr(status))); - /* - * If we hit this case, there is something completely - * wrong, so we better disconnect the transport connection. - */ - END_PROFILE(SMBtdis); - exit_server(__location__ ": smbXsrv_tcon_disconnect failed"); - return; - } - - TALLOC_FREE(tcon); - - reply_outbuf(req, 0, 0); - END_PROFILE(SMBtdis); - return; -} /**************************************************************************** Reply to a echo. -- 2.25.1.481.gfbce0eb801-goog