The Samba-Bugzilla – Attachment 15870 Details for
Bug 14301
smbd panic on force-close share during async io
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am fix for 4.12.next.
14301-4.12.patch (text/plain), 72.51 KB, created by
Jeremy Allison
on 2020-03-21 00:09:33 UTC
(
hide
)
Description:
git-am fix for 4.12.next.
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2020-03-21 00:09:33 UTC
Size:
72.51 KB
patch
obsolete
>From 9c83e302052f3fd8b91a27d4467765f72fa81945 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 4 Mar 2020 13:29:08 -0800 >Subject: [PATCH 01/25] s3: VFS: vfs_aio_pthread. Fix leak of state struct on > error. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit a1e247c3ba579ecc6ee03f5aad9679ed79fac5ac) >--- > source3/modules/vfs_aio_pthread.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/source3/modules/vfs_aio_pthread.c b/source3/modules/vfs_aio_pthread.c >index d13ce2fdc63..37ba0c2c8a2 100644 >--- a/source3/modules/vfs_aio_pthread.c >+++ b/source3/modules/vfs_aio_pthread.c >@@ -308,6 +308,7 @@ static int open_async(const files_struct *fsp, > fsp->conn->sconn->pool, > aio_open_worker, opd); > if (subreq == NULL) { >+ TALLOC_FREE(opd); > return -1; > } > tevent_req_set_callback(subreq, aio_open_handle_completion, opd); >-- >2.20.1 > > >From 7aca70199532a49e9c6e6402a61ee19dbd5020d5 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 4 Mar 2020 13:47:13 -0800 >Subject: [PATCH 02/25] s3: VFS: vfs_aio_pthread: Replace state destructor with > explicitly called teardown function. > >This will allow repurposing a real destructor to allow >connections structs to be freed whilst the aio open >request is in flight. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 8db831a318cd4a10ec9c1d629ebff4ca35b8acfe) >--- > source3/modules/vfs_aio_pthread.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > >diff --git a/source3/modules/vfs_aio_pthread.c b/source3/modules/vfs_aio_pthread.c >index 37ba0c2c8a2..820e1b89c44 100644 >--- a/source3/modules/vfs_aio_pthread.c >+++ b/source3/modules/vfs_aio_pthread.c >@@ -62,6 +62,7 @@ struct aio_open_private_data { > static struct aio_open_private_data *open_pd_list; > > static void aio_open_do(struct aio_open_private_data *opd); >+static void opd_free(struct aio_open_private_data *opd); > > /************************************************************************ > Find the open private data by mid. >@@ -145,7 +146,7 @@ static void aio_open_handle_completion(struct tevent_req *subreq) > close(opd->ret_fd); > opd->ret_fd = -1; > } >- TALLOC_FREE(opd); >+ opd_free(opd); > } > } > >@@ -207,16 +208,16 @@ static void aio_open_do(struct aio_open_private_data *opd) > } > > /************************************************************************ >- Open private data destructor. >+ Open private data teardown. > ***********************************************************************/ > >-static int opd_destructor(struct aio_open_private_data *opd) >+static void opd_free(struct aio_open_private_data *opd) > { > if (opd->dir_fd != -1) { > close(opd->dir_fd); > } > DLIST_REMOVE(open_pd_list, opd); >- return 0; >+ TALLOC_FREE(opd); > } > > /************************************************************************ >@@ -250,7 +251,7 @@ static struct aio_open_private_data *create_private_open_data(const files_struct > /* Copy our current credentials. */ > opd->ux_tok = copy_unix_token(opd, get_current_utok(fsp->conn)); > if (opd->ux_tok == NULL) { >- TALLOC_FREE(opd); >+ opd_free(opd); > return NULL; > } > >@@ -262,12 +263,12 @@ static struct aio_open_private_data *create_private_open_data(const files_struct > fsp->fsp_name->base_name, > &opd->dname, > &fname) == false) { >- TALLOC_FREE(opd); >+ opd_free(opd); > return NULL; > } > opd->fname = talloc_strdup(opd, fname); > if (opd->fname == NULL) { >- TALLOC_FREE(opd); >+ opd_free(opd); > return NULL; > } > >@@ -277,11 +278,10 @@ static struct aio_open_private_data *create_private_open_data(const files_struct > opd->dir_fd = open(opd->dname, O_RDONLY); > #endif > if (opd->dir_fd == -1) { >- TALLOC_FREE(opd); >+ opd_free(opd); > return NULL; > } > >- talloc_set_destructor(opd, opd_destructor); > DLIST_ADD_END(open_pd_list, opd); > return opd; > } >@@ -308,7 +308,7 @@ static int open_async(const files_struct *fsp, > fsp->conn->sconn->pool, > aio_open_worker, opd); > if (subreq == NULL) { >- TALLOC_FREE(opd); >+ opd_free(opd); > return -1; > } > tevent_req_set_callback(subreq, aio_open_handle_completion, opd); >@@ -365,7 +365,7 @@ static bool find_completed_open(files_struct *fsp, > smb_fname_str_dbg(fsp->fsp_name))); > > /* Now we can free the opd. */ >- TALLOC_FREE(opd); >+ opd_free(opd); > return true; > } > >-- >2.20.1 > > >From 3049ab93121de4be72dbcc315178658ca392a6e3 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 4 Mar 2020 16:39:39 -0800 >Subject: [PATCH 03/25] s3: VFS: vfs_aio_pthread. Move xconn into state struct > (opd). > >We will need this in future to cause a pending open to >be rescheduled after the connection struct we're using >has been shut down with an aio open in flight. This will >allow a correct error reply to an awaiting client. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit ddb9038fe776b1d8239e563a4c9a70b4097645f3) >--- > source3/modules/vfs_aio_pthread.c | 17 ++++++++--------- > 1 file changed, 8 insertions(+), 9 deletions(-) > >diff --git a/source3/modules/vfs_aio_pthread.c b/source3/modules/vfs_aio_pthread.c >index 820e1b89c44..d5919f83b3f 100644 >--- a/source3/modules/vfs_aio_pthread.c >+++ b/source3/modules/vfs_aio_pthread.c >@@ -51,6 +51,7 @@ struct aio_open_private_data { > const char *fname; > char *dname; > connection_struct *conn; >+ struct smbXsrv_connection *xconn; > const struct security_unix_token *ux_tok; > uint64_t initial_allocation_size; > /* Returns. */ >@@ -91,7 +92,6 @@ static void aio_open_handle_completion(struct tevent_req *subreq) > tevent_req_callback_data(subreq, > struct aio_open_private_data); > int ret; >- struct smbXsrv_connection *xconn; > > ret = pthreadpool_tevent_job_recv(subreq); > TALLOC_FREE(subreq); >@@ -128,15 +128,8 @@ static void aio_open_handle_completion(struct tevent_req *subreq) > > opd->in_progress = false; > >- /* >- * TODO: In future we need a proper algorithm >- * to find the correct connection for a fsp. >- * For now we only have one connection, so this is correct... >- */ >- xconn = opd->conn->sconn->client->connections; >- > /* Find outstanding event and reschedule. */ >- if (!schedule_deferred_open_message_smb(xconn, opd->mid)) { >+ if (!schedule_deferred_open_message_smb(opd->xconn, opd->mid)) { > /* > * Outstanding event didn't exist or was > * cancelled. Free up the fd and throw >@@ -245,6 +238,12 @@ static struct aio_open_private_data *create_private_open_data(const files_struct > .mid = fsp->mid, > .in_progress = true, > .conn = fsp->conn, >+ /* >+ * TODO: In future we need a proper algorithm >+ * to find the correct connection for a fsp. >+ * For now we only have one connection, so this is correct... >+ */ >+ .xconn = fsp->conn->sconn->client->connections, > .initial_allocation_size = fsp->initial_allocation_size, > }; > >-- >2.20.1 > > >From 758022992f172496cf8ff7afb620c720d44dad6c Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 6 Mar 2020 09:30:26 -0800 >Subject: [PATCH 04/25] s3: VFS: vfs_aio_pthread: Add a talloc context > parameter to create_private_open_data(). > >Pass in NULL for now so no behavior change. >We will be changing this from NULL to fsp->conn in a later commit. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit e566066605981549b670a5392683fbd81ce93d18) >--- > source3/modules/vfs_aio_pthread.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > >diff --git a/source3/modules/vfs_aio_pthread.c b/source3/modules/vfs_aio_pthread.c >index d5919f83b3f..be84e90cfb2 100644 >--- a/source3/modules/vfs_aio_pthread.c >+++ b/source3/modules/vfs_aio_pthread.c >@@ -217,11 +217,12 @@ static void opd_free(struct aio_open_private_data *opd) > Create and initialize a private data struct for async open. > ***********************************************************************/ > >-static struct aio_open_private_data *create_private_open_data(const files_struct *fsp, >+static struct aio_open_private_data *create_private_open_data(TALLOC_CTX *ctx, >+ const files_struct *fsp, > int flags, > mode_t mode) > { >- struct aio_open_private_data *opd = talloc_zero(NULL, >+ struct aio_open_private_data *opd = talloc_zero(ctx, > struct aio_open_private_data); > const char *fname = NULL; > >@@ -296,7 +297,7 @@ static int open_async(const files_struct *fsp, > struct aio_open_private_data *opd = NULL; > struct tevent_req *subreq = NULL; > >- opd = create_private_open_data(fsp, flags, mode); >+ opd = create_private_open_data(NULL, fsp, flags, mode); > if (opd == NULL) { > DEBUG(10, ("open_async: Could not create private data.\n")); > return -1; >-- >2.20.1 > > >From b5b04c88203ec9ec6d2e507695dccba2efe8f9fd Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 5 Mar 2020 10:22:00 -0800 >Subject: [PATCH 05/25] s3: VFS: vfs_aio_pthread: Make aio opens safe against > connection teardown. > >Allocate state off fsp->conn, not NULL, and add a destructor >that catches deallocation of conn which happens >on connection shutdown or force close. > >Note - We don't allocate off fsp as the passed in >fsp will get freed once we return EINPROGRESS/NT_STATUS_MORE_PROCESSING_REQUIRED. >A new fsp pointer gets allocated on every re-run of the >open code path. > >The destructor allows us to NULL out the saved conn struct pointer >when conn is deallocated so we know not to access deallocated memory. >This matches the async teardown code changes for bug #14301 >in pread/pwrite/fsync vfs_default.c and vfs_glusterfs.c > >state is still correctly deallocated in all code >paths so no memory leaks. > >This allows us to safely complete when the openat() >returns and then return the error NT_STATUS_NETWORK_NAME_DELETED >to the client open request. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> >(cherry picked from commit 6b567e0c138d1cf2bcf58c84872ed2b0e89d628d) >--- > source3/modules/vfs_aio_pthread.c | 66 ++++++++++++++++++++++++++++++- > 1 file changed, 65 insertions(+), 1 deletion(-) > >diff --git a/source3/modules/vfs_aio_pthread.c b/source3/modules/vfs_aio_pthread.c >index be84e90cfb2..1ccf89a6d8c 100644 >--- a/source3/modules/vfs_aio_pthread.c >+++ b/source3/modules/vfs_aio_pthread.c >@@ -95,6 +95,37 @@ static void aio_open_handle_completion(struct tevent_req *subreq) > > ret = pthreadpool_tevent_job_recv(subreq); > TALLOC_FREE(subreq); >+ >+ /* >+ * We're no longer in flight. Remove the >+ * destructor used to preserve opd so >+ * a talloc_free actually removes it. >+ */ >+ talloc_set_destructor(opd, NULL); >+ >+ if (opd->conn == 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("aio open request for %s/%s abandoned in flight\n", >+ opd->dname, >+ opd->fname); >+ if (opd->ret_fd != -1) { >+ close(opd->ret_fd); >+ opd->ret_fd = -1; >+ } >+ /* >+ * Find outstanding event and reschedule so the client >+ * gets an error message return from the open. >+ */ >+ schedule_deferred_open_message_smb(opd->xconn, opd->mid); >+ opd_free(opd); >+ return; >+ } >+ > if (ret != 0) { > bool ok; > >@@ -286,6 +317,22 @@ static struct aio_open_private_data *create_private_open_data(TALLOC_CTX *ctx, > return opd; > } > >+static int opd_inflight_destructor(struct aio_open_private_data *opd) >+{ >+ /* >+ * Setting conn to NULL allows us to >+ * discover the connection was torn >+ * down which kills the fsp that owns >+ * opd. >+ */ >+ DBG_NOTICE("aio open request for %s/%s cancelled\n", >+ opd->dname, >+ opd->fname); >+ opd->conn = NULL; >+ /* Don't let opd go away. */ >+ return -1; >+} >+ > /***************************************************************** > Setup an async open. > *****************************************************************/ >@@ -297,7 +344,18 @@ static int open_async(const files_struct *fsp, > struct aio_open_private_data *opd = NULL; > struct tevent_req *subreq = NULL; > >- opd = create_private_open_data(NULL, fsp, flags, mode); >+ /* >+ * Allocate off fsp->conn, not NULL or fsp. As we're going >+ * async fsp will get talloc_free'd when we return >+ * EINPROGRESS/NT_STATUS_MORE_PROCESSING_REQUIRED. A new fsp >+ * pointer gets allocated on every re-run of the >+ * open code path. Allocating on fsp->conn instead >+ * of NULL allows use to get notified via destructor >+ * if the conn is force-closed or we shutdown. >+ * opd is always safely freed in all codepath so no >+ * memory leaks. >+ */ >+ opd = create_private_open_data(fsp->conn, fsp, flags, mode); > if (opd == NULL) { > DEBUG(10, ("open_async: Could not create private data.\n")); > return -1; >@@ -318,6 +376,12 @@ static int open_async(const files_struct *fsp, > opd->dname, > opd->fname)); > >+ /* >+ * Add a destructor to protect us from connection >+ * teardown whilst the open thread is in flight. >+ */ >+ talloc_set_destructor(opd, opd_inflight_destructor); >+ > /* Cause the calling code to reschedule us. */ > errno = EINPROGRESS; /* Maps to NT_STATUS_MORE_PROCESSING_REQUIRED. */ > return -1; >-- >2.20.1 > > >From ae44875b2ff206cf3c39c55fbe78c553af797005 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 10 Mar 2020 10:55:22 -0700 >Subject: [PATCH 06/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 0c952bba1edf7c8173d05ccdc6fdaa7232d2c6aa) >--- > 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 d4a8fc9aa4572417f3422732971dde80f436f1e2 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 11 Mar 2020 14:47:50 -0700 >Subject: [PATCH 07/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit b90bc0f28918133badbf6810d5e298fc326bd1aa) >--- > 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 9f301bd2c3add46706aad04aaa81182ee17138eb Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 11 Mar 2020 17:25:59 -0700 >Subject: [PATCH 08/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit b7d09b30ad14d51bbcbe368a11348754121f6ff8) >--- > source3/smbd/reply.c | 7 +++++++ > 1 file changed, 7 insertions(+) > >diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c >index 40cd7483750..5887df41581 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -5659,6 +5659,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 915baed66b1c9499c40826e8f674fb9835fdb3d3 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 11 Mar 2020 15:16:35 -0700 >Subject: [PATCH 09/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 4287ea138e82103cce0a939e504f9810636b4747) >--- > 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 de75231e2829bf881d41461566f6b587ae447592 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 10 Mar 2020 12:25:58 -0700 >Subject: [PATCH 10/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit ac800ca6bcb43c74a1a6ef508b900e2e6cb532dc) >--- > source3/smbd/conn_idle.c | 5 +++++ > 1 file changed, 5 insertions(+) > >diff --git a/source3/smbd/conn_idle.c b/source3/smbd/conn_idle.c >index cd12e3f1266..698dc6e3a32 100644 >--- a/source3/smbd/conn_idle.c >+++ b/source3/smbd/conn_idle.c >@@ -103,6 +103,11 @@ void conn_force_tdis( > } > tcon = conn->tcon; > >+ if (!NT_STATUS_IS_OK(tcon->status)) { >+ /* In the process of already being disconnected. */ >+ continue; >+ } >+ > do_close = check_fn(conn, private_data); > if (!do_close) { > continue; >-- >2.20.1 > > >From 80f0e76d6e052f50ab037fa97938f3c9ca250c5f Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 09:28:21 -0700 >Subject: [PATCH 11/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 4f9e0459cd06f0332083a4a465f49b5f258838fa) >--- > 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 698dc6e3a32..b54cdf82274 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. >@@ -139,3 +140,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 c136392548b3265a3664fe926c34bad61fd07ff6 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 09:47:26 -0700 >Subject: [PATCH 12/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 7891302ab8eeba8261b92171a4d429e2f538b89a) >--- > 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 b54cdf82274..cf5a417bff7 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; >@@ -114,34 +114,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; > }; >@@ -286,4 +275,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 c1f9328e0c8ba555e27d258366f4a9ebbd843492 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 09:51:53 -0700 >Subject: [PATCH 13/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 86cc67d5a7de0a81131b11447dad57b2681d8e01) >--- > source3/smbd/reply.c | 173 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 173 insertions(+) > >diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c >index 5887df41581..3c7bdff49ac 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" >@@ -6067,6 +6068,178 @@ 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; >+ >+ req = tevent_req_create(smb1req, &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 sconn talloc-context. >+ * We need this to stick around until the wait_done >+ * callback is invoked. >+ */ >+ smb1req = talloc_move(smb1req->sconn, &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 76cb55666d15788611a4eda5f268524e0a13e9aa Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 09:55:30 -0700 >Subject: [PATCH 14/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit ca4521f1dd97bc5a05e381c652b05ae1eb8bd29b) >--- > 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 3c7bdff49ac..e67f598db58 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -6026,28 +6026,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", >@@ -6063,7 +6063,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 050b2a5ee21ba3271a92f178f25cb42eb66ef1d4 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 09:59:47 -0700 >Subject: [PATCH 15/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 71725f1c4adaa04ef04c0dd400c49399952ef5fa) >--- > 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 e67f598db58..d9e05c09830 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -6034,8 +6034,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 868a7c75f796f9f55da9828b1c03b1538b8a11c2 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 10:03:27 -0700 >Subject: [PATCH 16/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 7613998e10c5f13c896667257fdef33824a45d2a) >--- > 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 d9e05c09830..9f79f7de6ce 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -6026,49 +6026,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; > }; >@@ -6238,7 +6232,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 d5d78f52567ae76147db719be8f940f8bdb7e234 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 10:11:14 -0700 >Subject: [PATCH 17/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 4dd3012cb1b5e000ccf68d2601dbdbcb7ff538b5) >--- > source3/smbd/reply.c | 168 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 168 insertions(+) > >diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c >index 9f79f7de6ce..c2e3719ac57 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -2664,6 +2664,174 @@ 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; >+ >+ req = tevent_req_create(smb1req, &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 sconn talloc-context. >+ * We need this to stick around until the wait_done >+ * callback is invoked. >+ */ >+ smb1req = talloc_move(sconn, &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 ba6f525b93a7020ed1898d59c79a8ec8b83fb7df Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 10:15:10 -0700 >Subject: [PATCH 18/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 5c073aa01b304f54a0039d9cd9dc74123191eb4b) >--- > 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 c2e3719ac57..66fb9b2bd63 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -2612,7 +2612,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; >@@ -2620,16 +2620,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; > } >@@ -2653,15 +2653,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 4495645dab0728af824dfd0ab17ba24511e476ef Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 10:16:42 -0700 >Subject: [PATCH 19/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 9cda76ad29db0cfbffa3dbb0764ec5dda24490f9) >--- > 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 66fb9b2bd63..7b5ed7ef0db 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -2625,8 +2625,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 f1116b3cd4d414b9a1267d052aeeb5eef664c9d7 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 10:19:31 -0700 >Subject: [PATCH 20/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 446b64ca66591d8ae5b4bf1aabdd46a1e8cb1c1c) >--- > 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 7b5ed7ef0db..87f78df85d9 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -2612,19 +2612,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); > >@@ -2634,37 +2646,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; >@@ -2830,7 +2825,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 ba5120611008766494070baba44c0c3f547b62b2 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 10:59:16 -0700 >Subject: [PATCH 21/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 8f58feab58afbc7aa214fac2a1728dda68303c6b) >--- > source3/smbd/reply.c | 201 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 201 insertions(+) > >diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c >index 87f78df85d9..7b145da4f47 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -5767,6 +5767,207 @@ 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; >+ >+ req = tevent_req_create(smb1req, &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 conn talloc-context. >+ * We need this to stick around until the wait_done >+ * callback is invoked. >+ */ >+ smb1req = talloc_move(sconn, &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 d396723b6520a11613abfda77eefe6bf5b0d7063 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 11:01:05 -0700 >Subject: [PATCH 22/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 1de0daa715f3324e3620ae8152b7fbaeb40ee9d9) >--- > 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 7b145da4f47..2d4bb07305e 100644 >--- a/source3/smbd/reply.c >+++ b/source3/smbd/reply.c >@@ -5753,21 +5753,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; > }; >@@ -5966,7 +5978,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 9f695881c4601bb9369e5bd20e2c881aad5c3b8b Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 12 Mar 2020 11:02:19 -0700 >Subject: [PATCH 23/25] 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 <jra@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 584933439c70af3d2fd047e62a3456c1c2eca45e) >--- > 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 96d574023a5..911fe6dad3c 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 8b7bc0e44885cc2512402d475938c08a343d4f3a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Mon, 2 Mar 2020 13:11:06 -0800 >Subject: [PATCH 24/25] smbd: enforce AIO requests draining > >Assert we have no aio on a close. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Ralph Boehme <slow@samba.org> >Reviewed-by: Jeremy Allison <jra@samba.org> > >Combined squash of commits: > >(cherry picked from commit 410e7599bd2ae9b35429f60a529bb7c4aa88df25) >(cherry picked from commit acb0b01761330864a23932f643f7ad4e3d374634) >(cherry picked from commit f94cd10a211e2eae966ba4bd26921556bbe513fc) >(cherry picked from commit 0ae4f368c6c8d2c8c7aa34069007a984055df0da) >(cherry picked from commit 86dd5a080969e14ab0d131d8cb1054ec624a41ba) >--- > source3/smbd/close.c | 80 ++++++++++---------------------------------- > 1 file changed, 17 insertions(+), 63 deletions(-) > >diff --git a/source3/smbd/close.c b/source3/smbd/close.c >index f45371e656c..d5af62a277c 100644 >--- a/source3/smbd/close.c >+++ b/source3/smbd/close.c >@@ -31,6 +31,7 @@ > #include "auth.h" > #include "messages.h" > #include "../librpc/gen_ndr/open_files.h" >+#include "lib/util/tevent_ntstatus.h" > > /**************************************************************************** > Run a file if it is a magic script. >@@ -635,6 +636,20 @@ static NTSTATUS ntstatus_keeperror(NTSTATUS s1, NTSTATUS s2) > return s2; > } > >+static void assert_no_pending_aio(struct files_struct *fsp, >+ enum file_close_type close_type) >+{ >+ unsigned num_requests = fsp->num_aio_requests; >+ >+ if (num_requests == 0) { >+ return; >+ } >+ >+ DBG_ERR("fsp->num_aio_requests=%u\n", num_requests); >+ smb_panic("can not close with outstanding aio requests"); >+ return; >+} >+ > /**************************************************************************** > Close a file. > >@@ -651,45 +666,7 @@ static NTSTATUS close_normal_file(struct smb_request *req, files_struct *fsp, > connection_struct *conn = fsp->conn; > bool is_durable = false; > >- if (fsp->num_aio_requests != 0) { >- >- if (close_type != SHUTDOWN_CLOSE) { >- /* >- * reply_close and the smb2 close must have >- * taken care of this. No other callers of >- * close_file should ever have created async >- * I/O. >- * >- * We need to panic here because if we close() >- * the fd while we have outstanding async I/O >- * requests, in the worst case we could end up >- * writing to the wrong file. >- */ >- DEBUG(0, ("fsp->num_aio_requests=%u\n", >- fsp->num_aio_requests)); >- smb_panic("can not close with outstanding aio " >- "requests"); >- } >- >- /* >- * For shutdown close, just drop the async requests >- * including a potential close request pending for >- * this fsp. Drop the close request first, the >- * destructor for the aio_requests would execute it. >- */ >- TALLOC_FREE(fsp->deferred_close); >- >- while (fsp->num_aio_requests != 0) { >- /* >- * The destructor of the req will remove >- * itself from the fsp. >- * Don't use TALLOC_FREE here, this will overwrite >- * what the destructor just wrote into >- * aio_requests[0]. >- */ >- talloc_free(fsp->aio_requests[0]); >- } >- } >+ assert_no_pending_aio(fsp, close_type); > > while (talloc_array_length(fsp->blocked_smb1_lock_reqs) != 0) { > smbd_smb1_brl_finish_by_req( >@@ -1134,30 +1111,7 @@ static NTSTATUS close_directory(struct smb_request *req, files_struct *fsp, > notify_status = NT_STATUS_OK; > } > >- if (fsp->num_aio_requests != 0) { >- if (close_type != SHUTDOWN_CLOSE) { >- /* >- * We panic here because if we close() the fd while we >- * have outstanding async I/O requests, an async IO >- * request might use the fd. For directories the fd is >- * read-only, so this is not as bad as with files, but >- * still, better safe then sorry. >- */ >- DBG_ERR("fsp->num_aio_requests=%u\n", >- fsp->num_aio_requests); >- smb_panic("close with outstanding aio requests"); >- return NT_STATUS_INTERNAL_ERROR; >- } >- >- while (fsp->num_aio_requests != 0) { >- /* >- * The destructor of the req will remove itself from the >- * fsp. Don't use TALLOC_FREE here, this will overwrite >- * what the destructor just wrote into aio_requests[0]. >- */ >- talloc_free(fsp->aio_requests[0]); >- } >- } >+ assert_no_pending_aio(fsp, close_type); > > /* > * NT can set delete_on_close of the last open >-- >2.20.1 > > >From 7d2e8b46a2d155e40dc76d30e75e8824fececf6e Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 3 Mar 2020 13:31:18 -0800 >Subject: [PATCH 25/25] s3: tests: Add samba3.blackbox.force-close-share > >Checks server stays up whilst writing to a force closed share. >Uses existing aio_delay_inject share to delay writes while >we force close the share. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Volker Lendecke <vl@samba.org> > >Autobuild-User(master): Jeremy Allison <jra@samba.org> >Autobuild-Date(master): Sun Mar 8 19:34:14 UTC 2020 on sn-devel-184 > >(cherry picked from commit bb22be08b077b7d5911ccdeb1012f4dea85647e5) >(cherry picked from commit 566658d914176c41942e3c6aba404ae369aeb123) >--- > .../script/tests/test_force_close_share.sh | 100 ++++++++++++++++++ > source3/selftest/tests.py | 9 ++ > 2 files changed, 109 insertions(+) > create mode 100755 source3/script/tests/test_force_close_share.sh > >diff --git a/source3/script/tests/test_force_close_share.sh b/source3/script/tests/test_force_close_share.sh >new file mode 100755 >index 00000000000..da78b5a752e >--- /dev/null >+++ b/source3/script/tests/test_force_close_share.sh >@@ -0,0 +1,100 @@ >+#!/bin/bash >+# >+# Test smbcontrol close-share command. >+# >+# Copyright (C) 2020 Volker Lendecke >+# Copyright (C) 2020 Jeremy Allison >+# >+# Note this is designed to be run against >+# the aio_delay_inject share which is preconfigured >+# with 2 second delays on pread/pwrite. >+ >+if [ $# -lt 5 ]; then >+ echo Usage: test_share_force_close.sh \ >+ SERVERCONFFILE SMBCLIENT SMBCONTROL IP aio_delay_inject_sharename >+exit 1 >+fi >+ >+CONF=$1 >+SMBCLIENT=$2 >+SMBCONTROL=$3 >+SERVER=$4 >+SHARE=$5 >+ >+incdir=$(dirname $0)/../../../testprogs/blackbox >+. $incdir/subunit.sh >+ >+failed=0 >+ >+# Create the smbclient communication pipes. >+rm -f smbclient-stdin smbclient-stdout smbclient-stderr >+mkfifo smbclient-stdin smbclient-stdout smbclient-stderr >+ >+# Create a large-ish testfile >+rm testfile >+head -c 20MB /dev/zero >testfile >+ >+CLI_FORCE_INTERACTIVE=1; export CLI_FORCE_INTERACTIVE >+ >+${SMBCLIENT} //${SERVER}/${SHARE} ${CONF} -U${USER}%${PASSWORD} \ >+ < smbclient-stdin > smbclient-stdout 2>smbclient-stderr & >+CLIENT_PID=$! >+ >+sleep 1 >+ >+exec 100>smbclient-stdin 101<smbclient-stdout 102<smbclient-stderr >+ >+# consume the smbclient startup messages >+head -n 1 <&101 >+head -n 1 <&102 >+ >+# Ensure we're putting a fresh file. >+echo "del testfile" >&100 >+echo "put testfile" >&100 >+ >+sleep 1 >+ >+# Close the aio_delay_inject share whilst we have outstanding writes. >+ >+testit "smbcontrol" ${SMBCONTROL} ${CONF} smbd close-share ${SHARE} || >+ failed=$(expr $failed + 1) >+ >+sleep 1 >+ >+# If we get one or more NT_STATUS_NETWORK_NAME_DELETED >+# or NT_STATUS_INVALID_HANDLE on stderr from the writes we >+# know the server stayed up and didn't crash when the >+# close-share removed the share. >+# >+# BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 >+# >+COUNT=$(head -n 2 <&102 | >+ grep -e NT_STATUS_NETWORK_NAME_DELETED -e NT_STATUS_INVALID_HANDLE | >+ wc -l) >+ >+testit "Verify close-share did cancel the file put" \ >+ test $COUNT -ge 1 || failed=$(expr $failed + 1) >+ >+kill ${CLIENT_PID} >+ >+# Rerun smbclient to remove the testfile on the server. >+rm -f smbclient-stdin smbclient-stdout smbclient-stderr testfile >+mkfifo smbclient-stdin smbclient-stdout >+ >+${SMBCLIENT} //${SERVER}/${SHARE} ${CONF} -U${USER}%${PASSWORD} \ >+ < smbclient-stdin > smbclient-stdout & >+CLIENT_PID=$! >+ >+sleep 1 >+ >+exec 100>smbclient-stdin 101<smbclient-stdout >+ >+echo "del testfile" >&100 >+ >+sleep 1 >+ >+kill ${CLIENT_PID} >+ >+rm -f smbclient-stdin smbclient-stdout testfile >+ >+testok $0 $failed >diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py >index dddcb03230c..6bdbb6f97a4 100755 >--- a/source3/selftest/tests.py >+++ b/source3/selftest/tests.py >@@ -802,6 +802,15 @@ plantestsuite("samba3.blackbox.close-denied-share", "simpleserver:local", > '$SERVER_IP', > "tmp"]) > >+plantestsuite("samba3.blackbox.force-close-share", "simpleserver:local", >+ [os.path.join(samba3srcdir, >+ "script/tests/test_force_close_share.sh"), >+ configuration, >+ os.path.join(bindir(), "smbclient"), >+ os.path.join(bindir(), "smbcontrol"), >+ '$SERVER_IP', >+ "aio_delay_inject"]) >+ > plantestsuite("samba3.blackbox.open-eintr", "simpleserver:local", > [os.path.join(samba3srcdir, > "script/tests/test_open_eintr.sh"), >-- >2.20.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
slow
:
review+
Actions:
View
Attachments on
bug 14301
:
15825
|
15837
|
15844
|
15847
|
15854
|
15858
|
15861
|
15867
|
15870
|
16043
|
16044
|
16054
|
16055
|
16064
|
16065
|
16069
|
16091