From dce62cf5df41ce8f357143ea85e38133dec61bb1 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 15:45:16 -0700 Subject: [PATCH 01/39] s3: tests: Slight tweak to the force-close share test. Turns out on a fast desktop machine 10MB is too small, and by the time we've done the 'sleep 1' to make sure the smbclient got scheduled and started processing the 'put' command, it's already done. Tweak the file size to be 20MB from 10MB. 10MB seems to work reliably on gitlab-ci and on sn-devel, but making the put size 20MB makes sure it's still in flight when we force-close the share, even on a fast desktop box. 20MB shouldn't be too burdonsome even on ci VM's. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/script/tests/test_force_close_share.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/script/tests/test_force_close_share.sh b/source3/script/tests/test_force_close_share.sh index ebfff5af77c..da78b5a752e 100755 --- a/source3/script/tests/test_force_close_share.sh +++ b/source3/script/tests/test_force_close_share.sh @@ -32,7 +32,7 @@ mkfifo smbclient-stdin smbclient-stdout smbclient-stderr # Create a large-ish testfile rm testfile -head -c 10MB /dev/zero >testfile +head -c 20MB /dev/zero >testfile CLI_FORCE_INTERACTIVE=1; export CLI_FORCE_INTERACTIVE -- 2.20.1 From 34f436e7403feb524f7b3839cbf5292e9f87e9b4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Mar 2020 10:55:22 -0700 Subject: [PATCH 02/39] 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. We'll deal with any performance issues in the next commit. 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.20.1 From c3bc87effa2d0833257854303eff49853e8e356b Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 11 Mar 2020 14:47:50 -0700 Subject: [PATCH 03/39] s3: smbd: Now we free fsp->aio_requests when it gets zero entries, talloc in chunks of 10 instead of 1. Prevents incremental +1 tallocs, and the original idea of this array was that it wasn't freed for io efficiency reasons. Add paranoia integer wrap protection also. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/aio.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/source3/smbd/aio.c b/source3/smbd/aio.c index afe76608cd3..cf35f3297ec 100644 --- a/source3/smbd/aio.c +++ b/source3/smbd/aio.c @@ -122,9 +122,19 @@ bool aio_add_req_to_fsp(files_struct *fsp, struct tevent_req *req) if (array_len <= fsp->num_aio_requests) { struct tevent_req **tmp; + if (fsp->num_aio_requests + 10 < 10) { + /* Integer wrap. */ + TALLOC_FREE(lnk); + return false; + } + + /* + * Allocate in blocks of 10 so we don't allocate + * on every aio request. + */ tmp = talloc_realloc( fsp, fsp->aio_requests, struct tevent_req *, - fsp->num_aio_requests+1); + fsp->num_aio_requests+10); if (tmp == NULL) { TALLOC_FREE(lnk); return false; -- 2.20.1 From af7009fe3be01e52eb321a5f6be639ccea8b37db Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 11 Mar 2020 17:25:59 -0700 Subject: [PATCH 04/39] s3: smbd: In async SMB1 reply_close() set fsp->closing = true, as we already do in SMB2 async close. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index aef34d9ede8..55c77346050 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -5672,6 +5672,13 @@ void reply_close(struct smb_request *req) DEBUG(10, ("closing with aio %u requests pending\n", fsp->num_aio_requests)); + /* + * Flag the file as close in progress. + * This will prevent any more IO being + * done on it. + */ + fsp->closing = true; + /* * We depend on the aio_extra destructor to take care of this * close request once fsp->num_aio_request drops to 0. -- 2.20.1 From 856cc042298b4866e089ee56e9f4f38e300d0855 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 11 Mar 2020 15:16:35 -0700 Subject: [PATCH 05/39] s3: smbd: Every place we check fsp->deferred_close, also check for fsp->closing. Eventually this will allow us to remove fsp->deferred_close from the fsp struct (and also source3/lib/tevent_wait.[ch]). BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/offload_token.c | 10 ++++++++++ source3/smbd/files.c | 14 ++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/source3/modules/offload_token.c b/source3/modules/offload_token.c index 3fb84dabdff..03bb3309f38 100644 --- a/source3/modules/offload_token.c +++ b/source3/modules/offload_token.c @@ -280,6 +280,16 @@ NTSTATUS vfs_offload_token_check_handles(uint32_t fsctl, return NT_STATUS_ACCESS_DENIED; } + if (src_fsp->closing) { + DBG_INFO("copy chunk src handle with closing in progress.\n"); + return NT_STATUS_ACCESS_DENIED; + } + + if (dst_fsp->closing) { + DBG_INFO("copy chunk dst handle with closing in progress.\n"); + return NT_STATUS_ACCESS_DENIED; + } + if (src_fsp->is_directory) { DBG_INFO("copy chunk no read on src directory handle (%s).\n", smb_fname_str_dbg(src_fsp->fsp_name)); diff --git a/source3/smbd/files.c b/source3/smbd/files.c index 99b2f343685..b06511147ab 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -598,6 +598,9 @@ files_struct *file_fsp(struct smb_request *req, uint16_t fid) if (req->chain_fsp->deferred_close) { return NULL; } + if (req->chain_fsp->closing) { + return NULL; + } return req->chain_fsp; } @@ -622,6 +625,10 @@ files_struct *file_fsp(struct smb_request *req, uint16_t fid) return NULL; } + if (fsp->closing) { + return NULL; + } + req->chain_fsp = fsp; return fsp; } @@ -669,6 +676,10 @@ struct files_struct *file_fsp_get(struct smbd_smb2_request *smb2req, return NULL; } + if (fsp->closing) { + return NULL; + } + return fsp; } @@ -682,6 +693,9 @@ struct files_struct *file_fsp_smb2(struct smbd_smb2_request *smb2req, if (smb2req->compat_chain_fsp->deferred_close) { return NULL; } + if (smb2req->compat_chain_fsp->closing) { + return NULL; + } return smb2req->compat_chain_fsp; } -- 2.20.1 From 5b95b45969e69152404775f157b8fd0d65141964 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 10 Mar 2020 12:25:58 -0700 Subject: [PATCH 06/39] 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.20.1 From 2b40971dc801543c9e78b2de3bd7f665305bdff2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 09:28:21 -0700 Subject: [PATCH 07/39] s3: smbd: Add async internals 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 | 148 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/source3/smbd/conn_idle.c b/source3/smbd/conn_idle.c index 9de2ae91d94..7b2863c85d6 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. @@ -140,3 +141,150 @@ void conn_force_tdis( change_to_root_user(); reload_services(sconn, conn_snum_used, 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; + } + /* + * Flag the file as close in progress. + * This will prevent any more IO being + * done on it. Not strictly needed, but + * doesn't hurt to flag it as closing. + */ + fsp->closing = true; + + 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); +} +#endif -- 2.20.1 From e852f190437c98eac3bfb75669184b4b0afbcd9e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 09:47:26 -0700 Subject: [PATCH 08/39] 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 | 44 +++++++++++++++------------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/source3/smbd/conn_idle.c b/source3/smbd/conn_idle.c index 7b2863c85d6..6d818c4b9ab 100644 --- a/source3/smbd/conn_idle.c +++ b/source3/smbd/conn_idle.c @@ -77,27 +77,27 @@ bool conn_idle_all(struct smbd_server_connection *sconn, time_t t) } /**************************************************************************** - Forcibly unmount a share. + Forcibly unmount a share - async All instances of the parameter 'sharename' share are unmounted. The special sharename '*' forces unmount of all shares. ****************************************************************************/ +static struct tevent_req *conn_force_tdis_send(connection_struct *conn); +static void conn_force_tdis_done(struct tevent_req *req); + 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; + connection_struct *conn; /* SMB1 and SMB 2*/ - for (conn = sconn->connections; conn; conn = next) { + for (conn = sconn->connections; conn; conn = conn->next) { struct smbXsrv_tcon *tcon; bool do_close = false; - NTSTATUS status; - uint64_t vuid = UID_FIELD_INVALID; - - next = conn->next; + struct tevent_req *req; if (conn->tcon == NULL) { continue; @@ -115,34 +115,23 @@ void conn_force_tdis( continue; } + req = conn_force_tdis_send(conn); + if (req == NULL) { + DBG_WARNING("talloc_fail forcing async close of " + "share '%s'\n", + tcon->global->share_name); + 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); + tevent_req_set_callback(req, conn_force_tdis_done, conn); } - - change_to_root_user(); - reload_services(sconn, conn_snum_used, true); } -#if 0 struct conn_force_tdis_state { struct tevent_queue *wait_queue; }; @@ -287,4 +276,3 @@ static void conn_force_tdis_done(struct tevent_req *req) change_to_root_user(); reload_services(sconn, conn_snum_used, true); } -#endif -- 2.20.1 From 146f4212d9b018ffec741da795f1fbe81e0374df Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 09:51:53 -0700 Subject: [PATCH 09/39] s3: smbd: Add async internals of reply_tdis(). Waits until all aio requests on all fsp's under this conn struct are finished before returning to the client. Charges the profile time in the done function. Not strictly correct but better than the other SMB1 async code that double-charges profiling in both send and done at the moment. Done this way (commented out) so it is a clean diff and it's clear what is being added. A later 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 | 174 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 55c77346050..65b224c8a46 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" @@ -6080,6 +6081,179 @@ void reply_tdis(struct smb_request *req) return; } +#if 0 +struct reply_tdis_state { + struct tevent_queue *wait_queue; +}; + +static void reply_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 *reply_tdis_send(struct smb_request *smb1req) +{ + struct tevent_req *req; + struct reply_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 reply_tdis_state); + if (req == NULL) { + return NULL; + } + state->wait_queue = tevent_queue_create(state, "reply_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; + } + /* + * Flag the file as close in progress. + * This will prevent any more IO being + * done on it. Not strictly needed, but + * doesn't hurt to flag it as closing. + */ + fsp->closing = true; + + 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, reply_tdis_wait_done, req); + + return req; +} + +static void reply_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 reply_tdis_recv(struct tevent_req *req) +{ + return tevent_req_simple_recv_ntstatus(req); +} + +static void reply_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; + bool ok; + + /* + * Take the profile charge here. Not strictly + * correct but better than the other SMB1 async + * code that double-charges at the moment. + */ + START_PROFILE(SMBtdis); + + status = reply_tdis_recv(req); + TALLOC_FREE(req); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb1req); + END_PROFILE(SMBtdis); + exit_server(__location__ ": reply_tdis_recv failed"); + return; + } + + /* + * As we've been awoken, we may have changed + * directory in the meantime. + * reply_tdis() has the DO_CHDIR flag set. + */ + ok = chdir_current_service(smb1req->conn); + if (!ok) { + reply_force_doserror(smb1req, ERRSRV, ERRinvnid); + smb_request_done(smb1req); + END_PROFILE(SMBtdis); + } + + 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); +} +#endif + /**************************************************************************** Reply to a echo. conn POINTER CAN BE NULL HERE ! -- 2.20.1 From edc16a9e4e6f710a89122899cc4ab5c6e88a89b2 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 09:55:30 -0700 Subject: [PATCH 10/39] s3: smbd: In reply_tdis(), replace req -> smb1req. Minimises the diff in the next commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 65b224c8a46..b1f3e0f7f60 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -6039,28 +6039,28 @@ void reply_unlock(struct smb_request *req) conn POINTER CAN BE NULL HERE ! ****************************************************************************/ -void reply_tdis(struct smb_request *req) +void reply_tdis(struct smb_request *smb1req) { NTSTATUS status; - connection_struct *conn = req->conn; + connection_struct *conn = smb1req->conn; struct smbXsrv_tcon *tcon; START_PROFILE(SMBtdis); if (!conn) { DEBUG(4,("Invalid connection in tdis\n")); - reply_force_doserror(req, ERRSRV, ERRinvnid); + reply_force_doserror(smb1req, ERRSRV, ERRinvnid); END_PROFILE(SMBtdis); return; } tcon = conn->tcon; - req->conn = NULL; + smb1req->conn = NULL; /* * TODO: cancel all outstanding requests on the tcon */ - status = smbXsrv_tcon_disconnect(tcon, req->vuid); + status = smbXsrv_tcon_disconnect(tcon, smb1req->vuid); if (!NT_STATUS_IS_OK(status)) { DEBUG(0, ("reply_tdis: " "smbXsrv_tcon_disconnect() failed: %s\n", @@ -6076,7 +6076,7 @@ void reply_tdis(struct smb_request *req) TALLOC_FREE(tcon); - reply_outbuf(req, 0, 0); + reply_outbuf(smb1req, 0, 0); END_PROFILE(SMBtdis); return; } -- 2.20.1 From dca74713c8d6ede6a44a3737fd431f4f4df35594 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 09:59:47 -0700 Subject: [PATCH 11/39] s3: smbd: reply_tdis() Update to modern coding standards. Minimizes the diff in the next commit. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index b1f3e0f7f60..b5684fb1de5 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -6047,8 +6047,8 @@ void reply_tdis(struct smb_request *smb1req) START_PROFILE(SMBtdis); - if (!conn) { - DEBUG(4,("Invalid connection in tdis\n")); + if (conn == NULL) { + DBG_INFO("Invalid connection in tdis\n"); reply_force_doserror(smb1req, ERRSRV, ERRinvnid); END_PROFILE(SMBtdis); return; -- 2.20.1 From b479a36270e17152f40a8da8104977ac1a960bff Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 10:03:27 -0700 Subject: [PATCH 12/39] 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 | 45 +++++++++++++++++++------------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index b5684fb1de5..165554791cd 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -6039,49 +6039,43 @@ void reply_unlock(struct smb_request *req) conn POINTER CAN BE NULL HERE ! ****************************************************************************/ +static struct tevent_req *reply_tdis_send(struct smb_request *smb1req); +static void reply_tdis_done(struct tevent_req *req); + void reply_tdis(struct smb_request *smb1req) { - NTSTATUS status; connection_struct *conn = smb1req->conn; - struct smbXsrv_tcon *tcon; + struct tevent_req *req; - START_PROFILE(SMBtdis); + /* + * Don't setup the profile charge here, take + * it in reply_tdis_done(). Not strictly correct + * but better than the other SMB1 async + * code that double-charges at the moment. + */ if (conn == NULL) { + /* Not going async, profile here. */ + START_PROFILE(SMBtdis); DBG_INFO("Invalid connection in tdis\n"); reply_force_doserror(smb1req, ERRSRV, ERRinvnid); END_PROFILE(SMBtdis); return; } - tcon = conn->tcon; - smb1req->conn = NULL; - - /* - * TODO: cancel all outstanding requests on the tcon - */ - status = smbXsrv_tcon_disconnect(tcon, smb1req->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. - */ + req = reply_tdis_send(smb1req); + if (req == NULL) { + /* Not going async, profile here. */ + START_PROFILE(SMBtdis); + reply_force_doserror(smb1req, ERRDOS, ERRnomem); END_PROFILE(SMBtdis); - exit_server(__location__ ": smbXsrv_tcon_disconnect failed"); return; } - - TALLOC_FREE(tcon); - - reply_outbuf(smb1req, 0, 0); - END_PROFILE(SMBtdis); + /* We're async. This will complete later. */ + tevent_req_set_callback(req, reply_tdis_done, smb1req); return; } -#if 0 struct reply_tdis_state { struct tevent_queue *wait_queue; }; @@ -6252,7 +6246,6 @@ static void reply_tdis_done(struct tevent_req *req) smb_request_done(smb1req); END_PROFILE(SMBtdis); } -#endif /**************************************************************************** Reply to a echo. -- 2.20.1 From 42f498599c65acfcd5c9df982058af8c5f78f3a4 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 10:11:14 -0700 Subject: [PATCH 13/39] s3: smbd: Add async internals of reply_ulogoffX. Waits until all aio requests on all fsp's owned by this vuid are finished before returning to the client. Charges the profile time in the done function. Not strictly correct but better than the other SMB1 async code that double-charges profiling in both send and done at the moment. Done this way (commented out) so it is a clean diff and it's clear what is being added. A later 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 | 169 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 165554791cd..348c0eb625c 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -2665,6 +2665,175 @@ void reply_ulogoffX(struct smb_request *req) req->vuid = UID_FIELD_INVALID; } +#if 0 +struct reply_ulogoffX_state { + struct tevent_queue *wait_queue; + struct smbXsrv_session *session; +}; + +static void reply_ulogoffX_wait_done(struct tevent_req *subreq); + +/**************************************************************************** + Async SMB1 ulogoffX. + 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 *reply_ulogoffX_send(struct smb_request *smb1req, + struct smbXsrv_session *session) +{ + struct tevent_req *req; + struct reply_ulogoffX_state *state; + struct tevent_req *subreq; + files_struct *fsp; + struct smbd_server_connection *sconn = session->client->sconn; + uint64_t vuid = session->global->session_wire_id; + + /* Create this off the NULL context. We must clean up on return. */ + req = tevent_req_create(NULL, &state, + struct reply_ulogoffX_state); + if (req == NULL) { + return NULL; + } + state->wait_queue = tevent_queue_create(state, + "reply_ulogoffX_wait_queue"); + if (tevent_req_nomem(state->wait_queue, req)) { + TALLOC_FREE(req); + return NULL; + } + state->session = session; + + /* + * Make sure that no new request will be able to use this session. + * This ensures that once all outstanding fsp->aio_requests + * on this session are done, we are safe to close it. + */ + session->status = NT_STATUS_USER_SESSION_DELETED; + + for (fsp = sconn->files; fsp; fsp = fsp->next) { + if (fsp->vuid != vuid) { + continue; + } + /* + * Flag the file as close in progress. + * This will prevent any more IO being + * done on it. + */ + fsp->closing = true; + + 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, + 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, + 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, reply_ulogoffX_wait_done, req); + + return req; +} + +static void reply_ulogoffX_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 reply_ulogoffX_recv(struct tevent_req *req) +{ + return tevent_req_simple_recv_ntstatus(req); +} + +static void reply_ulogoffX_done(struct tevent_req *req) +{ + struct smb_request *smb1req = tevent_req_callback_data( + req, struct smb_request); + struct reply_ulogoffX_state *state = tevent_req_data(req, + struct reply_ulogoffX_state); + struct smbXsrv_session *session = state->session; + NTSTATUS status; + + /* + * Take the profile charge here. Not strictly + * correct but better than the other SMB1 async + * code that double-charges at the moment. + */ + START_PROFILE(SMBulogoffX); + + status = reply_ulogoffX_recv(req); + TALLOC_FREE(req); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb1req); + END_PROFILE(SMBulogoffX); + exit_server(__location__ ": reply_ulogoffX_recv failed"); + return; + } + + status = smbXsrv_session_logoff(session); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb1req); + END_PROFILE(SMBulogoffX); + exit_server(__location__ ": smbXsrv_session_logoff failed"); + return; + } + + TALLOC_FREE(session); + + reply_outbuf(smb1req, 2, 0); + SSVAL(smb1req->outbuf, smb_vwv0, 0xff); /* andx chain ends */ + SSVAL(smb1req->outbuf, smb_vwv1, 0); /* no andx offset */ + + DBG_NOTICE("ulogoffX vuid=%llu\n", + (unsigned long long)smb1req->vuid); + + smb1req->vuid = UID_FIELD_INVALID; + /* + * 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(SMBulogoffX); +} +#endif + /**************************************************************************** Reply to a mknew or a create. ****************************************************************************/ -- 2.20.1 From 18a9afa95fdad2c1e29581a2a6350a7880d81d30 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 10:15:10 -0700 Subject: [PATCH 14/39] s3: smbd: In reply_ulogoffX(), replace req -> smb1req. Minimises the diff in later commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 348c0eb625c..71126502043 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -2613,7 +2613,7 @@ void reply_open_and_X(struct smb_request *req) Reply to a SMBulogoffX. ****************************************************************************/ -void reply_ulogoffX(struct smb_request *req) +void reply_ulogoffX(struct smb_request *smb1req) { struct timeval now = timeval_current(); struct smbXsrv_session *session = NULL; @@ -2621,16 +2621,16 @@ void reply_ulogoffX(struct smb_request *req) START_PROFILE(SMBulogoffX); - status = smb1srv_session_lookup(req->xconn, - req->vuid, + status = smb1srv_session_lookup(smb1req->xconn, + smb1req->vuid, timeval_to_nttime(&now), &session); if (!NT_STATUS_IS_OK(status)) { DEBUG(3,("ulogoff, vuser id %llu does not map to user.\n", - (unsigned long long)req->vuid)); + (unsigned long long)smb1req->vuid)); - req->vuid = UID_FIELD_INVALID; - reply_force_doserror(req, ERRSRV, ERRbaduid); + smb1req->vuid = UID_FIELD_INVALID; + reply_force_doserror(smb1req, ERRSRV, ERRbaduid); END_PROFILE(SMBulogoffX); return; } @@ -2654,15 +2654,15 @@ void reply_ulogoffX(struct smb_request *req) TALLOC_FREE(session); - reply_outbuf(req, 2, 0); - SSVAL(req->outbuf, smb_vwv0, 0xff); /* andx chain ends */ - SSVAL(req->outbuf, smb_vwv1, 0); /* no andx offset */ + reply_outbuf(smb1req, 2, 0); + SSVAL(smb1req->outbuf, smb_vwv0, 0xff); /* andx chain ends */ + SSVAL(smb1req->outbuf, smb_vwv1, 0); /* no andx offset */ DEBUG(3, ("ulogoffX vuid=%llu\n", - (unsigned long long)req->vuid)); + (unsigned long long)smb1req->vuid)); END_PROFILE(SMBulogoffX); - req->vuid = UID_FIELD_INVALID; + smb1req->vuid = UID_FIELD_INVALID; } #if 0 -- 2.20.1 From c739c9d75fb87ff1bdd4d1e961090b65255123eb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 10:16:42 -0700 Subject: [PATCH 15/39] s3: smbd: reply_ulogoffX() Update to modern coding standards. Minimizes the diff in the later commits. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 71126502043..a25bf4cdece 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -2626,8 +2626,8 @@ void reply_ulogoffX(struct smb_request *smb1req) timeval_to_nttime(&now), &session); if (!NT_STATUS_IS_OK(status)) { - DEBUG(3,("ulogoff, vuser id %llu does not map to user.\n", - (unsigned long long)smb1req->vuid)); + DBG_WARNING("ulogoff, vuser id %llu does not map to user.\n", + (unsigned long long)smb1req->vuid); smb1req->vuid = UID_FIELD_INVALID; reply_force_doserror(smb1req, ERRSRV, ERRbaduid); -- 2.20.1 From 9d38fcf8c6786a8e6cb13ec502f94913d587ab1d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 10:19:31 -0700 Subject: [PATCH 16/39] s3: smbd: Remove old synchronous SMB1 reply_ulogoffX(). SMB1 user logoff 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, 21 insertions(+), 27 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index a25bf4cdece..ea66604ab27 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -2613,19 +2613,31 @@ void reply_open_and_X(struct smb_request *req) Reply to a SMBulogoffX. ****************************************************************************/ +static struct tevent_req *reply_ulogoffX_send(struct smb_request *smb1req, + struct smbXsrv_session *session); +static void reply_ulogoffX_done(struct tevent_req *req); + void reply_ulogoffX(struct smb_request *smb1req) { struct timeval now = timeval_current(); struct smbXsrv_session *session = NULL; + struct tevent_req *req; NTSTATUS status; - START_PROFILE(SMBulogoffX); + /* + * Don't setup the profile charge here, take + * it in reply_ulogoffX_done(). Not strictly correct + * but better than the other SMB1 async + * code that double-charges at the moment. + */ status = smb1srv_session_lookup(smb1req->xconn, smb1req->vuid, timeval_to_nttime(&now), &session); if (!NT_STATUS_IS_OK(status)) { + /* Not going async, profile here. */ + START_PROFILE(SMBulogoffX); DBG_WARNING("ulogoff, vuser id %llu does not map to user.\n", (unsigned long long)smb1req->vuid); @@ -2635,37 +2647,20 @@ void reply_ulogoffX(struct smb_request *smb1req) return; } - /* - * TODO: cancel all outstanding requests on the session - */ - status = smbXsrv_session_logoff(session); - if (!NT_STATUS_IS_OK(status)) { - DEBUG(0, ("reply_ulogoff: " - "smbXsrv_session_logoff() failed: %s\n", - nt_errstr(status))); - /* - * If we hit this case, there is something completely - * wrong, so we better disconnect the transport connection. - */ + req = reply_ulogoffX_send(smb1req, session); + if (req == NULL) { + /* Not going async, profile here. */ + START_PROFILE(SMBulogoffX); + reply_force_doserror(smb1req, ERRDOS, ERRnomem); END_PROFILE(SMBulogoffX); - exit_server(__location__ ": smbXsrv_session_logoff failed"); return; } - TALLOC_FREE(session); - - reply_outbuf(smb1req, 2, 0); - SSVAL(smb1req->outbuf, smb_vwv0, 0xff); /* andx chain ends */ - SSVAL(smb1req->outbuf, smb_vwv1, 0); /* no andx offset */ - - DEBUG(3, ("ulogoffX vuid=%llu\n", - (unsigned long long)smb1req->vuid)); - - END_PROFILE(SMBulogoffX); - smb1req->vuid = UID_FIELD_INVALID; + /* We're async. This will complete later. */ + tevent_req_set_callback(req, reply_ulogoffX_done, smb1req); + return; } -#if 0 struct reply_ulogoffX_state { struct tevent_queue *wait_queue; struct smbXsrv_session *session; @@ -2832,7 +2827,6 @@ static void reply_ulogoffX_done(struct tevent_req *req) smb_request_done(smb1req); END_PROFILE(SMBulogoffX); } -#endif /**************************************************************************** Reply to a mknew or a create. -- 2.20.1 From 1def1e3cce9f4d1adb33ad9c7927920621560349 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 10:59:16 -0700 Subject: [PATCH 17/39] s3: smbd: Add async internals of reply_exit(). Waits until all aio requests on all fsp's owned by this vuid are finished before returning to the client. Charges the profile time in the done function. Not strictly correct but better than the other SMB1 async code that double-charges profiling in both send and done at the moment. Done this way (commented out) so it is a clean diff and it's clear what is being added. A later 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 | 202 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 202 insertions(+) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index ea66604ab27..067a87e680f 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -5781,6 +5781,208 @@ void reply_exit(struct smb_request *req) return; } +#if 0 +struct reply_exit_state { + struct tevent_queue *wait_queue; +}; + +static void reply_exit_wait_done(struct tevent_req *subreq); + +/**************************************************************************** + Async SMB1 exit. + 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 *reply_exit_send(struct smb_request *smb1req) +{ + struct tevent_req *req; + struct reply_exit_state *state; + struct tevent_req *subreq; + files_struct *fsp; + struct smbd_server_connection *sconn = smb1req->sconn; + + /* Create this off the NULL context. We must clean up on return. */ + req = tevent_req_create(NULL, &state, + struct reply_exit_state); + if (req == NULL) { + return NULL; + } + state->wait_queue = tevent_queue_create(state, + "reply_exit_wait_queue"); + if (tevent_req_nomem(state->wait_queue, req)) { + TALLOC_FREE(req); + return NULL; + } + + for (fsp = sconn->files; fsp; fsp = fsp->next) { + if (fsp->file_pid != smb1req->smbpid) { + continue; + } + if (fsp->vuid != smb1req->vuid) { + continue; + } + /* + * Flag the file as close in progress. + * This will prevent any more IO being + * done on it. + */ + fsp->closing = true; + + 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, + 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, + 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, reply_exit_wait_done, req); + + return req; +} + +static void reply_exit_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 reply_exit_recv(struct tevent_req *req) +{ + return tevent_req_simple_recv_ntstatus(req); +} + +static void reply_exit_done(struct tevent_req *req) +{ + struct smb_request *smb1req = tevent_req_callback_data( + req, struct smb_request); + struct smbd_server_connection *sconn = smb1req->sconn; + struct smbXsrv_connection *xconn = smb1req->xconn; + NTTIME now = timeval_to_nttime(&smb1req->request_time); + struct smbXsrv_session *session = NULL; + files_struct *fsp, *next; + NTSTATUS status; + + /* + * Take the profile charge here. Not strictly + * correct but better than the other SMB1 async + * code that double-charges at the moment. + */ + START_PROFILE(SMBexit); + + status = reply_exit_recv(req); + TALLOC_FREE(req); + if (!NT_STATUS_IS_OK(status)) { + TALLOC_FREE(smb1req); + END_PROFILE(SMBexit); + exit_server(__location__ ": reply_exit_recv failed"); + return; + } + + /* + * Ensure the session is still valid. + */ + status = smb1srv_session_lookup(xconn, + smb1req->vuid, + now, + &session); + if (!NT_STATUS_IS_OK(status)) { + reply_force_doserror(smb1req, ERRSRV, ERRinvnid); + smb_request_done(smb1req); + END_PROFILE(SMBexit); + } + + /* + * Ensure the vuid is still valid - no one + * called reply_ulogoffX() in the meantime. + * reply_exit() doesn't have AS_USER set, so + * use set_current_user_info() directly. + * This is the same logic as in switch_message(). + */ + if (session->global->auth_session_info != NULL) { + set_current_user_info( + session->global->auth_session_info->unix_info->sanitized_username, + session->global->auth_session_info->unix_info->unix_name, + session->global->auth_session_info->info->domain_name); + } + + /* No more aio - do the actual closes. */ + for (fsp = sconn->files; fsp; fsp = next) { + bool ok; + next = fsp->next; + + if (fsp->file_pid != smb1req->smbpid) { + continue; + } + if (fsp->vuid != smb1req->vuid) { + continue; + } + if (!fsp->closing) { + continue; + } + + /* + * reply_exit() has the DO_CHDIR flag set. + */ + ok = chdir_current_service(fsp->conn); + if (!ok) { + reply_force_doserror(smb1req, ERRSRV, ERRinvnid); + smb_request_done(smb1req); + END_PROFILE(SMBexit); + } + close_file(NULL, fsp, SHUTDOWN_CLOSE); + } + + 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); + DBG_INFO("reply_exit complete\n"); + END_PROFILE(SMBexit); + return; +} +#endif + struct reply_close_state { files_struct *fsp; struct smb_request *smbreq; -- 2.20.1 From d5223d0e2e9e74857a323224942b0a22e4704256 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 11:01:05 -0700 Subject: [PATCH 18/39] s3: smbd: Remove old synchronous SMB1 reply_exit(). SMB1 exit is now fully async. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/reply.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 067a87e680f..16653432ace 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -5767,21 +5767,33 @@ void reply_flush(struct smb_request *req) conn POINTER CAN BE NULL HERE ! ****************************************************************************/ -void reply_exit(struct smb_request *req) -{ - START_PROFILE(SMBexit); - - file_close_pid(req->sconn, req->smbpid, req->vuid); +static struct tevent_req *reply_exit_send(struct smb_request *smb1req); +static void reply_exit_done(struct tevent_req *req); - reply_outbuf(req, 0, 0); +void reply_exit(struct smb_request *smb1req) +{ + struct tevent_req *req; - DEBUG(3,("exit\n")); + /* + * Don't setup the profile charge here, take + * it in reply_exit_done(). Not strictly correct + * but better than the other SMB1 async + * code that double-charges at the moment. + */ + req = reply_exit_send(smb1req); + if (req == NULL) { + /* Not going async, profile here. */ + START_PROFILE(SMBexit); + reply_force_doserror(smb1req, ERRDOS, ERRnomem); + END_PROFILE(SMBexit); + return; + } - END_PROFILE(SMBexit); + /* We're async. This will complete later. */ + tevent_req_set_callback(req, reply_exit_done, smb1req); return; } -#if 0 struct reply_exit_state { struct tevent_queue *wait_queue; }; @@ -5981,7 +5993,6 @@ static void reply_exit_done(struct tevent_req *req) END_PROFILE(SMBexit); return; } -#endif struct reply_close_state { files_struct *fsp; -- 2.20.1 From 29daefe6d47a1d8dbc4db951c42d64ea425b24cb Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 12 Mar 2020 11:02:19 -0700 Subject: [PATCH 19/39] s3: smbd: Remove file_close_pid(). The old synchronous reply_exit() was the only user. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/smbd/files.c | 17 ----------------- source3/smbd/proto.h | 2 -- 2 files changed, 19 deletions(-) diff --git a/source3/smbd/files.c b/source3/smbd/files.c index b06511147ab..a982c0a5980 100644 --- a/source3/smbd/files.c +++ b/source3/smbd/files.c @@ -188,23 +188,6 @@ void file_close_conn(connection_struct *conn) } } -/**************************************************************************** - Close all open files for a pid and a vuid. -****************************************************************************/ - -void file_close_pid(struct smbd_server_connection *sconn, uint16_t smbpid, - uint64_t vuid) -{ - files_struct *fsp, *next; - - for (fsp=sconn->files;fsp;fsp=next) { - next = fsp->next; - if ((fsp->file_pid == smbpid) && (fsp->vuid == vuid)) { - close_file(NULL, fsp, SHUTDOWN_CLOSE); - } - } -} - /**************************************************************************** Initialise file structures. ****************************************************************************/ diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 199f8544e01..0f773c06225 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -379,8 +379,6 @@ void fsp_set_gen_id(files_struct *fsp); NTSTATUS file_new(struct smb_request *req, connection_struct *conn, files_struct **result); void file_close_conn(connection_struct *conn); -void file_close_pid(struct smbd_server_connection *sconn, uint16_t smbpid, - uint64_t vuid); bool file_init_global(void); bool file_init(struct smbd_server_connection *sconn); void file_close_user(struct smbd_server_connection *sconn, uint64_t vuid); -- 2.20.1 From a8b6c427eb7165db4ebb0e7db6587c309c001c5d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:34:46 -0700 Subject: [PATCH 20/39] Revert "vfs_default: Protect vfs_getxattrat_done() from accessing a freed req pointer" This reverts commit 95cfcda13fe9a70b9955a7c44173d619eacb34c1. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index cf24e66e048..bd39eb47e9a 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -3289,15 +3289,6 @@ struct vfswrap_getxattrat_state { static int vfswrap_getxattrat_state_destructor( struct vfswrap_getxattrat_state *state) { - /* - * This destructor only gets called if the request is still - * in flight, which is why we deny it by returning -1. We - * also set the req pointer to NULL so the _done function - * can detect the caller doesn't want the result anymore. - * - * Forcing the fsp closed by a SHUTDOWN_CLOSE can cause this. - */ - state->req = NULL; return -1; } @@ -3528,16 +3519,6 @@ static void vfswrap_getxattrat_done(struct tevent_req *subreq) TALLOC_FREE(subreq); SMBPROFILE_BYTES_ASYNC_END(state->profile_bytes); talloc_set_destructor(state, NULL); - if (req == NULL) { - /* - * We were shutdown closed in flight. No one wants the result, - * and state has been reparented to the NULL context, so just - * free it so we don't leak memory. - */ - DBG_NOTICE("getxattr request abandoned in flight\n"); - TALLOC_FREE(state); - return; - } if (ret != 0) { if (ret != EAGAIN) { tevent_req_error(req, ret); -- 2.20.1 From 74a69bebf883e1d07178f8caf0f2e4f106b340d0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:35:04 -0700 Subject: [PATCH 21/39] Revert "vfs_default: pass in state as the callback data to the subreq" This reverts commit 0e894f3e48285415f72cf7a68e26f1802fe8045d. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index bd39eb47e9a..fac7fa30ab7 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -3408,7 +3408,7 @@ static struct tevent_req *vfswrap_getxattrat_send( if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, vfswrap_getxattrat_done, state); + tevent_req_set_callback(subreq, vfswrap_getxattrat_done, req); talloc_set_destructor(state, vfswrap_getxattrat_state_destructor); @@ -3503,9 +3503,10 @@ end_profile: static void vfswrap_getxattrat_done(struct tevent_req *subreq) { - struct vfswrap_getxattrat_state *state = tevent_req_callback_data( - subreq, struct vfswrap_getxattrat_state); - struct tevent_req *req = state->req; + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct vfswrap_getxattrat_state *state = tevent_req_data( + req, struct vfswrap_getxattrat_state); int ret; bool ok; -- 2.20.1 From ab7ea115ccdcf7171b0235b18a5d444e72bd10e0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:35:21 -0700 Subject: [PATCH 22/39] Revert "s3: VFS: vfs_glusterfs. Protect vfs_gluster_fsync_done() from accessing a freed req pointer." This reverts commit 9ecbda263f102a24257fd47142b7c24d1f429d8d. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index b5300282b7b..4706e6f9189 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -1197,15 +1197,6 @@ static void vfs_gluster_fsync_do(void *private_data) static int vfs_gluster_fsync_state_destructor(struct vfs_gluster_fsync_state *state) { - /* - * This destructor only gets called if the request is still - * in flight, which is why we deny it by returning -1. We - * also set the req pointer to NULL so the _done function - * can detect the caller doesn't want the result anymore. - * - * Forcing the fsp closed by a SHUTDOWN_CLOSE can cause this. - */ - state->req = NULL; return -1; } @@ -1220,17 +1211,6 @@ static void vfs_gluster_fsync_done(struct tevent_req *subreq) TALLOC_FREE(subreq); SMBPROFILE_BYTES_ASYNC_END(state->profile_bytes); talloc_set_destructor(state, NULL); - if (req == NULL) { - /* - * We were shutdown closed in flight. No one - * wants the result, and state has been reparented - * to the NULL context, so just free it so we - * don't leak memory. - */ - DBG_NOTICE("gluster fsync request abandoned in flight\n"); - TALLOC_FREE(state); - return; - } if (ret != 0) { if (ret != EAGAIN) { tevent_req_error(req, ret); -- 2.20.1 From 274ec61176f8bc7d2dceb701506d8db137621611 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:35:44 -0700 Subject: [PATCH 23/39] Revert "s3: VFS: vfs_glusterfs. Pass in struct vfs_gluster_fsync_state as the callback data to the subreq." This reverts commit cdde55a69d0dacd2f9939c2f00cd356c0186f791. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index 4706e6f9189..d5d402d72ab 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -1158,7 +1158,7 @@ static struct tevent_req *vfs_gluster_fsync_send(struct vfs_handle_struct if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, vfs_gluster_fsync_done, state); + tevent_req_set_callback(subreq, vfs_gluster_fsync_done, req); talloc_set_destructor(state, vfs_gluster_fsync_state_destructor); @@ -1202,9 +1202,10 @@ static int vfs_gluster_fsync_state_destructor(struct vfs_gluster_fsync_state *st static void vfs_gluster_fsync_done(struct tevent_req *subreq) { - struct vfs_gluster_fsync_state *state = tevent_req_callback_data( - subreq, struct vfs_gluster_fsync_state); - struct tevent_req *req = state->req; + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct vfs_gluster_fsync_state *state = tevent_req_data( + req, struct vfs_gluster_fsync_state); int ret; ret = pthreadpool_tevent_job_recv(subreq); -- 2.20.1 From e6db4bb4e2c64cde1d06233224ba63ba64bf8549 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:36:03 -0700 Subject: [PATCH 24/39] Revert "s3: VFS: vfs_glusterfs: Add tevent_req pointer to state struct in vfs_gluster_fsync_state." This reverts commit c0c088b1b786f0ba248960114191277e91bbba2f. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index d5d402d72ab..4e978f168d6 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -1114,7 +1114,6 @@ static int vfs_gluster_renameat(struct vfs_handle_struct *handle, } struct vfs_gluster_fsync_state { - struct tevent_req *req; ssize_t ret; glfs_fd_t *fd; @@ -1145,7 +1144,6 @@ static struct tevent_req *vfs_gluster_fsync_send(struct vfs_handle_struct return NULL; } - state->req = req; state->ret = -1; state->fd = glfd; -- 2.20.1 From 1bc034f1bb5f1e1a1134e5aca8f9caa16311bd71 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:36:19 -0700 Subject: [PATCH 25/39] Revert "s3: VFS: vfs_glusterfs. Protect vfs_gluster_pwrite_done() from accessing a freed req pointer." This reverts commit 67910c751c9f5ce8cdd1e57b34e51e5b7163838b. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index 4e978f168d6..52c33725b8d 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -967,15 +967,6 @@ static void vfs_gluster_pwrite_do(void *private_data) static int vfs_gluster_pwrite_state_destructor(struct vfs_gluster_pwrite_state *state) { - /* - * This destructor only gets called if the request is still - * in flight, which is why we deny it by returning -1. We - * also set the req pointer to NULL so the _done function - * can detect the caller doesn't want the result anymore. - * - * Forcing the fsp closed by a SHUTDOWN_CLOSE can cause this. - */ - state->req = NULL; return -1; } @@ -990,17 +981,6 @@ static void vfs_gluster_pwrite_done(struct tevent_req *subreq) TALLOC_FREE(subreq); SMBPROFILE_BYTES_ASYNC_END(state->profile_bytes); talloc_set_destructor(state, NULL); - if (req == NULL) { - /* - * We were shutdown closed in flight. No one - * wants the result, and state has been reparented - * to the NULL context, so just free it so we - * don't leak memory. - */ - DBG_NOTICE("gluster pwrite request abandoned in flight\n"); - TALLOC_FREE(state); - return; - } if (ret != 0) { if (ret != EAGAIN) { tevent_req_error(req, ret); -- 2.20.1 From 0b4e3dc55eb0c7a562c1f74dd88300cbbcdf384c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:36:39 -0700 Subject: [PATCH 26/39] Revert "s3: VFS: vfs_glusterfs. Pass in struct vfs_gluster_pwrite_state as the callback data to the subreq." This reverts commit 3357a77d0823eddc1b0db68cfa251a0d54058c88. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index 52c33725b8d..456e0c8a498 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -926,7 +926,7 @@ static struct tevent_req *vfs_gluster_pwrite_send(struct vfs_handle_struct if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, vfs_gluster_pwrite_done, state); + tevent_req_set_callback(subreq, vfs_gluster_pwrite_done, req); talloc_set_destructor(state, vfs_gluster_pwrite_state_destructor); @@ -972,9 +972,10 @@ static int vfs_gluster_pwrite_state_destructor(struct vfs_gluster_pwrite_state * static void vfs_gluster_pwrite_done(struct tevent_req *subreq) { - struct vfs_gluster_pwrite_state *state = tevent_req_callback_data( - subreq, struct vfs_gluster_pwrite_state); - struct tevent_req *req = state->req; + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct vfs_gluster_pwrite_state *state = tevent_req_data( + req, struct vfs_gluster_pwrite_state); int ret; ret = pthreadpool_tevent_job_recv(subreq); -- 2.20.1 From cc9083c51e109a65a531ecaefc48f91f55ff4eb0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:36:59 -0700 Subject: [PATCH 27/39] Revert "s3: VFS: vfs_glusterfs: Add tevent_req pointer to state struct in vfs_gluster_pwrite_state." This reverts commit 058a7effd00b47e4778f7d680cc9c2a7d40d5fa8. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index 456e0c8a498..7924f123cca 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -873,7 +873,6 @@ static ssize_t vfs_gluster_pread_recv(struct tevent_req *req, } struct vfs_gluster_pwrite_state { - struct tevent_req *req; ssize_t ret; glfs_fd_t *fd; const void *buf; @@ -909,7 +908,6 @@ static struct tevent_req *vfs_gluster_pwrite_send(struct vfs_handle_struct return NULL; } - state->req = req; state->ret = -1; state->fd = glfd; state->buf = data; -- 2.20.1 From 34fdd76b16f0a55ed2993ff4a6cb29846daa1eb9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:37:16 -0700 Subject: [PATCH 28/39] Revert "s3: VFS: vfs_glusterfs. Protect vfs_gluster_pread_done() from accessing a freed req pointer." This reverts commit 99283871c5230e640a8102943ebed685459ed9af. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index 7924f123cca..84b284152c6 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -807,15 +807,6 @@ static void vfs_gluster_pread_do(void *private_data) static int vfs_gluster_pread_state_destructor(struct vfs_gluster_pread_state *state) { - /* - * This destructor only gets called if the request is still - * in flight, which is why we deny it by returning -1. We - * also set the req pointer to NULL so the _done function - * can detect the caller doesn't want the result anymore. - * - * Forcing the fsp closed by a SHUTDOWN_CLOSE can cause this. - */ - state->req = NULL; return -1; } @@ -830,17 +821,6 @@ static void vfs_gluster_pread_done(struct tevent_req *subreq) TALLOC_FREE(subreq); SMBPROFILE_BYTES_ASYNC_END(state->profile_bytes); talloc_set_destructor(state, NULL); - if (req == NULL) { - /* - * We were shutdown closed in flight. No one - * wants the result, and state has been reparented - * to the NULL context, so just free it so we - * don't leak memory. - */ - DBG_NOTICE("gluster pread request abandoned in flight\n"); - TALLOC_FREE(state); - return; - } if (ret != 0) { if (ret != EAGAIN) { tevent_req_error(req, ret); -- 2.20.1 From 26c9ce9ed6be62868c04e128e1e9a98afc3e698d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:37:30 -0700 Subject: [PATCH 29/39] Revert "s3: VFS: vfs_glusterfs. Pass in struct vfs_gluster_pread_state as the callback data to the subreq." This reverts commit c6c4e2de22cd3d84f45f5c21e6b09b09274f7f7b. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index 84b284152c6..6598aadad17 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -766,7 +766,7 @@ static struct tevent_req *vfs_gluster_pread_send(struct vfs_handle_struct if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, vfs_gluster_pread_done, state); + tevent_req_set_callback(subreq, vfs_gluster_pread_done, req); talloc_set_destructor(state, vfs_gluster_pread_state_destructor); @@ -812,9 +812,10 @@ static int vfs_gluster_pread_state_destructor(struct vfs_gluster_pread_state *st static void vfs_gluster_pread_done(struct tevent_req *subreq) { - struct vfs_gluster_pread_state *state = tevent_req_callback_data( - subreq, struct vfs_gluster_pread_state); - struct tevent_req *req = state->req; + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct vfs_gluster_pread_state *state = tevent_req_data( + req, struct vfs_gluster_pread_state); int ret; ret = pthreadpool_tevent_job_recv(subreq); -- 2.20.1 From 7201295c4993ebfc54457eca1aac31a020b835dd Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:37:48 -0700 Subject: [PATCH 30/39] Revert "s3: VFS: vfs_glusterfs: Add tevent_req pointer to state struct in vfs_gluster_pread_state." This reverts commit 0e3dc0078ebd6aa79553bf2afa8e72945e23dfb0. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_glusterfs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c index 6598aadad17..d4b68fba376 100644 --- a/source3/modules/vfs_glusterfs.c +++ b/source3/modules/vfs_glusterfs.c @@ -713,7 +713,6 @@ static ssize_t vfs_gluster_pread(struct vfs_handle_struct *handle, } struct vfs_gluster_pread_state { - struct tevent_req *req; ssize_t ret; glfs_fd_t *fd; void *buf; @@ -749,7 +748,6 @@ static struct tevent_req *vfs_gluster_pread_send(struct vfs_handle_struct return NULL; } - state->req = req; state->ret = -1; state->fd = glfd; state->buf = data; -- 2.20.1 From ca99853bfebb752c595a1d98538719957e447610 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:38:03 -0700 Subject: [PATCH 31/39] Revert "s3: VFS: vfs_default. Protect vfs_fsync_done() from accessing a freed req pointer." This reverts commit 18671534e42f66b904e51c3fbe887e998ff79493. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index fac7fa30ab7..f9d958a003d 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1151,15 +1151,6 @@ static void vfs_fsync_do(void *private_data) static int vfs_fsync_state_destructor(struct vfswrap_fsync_state *state) { - /* - * This destructor only gets called if the request is still - * in flight, which is why we deny it by returning -1. We - * also set the req pointer to NULL so the _done function - * can detect the caller doesn't want the result anymore. - * - * Forcing the fsp closed by a SHUTDOWN_CLOSE can cause this. - */ - state->req = NULL; return -1; } @@ -1174,17 +1165,6 @@ static void vfs_fsync_done(struct tevent_req *subreq) TALLOC_FREE(subreq); SMBPROFILE_BYTES_ASYNC_END(state->profile_bytes); talloc_set_destructor(state, NULL); - if (req == NULL) { - /* - * We were shutdown closed in flight. No one - * wants the result, and state has been reparented - * to the NULL context, so just free it so we - * don't leak memory. - */ - DBG_NOTICE("fsync request abandoned in flight\n"); - TALLOC_FREE(state); - return; - } if (ret != 0) { if (ret != EAGAIN) { tevent_req_error(req, ret); -- 2.20.1 From d195aa5346b3a4d9f7b033a1a31e6f05474649f7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:38:18 -0700 Subject: [PATCH 32/39] Revert "s3: VFS: vfs_default. Pass in struct vfswrap_fsync_state as the callback data to the subreq." This reverts commit d623779913e0d4a46d7e299dc41b5c83cb127872. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index f9d958a003d..28b8c04dee4 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1116,7 +1116,7 @@ static struct tevent_req *vfswrap_fsync_send(struct vfs_handle_struct *handle, if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, vfs_fsync_done, state); + tevent_req_set_callback(subreq, vfs_fsync_done, req); talloc_set_destructor(state, vfs_fsync_state_destructor); @@ -1156,9 +1156,10 @@ static int vfs_fsync_state_destructor(struct vfswrap_fsync_state *state) static void vfs_fsync_done(struct tevent_req *subreq) { - struct vfswrap_fsync_state *state = tevent_req_callback_data( - subreq, struct vfswrap_fsync_state); - struct tevent_req *req = state->req; + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct vfswrap_fsync_state *state = tevent_req_data( + req, struct vfswrap_fsync_state); int ret; ret = pthreadpool_tevent_job_recv(subreq); -- 2.20.1 From d725d5e6c84b8ec7fa2cc574a017d040a621370f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:39:07 -0700 Subject: [PATCH 33/39] Revert "s3: VFS: vfs_default: Add tevent_req pointer to state struct in vfswrap_fsync_state." This reverts commit 4adde71b99d4ab09914072458329d5f1008b77e3. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 28b8c04dee4..3425ee31dcb 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1078,7 +1078,6 @@ static ssize_t vfswrap_pwrite_recv(struct tevent_req *req, } struct vfswrap_fsync_state { - struct tevent_req *req; ssize_t ret; int fd; @@ -1103,7 +1102,6 @@ static struct tevent_req *vfswrap_fsync_send(struct vfs_handle_struct *handle, return NULL; } - state->req = req; state->ret = -1; state->fd = fsp->fh->fd; -- 2.20.1 From 3dde94782f5bdf1eec55c05f0d3c62d235cec7e5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:39:31 -0700 Subject: [PATCH 34/39] Revert "s3: VFS: vfs_default. Protect vfs_pwrite_done() from accessing a freed req pointer." This reverts commit c8cd93dd54cb9f78665928d4bc8fcc3baf084b6f. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 3425ee31dcb..641764e41f1 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1012,15 +1012,6 @@ static void vfs_pwrite_do(void *private_data) static int vfs_pwrite_state_destructor(struct vfswrap_pwrite_state *state) { - /* - * This destructor only gets called if the request is still - * in flight, which is why we deny it by returning -1. We - * also set the req pointer to NULL so the _done function - * can detect the caller doesn't want the result anymore. - * - * Forcing the fsp closed by a SHUTDOWN_CLOSE can cause this. - */ - state->req = NULL; return -1; } @@ -1035,17 +1026,6 @@ static void vfs_pwrite_done(struct tevent_req *subreq) TALLOC_FREE(subreq); SMBPROFILE_BYTES_ASYNC_END(state->profile_bytes); talloc_set_destructor(state, NULL); - if (req == NULL) { - /* - * We were shutdown closed in flight. No one - * wants the result, and state has been reparented - * to the NULL context, so just free it so we - * don't leak memory. - */ - DBG_NOTICE("pwrite request abandoned in flight\n"); - TALLOC_FREE(state); - return; - } if (ret != 0) { if (ret != EAGAIN) { tevent_req_error(req, ret); -- 2.20.1 From ef9fd70376c73b7afe352d784a6143ff53553b61 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:39:46 -0700 Subject: [PATCH 35/39] Revert "s3: VFS: vfs_default. Pass in struct vfswrap_pwrite_state as the callback data to the subreq." This reverts commit 13e25d68385aa951115e0e063ec6a9a281fea4a4. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 641764e41f1..cbc8335cd12 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -976,7 +976,7 @@ static struct tevent_req *vfswrap_pwrite_send(struct vfs_handle_struct *handle, if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, vfs_pwrite_done, state); + tevent_req_set_callback(subreq, vfs_pwrite_done, req); talloc_set_destructor(state, vfs_pwrite_state_destructor); @@ -1017,9 +1017,10 @@ static int vfs_pwrite_state_destructor(struct vfswrap_pwrite_state *state) static void vfs_pwrite_done(struct tevent_req *subreq) { - struct vfswrap_pwrite_state *state = tevent_req_callback_data( - subreq, struct vfswrap_pwrite_state); - struct tevent_req *req = state->req; + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct vfswrap_pwrite_state *state = tevent_req_data( + req, struct vfswrap_pwrite_state); int ret; ret = pthreadpool_tevent_job_recv(subreq); -- 2.20.1 From 36b8fa57afcb041223d90443eeff637e0d131ab7 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:40:02 -0700 Subject: [PATCH 36/39] Revert "s3: VFS: vfs_default: Add tevent_req pointer to state struct in vfswrap_pwrite_state." This reverts commit 86cc7439501ab9b9eb018a18dbbef9567eb9b6f9. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index cbc8335cd12..21bc9c7adf7 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -929,7 +929,6 @@ static ssize_t vfswrap_pread_recv(struct tevent_req *req, } struct vfswrap_pwrite_state { - struct tevent_req *req; ssize_t ret; int fd; const void *buf; @@ -959,7 +958,6 @@ static struct tevent_req *vfswrap_pwrite_send(struct vfs_handle_struct *handle, return NULL; } - state->req = req; state->ret = -1; state->fd = fsp->fh->fd; state->buf = data; -- 2.20.1 From 8cccf2f0c09692c4a69a65d65d25b5dc9ac6cf6d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:40:19 -0700 Subject: [PATCH 37/39] Revert "s3: VFS: vfs_default. Protect vfs_pread_done() from accessing a freed req pointer." This reverts commit b9ad06079fe362385cc4c77f8e8d54f5f74d6db6. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 21bc9c7adf7..b8c36180b7c 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -863,15 +863,6 @@ static void vfs_pread_do(void *private_data) static int vfs_pread_state_destructor(struct vfswrap_pread_state *state) { - /* - * This destructor only gets called if the request is still - * in flight, which is why we deny it by returning -1. We - * also set the req pointer to NULL so the _done function - * can detect the caller doesn't want the result anymore. - * - * Forcing the fsp closed by a SHUTDOWN_CLOSE can cause this. - */ - state->req = NULL; return -1; } @@ -886,17 +877,6 @@ static void vfs_pread_done(struct tevent_req *subreq) TALLOC_FREE(subreq); SMBPROFILE_BYTES_ASYNC_END(state->profile_bytes); talloc_set_destructor(state, NULL); - if (req == NULL) { - /* - * We were shutdown closed in flight. No one - * wants the result, and state has been reparented - * to the NULL context, so just free it so we - * don't leak memory. - */ - DBG_NOTICE("pread request abandoned in flight\n"); - TALLOC_FREE(state); - return; - } if (ret != 0) { if (ret != EAGAIN) { tevent_req_error(req, ret); -- 2.20.1 From a4f93ee13b388b45d8e678026d2a6d68cd32c286 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:40:33 -0700 Subject: [PATCH 38/39] Revert "s3: VFS: vfs_default. Pass in struct vfswrap_pread_state as the callback data to the subreq." This reverts commit e102908f112866d657b8c0cd6a5b217d070210c8. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index b8c36180b7c..4bb4adf5f7e 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -827,7 +827,7 @@ static struct tevent_req *vfswrap_pread_send(struct vfs_handle_struct *handle, if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, vfs_pread_done, state); + tevent_req_set_callback(subreq, vfs_pread_done, req); talloc_set_destructor(state, vfs_pread_state_destructor); @@ -868,9 +868,10 @@ static int vfs_pread_state_destructor(struct vfswrap_pread_state *state) static void vfs_pread_done(struct tevent_req *subreq) { - struct vfswrap_pread_state *state = tevent_req_callback_data( - subreq, struct vfswrap_pread_state); - struct tevent_req *req = state->req; + struct tevent_req *req = tevent_req_callback_data( + subreq, struct tevent_req); + struct vfswrap_pread_state *state = tevent_req_data( + req, struct vfswrap_pread_state); int ret; ret = pthreadpool_tevent_job_recv(subreq); -- 2.20.1 From 9e48e080a555343136160a18843956f6115218ba Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 13 Mar 2020 10:40:50 -0700 Subject: [PATCH 39/39] Revert "s3: VFS: vfs_default: Add tevent_req pointer to state struct in vfswrap_pread_state." This reverts commit 594a435b33e8447625ca83b50daec2d08cf66d64. Now we wait for all aio to finish on all SHUTDOWN_CLOSE cases, this is no longer needed. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 Signed-off-by: Jeremy Allison --- source3/modules/vfs_default.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 4bb4adf5f7e..a30f3ba1d31 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -780,7 +780,6 @@ static ssize_t vfswrap_pwrite(vfs_handle_struct *handle, files_struct *fsp, cons } struct vfswrap_pread_state { - struct tevent_req *req; ssize_t ret; int fd; void *buf; @@ -810,7 +809,6 @@ static struct tevent_req *vfswrap_pread_send(struct vfs_handle_struct *handle, return NULL; } - state->req = req; state->ret = -1; state->fd = fsp->fh->fd; state->buf = data; -- 2.20.1