The Samba-Bugzilla – Attachment 15844 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 master
bug-14301-master-1 (text/plain), 47.94 KB, created by
Jeremy Allison
on 2020-03-05 23:09:27 UTC
(
hide
)
Description:
git-am fix for master
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2020-03-05 23:09:27 UTC
Size:
47.94 KB
patch
obsolete
>From 0cb7622fc5ef57453362785ff9759edef4e39e9f Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:30:51 -0800 >Subject: [PATCH 01/24] s3: VFS: vfs_default: Add tevent_req pointer to state > struct in vfswrap_pread_state. > >We will need this to detect when this request is outstanding but >has been destroyed in a SHUTDOWN_CLOSE on this file. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index a30f3ba1d31..4bb4adf5f7e 100644 >--- a/source3/modules/vfs_default.c >+++ b/source3/modules/vfs_default.c >@@ -780,6 +780,7 @@ 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; >@@ -809,6 +810,7 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 4b5704442f1b6f75aad501235f27a74c548d14fe Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:34:51 -0800 >Subject: [PATCH 02/24] s3: VFS: vfs_default. Pass in struct > vfswrap_pread_state as the callback data to the subreq. > >Find the req we're finishing off by looking inside vfswrap_pread_state. >In a shutdown close the caller calls talloc_free(req), so we can't >access it directly as callback data. > >The next commit will NULL out the vfswrap_pread_state->req pointer >when a caller calls talloc_free(req), and the request is still in >flight. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index 4bb4adf5f7e..b8c36180b7c 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, req); >+ tevent_req_set_callback(subreq, vfs_pread_done, state); > > talloc_set_destructor(state, vfs_pread_state_destructor); > >@@ -868,10 +868,9 @@ static int vfs_pread_state_destructor(struct vfswrap_pread_state *state) > > static void vfs_pread_done(struct tevent_req *subreq) > { >- 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); >+ struct vfswrap_pread_state *state = tevent_req_callback_data( >+ subreq, struct vfswrap_pread_state); >+ struct tevent_req *req = state->req; > int ret; > > ret = pthreadpool_tevent_job_recv(subreq); >-- >2.25.0.265.gbab2e86ba0-goog > > >From 4d018cd2c2d0d077523234fbfb484a47b043d096 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:40:46 -0800 >Subject: [PATCH 03/24] s3: VFS: vfs_default. Protect vfs_pread_done() from > accessing a freed req pointer. > >If the fsp is forced closed by a SHUTDOWN_CLOSE whilst the >request is in flight (share forced closed by smbcontrol), >then we set state->req = NULL in the state destructor. > >The existing state destructor prevents the state memory >from being freed, so when the thread completes and calls >vfs_pread_done(), just throw away the result if >state->req == NULL. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index b8c36180b7c..21bc9c7adf7 100644 >--- a/source3/modules/vfs_default.c >+++ b/source3/modules/vfs_default.c >@@ -863,6 +863,15 @@ 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; > } > >@@ -877,6 +886,17 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 828f0d2015765d89a481ee9f09405b7ad739a0dc Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:44:39 -0800 >Subject: [PATCH 04/24] s3: VFS: vfs_default: Add tevent_req pointer to state > struct in vfswrap_pwrite_state. > >We will need this to detect when this request is outstanding but >has been destroyed in a SHUTDOWN_CLOSE on this file. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index 21bc9c7adf7..cbc8335cd12 100644 >--- a/source3/modules/vfs_default.c >+++ b/source3/modules/vfs_default.c >@@ -929,6 +929,7 @@ 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; >@@ -958,6 +959,7 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 6ae4c8e1b2af38ba5ced0871fa3e6a47755d152a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:49:38 -0800 >Subject: [PATCH 05/24] s3: VFS: vfs_default. Pass in struct > vfswrap_pwrite_state as the callback data to the subreq. > >Find the req we're finishing off by looking inside vfswrap_pwrite_state. >In a shutdown close the caller calls talloc_free(req), so we can't >access it directly as callback data. > >The next commit will NULL out the vfswrap_pwrite_state->req pointer >when a caller calls talloc_free(req), and the request is still in >flight. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index cbc8335cd12..641764e41f1 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, req); >+ tevent_req_set_callback(subreq, vfs_pwrite_done, state); > > talloc_set_destructor(state, vfs_pwrite_state_destructor); > >@@ -1017,10 +1017,9 @@ static int vfs_pwrite_state_destructor(struct vfswrap_pwrite_state *state) > > static void vfs_pwrite_done(struct tevent_req *subreq) > { >- 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); >+ struct vfswrap_pwrite_state *state = tevent_req_callback_data( >+ subreq, struct vfswrap_pwrite_state); >+ struct tevent_req *req = state->req; > int ret; > > ret = pthreadpool_tevent_job_recv(subreq); >-- >2.25.0.265.gbab2e86ba0-goog > > >From 0c7f9dacc930db61588e5217c093c609a57ab8f1 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:51:35 -0800 >Subject: [PATCH 06/24] s3: VFS: vfs_default. Protect vfs_pwrite_done() from > accessing a freed req pointer. > >If the fsp is forced closed by a SHUTDOWN_CLOSE whilst the >request is in flight (share forced closed by smbcontrol), >then we set state->req = NULL in the state destructor. > >The existing state destructor prevents the state memory >from being freed, so when the thread completes and calls >vfs_pwrite_done(), just throw away the result if >state->req == NULL. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index 641764e41f1..3425ee31dcb 100644 >--- a/source3/modules/vfs_default.c >+++ b/source3/modules/vfs_default.c >@@ -1012,6 +1012,15 @@ 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; > } > >@@ -1026,6 +1035,17 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 6a6f281b0ceced08dc0bac42140ffb0a0ca5e4ae Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:53:10 -0800 >Subject: [PATCH 07/24] s3: VFS: vfs_default: Add tevent_req pointer to state > struct in vfswrap_fsync_state. > >We will need this to detect when this request is outstanding but >has been destroyed in a SHUTDOWN_CLOSE on this file. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index 3425ee31dcb..28b8c04dee4 100644 >--- a/source3/modules/vfs_default.c >+++ b/source3/modules/vfs_default.c >@@ -1078,6 +1078,7 @@ static ssize_t vfswrap_pwrite_recv(struct tevent_req *req, > } > > struct vfswrap_fsync_state { >+ struct tevent_req *req; > ssize_t ret; > int fd; > >@@ -1102,6 +1103,7 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 354418f43854fe9e42bc5a431db1b2cfdd336895 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:54:47 -0800 >Subject: [PATCH 08/24] s3: VFS: vfs_default. Pass in struct > vfswrap_fsync_state as the callback data to the subreq. > >Find the req we're finishing off by looking inside vfswrap_fsync_state. >In a shutdown close the caller calls talloc_free(req), so we can't >access it directly as callback data. > >The next commit will NULL out the vfswrap_fsync_state->req pointer >when a caller calls talloc_free(req), and the request is still in >flight. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index 28b8c04dee4..f9d958a003d 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, req); >+ tevent_req_set_callback(subreq, vfs_fsync_done, state); > > talloc_set_destructor(state, vfs_fsync_state_destructor); > >@@ -1156,10 +1156,9 @@ static int vfs_fsync_state_destructor(struct vfswrap_fsync_state *state) > > static void vfs_fsync_done(struct tevent_req *subreq) > { >- 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); >+ struct vfswrap_fsync_state *state = tevent_req_callback_data( >+ subreq, struct vfswrap_fsync_state); >+ struct tevent_req *req = state->req; > int ret; > > ret = pthreadpool_tevent_job_recv(subreq); >-- >2.25.0.265.gbab2e86ba0-goog > > >From d76aa10c02d666dfbf554967ca5ce9c85eec6110 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 27 Feb 2020 16:56:41 -0800 >Subject: [PATCH 09/24] s3: VFS: vfs_default. Protect vfs_fsync_done() from > accessing a freed req pointer. > >If the fsp is forced closed by a SHUTDOWN_CLOSE whilst the >request is in flight (share forced closed by smbcontrol), >then we set state->req = NULL in the state destructor. > >The existing state destructor prevents the state memory >from being freed, so when the thread completes and calls >vfs_fsync_done(), just throw away the result if >state->req == NULL. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_default.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > >diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c >index f9d958a003d..fac7fa30ab7 100644 >--- a/source3/modules/vfs_default.c >+++ b/source3/modules/vfs_default.c >@@ -1151,6 +1151,15 @@ 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; > } > >@@ -1165,6 +1174,17 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 277ca97cadd49d722ed1a1178e46362bf483f3ac Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 15:33:35 -0800 >Subject: [PATCH 10/24] s3: VFS: vfs_glusterfs: Add tevent_req pointer to state > struct in vfs_gluster_pread_state. > >We will need this to detect when this request is outstanding but >has been destroyed in a SHUTDOWN_CLOSE on this file. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index d4b68fba376..6598aadad17 100644 >--- a/source3/modules/vfs_glusterfs.c >+++ b/source3/modules/vfs_glusterfs.c >@@ -713,6 +713,7 @@ 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; >@@ -748,6 +749,7 @@ 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.25.0.265.gbab2e86ba0-goog > > >From c364533fc66a330a13cd0c7e8c580ca9da2ee5c5 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 15:35:46 -0800 >Subject: [PATCH 11/24] s3: VFS: vfs_glusterfs. Pass in struct > vfs_gluster_pread_state as the callback data to the subreq. > >Find the req we're finishing off by looking inside vfs_gluster_pread_state. >In a shutdown close the caller calls talloc_free(req), so we can't >access it directly as callback data. > >The next commit will NULL out the vfs_gluster_pread_state->req pointer >when a caller calls talloc_free(req), and the request is still in >flight. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index 6598aadad17..84b284152c6 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, req); >+ tevent_req_set_callback(subreq, vfs_gluster_pread_done, state); > > talloc_set_destructor(state, vfs_gluster_pread_state_destructor); > >@@ -812,10 +812,9 @@ static int vfs_gluster_pread_state_destructor(struct vfs_gluster_pread_state *st > > static void vfs_gluster_pread_done(struct tevent_req *subreq) > { >- 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); >+ struct vfs_gluster_pread_state *state = tevent_req_callback_data( >+ subreq, struct vfs_gluster_pread_state); >+ struct tevent_req *req = state->req; > int ret; > > ret = pthreadpool_tevent_job_recv(subreq); >-- >2.25.0.265.gbab2e86ba0-goog > > >From e46bd7ace5f14979ba6f66250236fab3f256084f Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 15:38:04 -0800 >Subject: [PATCH 12/24] s3: VFS: vfs_glusterfs. Protect > vfs_gluster_pread_done() from accessing a freed req pointer. > >If the fsp is forced closed by a SHUTDOWN_CLOSE whilst the >request is in flight (share forced closed by smbcontrol), >then we set state->req = NULL in the state destructor. > >The existing state destructor prevents the state memory >from being freed, so when the thread completes and calls >vfs_gluster_pread_done(), just throw away the result if >state->req == NULL. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index 84b284152c6..7924f123cca 100644 >--- a/source3/modules/vfs_glusterfs.c >+++ b/source3/modules/vfs_glusterfs.c >@@ -807,6 +807,15 @@ 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; > } > >@@ -821,6 +830,17 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 8f36bc0a13de006b7a10a0c6739b5cfe5c4a312b Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 15:47:52 -0800 >Subject: [PATCH 13/24] s3: VFS: vfs_glusterfs: Add tevent_req pointer to state > struct in vfs_gluster_pwrite_state. > >We will need this to detect when this request is outstanding but >has been destroyed in a SHUTDOWN_CLOSE on this file. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index 7924f123cca..456e0c8a498 100644 >--- a/source3/modules/vfs_glusterfs.c >+++ b/source3/modules/vfs_glusterfs.c >@@ -873,6 +873,7 @@ 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; >@@ -908,6 +909,7 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 3c2bff21114102a197ff035431e7f7487264020a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 15:53:19 -0800 >Subject: [PATCH 14/24] s3: VFS: vfs_glusterfs. Pass in struct > vfs_gluster_pwrite_state as the callback data to the subreq. > >Find the req we're finishing off by looking inside vfs_gluster_pwrite_state. >In a shutdown close the caller calls talloc_free(req), so we can't >access it directly as callback data. > >The next commit will NULL out the vfs_gluster_pwrite_state->req pointer >when a caller calls talloc_free(req), and the request is still in >flight. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index 456e0c8a498..52c33725b8d 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, req); >+ tevent_req_set_callback(subreq, vfs_gluster_pwrite_done, state); > > talloc_set_destructor(state, vfs_gluster_pwrite_state_destructor); > >@@ -972,10 +972,9 @@ static int vfs_gluster_pwrite_state_destructor(struct vfs_gluster_pwrite_state * > > static void vfs_gluster_pwrite_done(struct tevent_req *subreq) > { >- 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); >+ struct vfs_gluster_pwrite_state *state = tevent_req_callback_data( >+ subreq, struct vfs_gluster_pwrite_state); >+ struct tevent_req *req = state->req; > int ret; > > ret = pthreadpool_tevent_job_recv(subreq); >-- >2.25.0.265.gbab2e86ba0-goog > > >From ecc735fad2cc2af0012bac655aca162e0155c0c5 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 15:55:36 -0800 >Subject: [PATCH 15/24] s3: VFS: vfs_glusterfs. Protect > vfs_gluster_pwrite_done() from accessing a freed req pointer. > >If the fsp is forced closed by a SHUTDOWN_CLOSE whilst the >request is in flight (share forced closed by smbcontrol), >then we set state->req = NULL in the state destructor. > >The existing state destructor prevents the state memory >from being freed, so when the thread completes and calls >vfs_gluster_pwrite_done(), just throw away the result if >state->req == NULL. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index 52c33725b8d..4e978f168d6 100644 >--- a/source3/modules/vfs_glusterfs.c >+++ b/source3/modules/vfs_glusterfs.c >@@ -967,6 +967,15 @@ 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; > } > >@@ -981,6 +990,17 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 01fdf01ed6157c176f1c9635c3be18fe9a1df8fd Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 15:57:20 -0800 >Subject: [PATCH 16/24] s3: VFS: vfs_glusterfs: Add tevent_req pointer to state > struct in vfs_gluster_fsync_state. > >We will need this to detect when this request is outstanding but >has been destroyed in a SHUTDOWN_CLOSE on this file. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 2 ++ > 1 file changed, 2 insertions(+) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index 4e978f168d6..d5d402d72ab 100644 >--- a/source3/modules/vfs_glusterfs.c >+++ b/source3/modules/vfs_glusterfs.c >@@ -1114,6 +1114,7 @@ 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; > >@@ -1144,6 +1145,7 @@ static struct tevent_req *vfs_gluster_fsync_send(struct vfs_handle_struct > return NULL; > } > >+ state->req = req; > state->ret = -1; > state->fd = glfd; > >-- >2.25.0.265.gbab2e86ba0-goog > > >From 8d23a186e4fbbe0471eca10db11592334f0cac5a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 15:59:16 -0800 >Subject: [PATCH 17/24] s3: VFS: vfs_glusterfs. Pass in struct > vfs_gluster_fsync_state as the callback data to the subreq. > >Find the req we're finishing off by looking inside vfs_gluster_fsync_state. >In a shutdown close the caller calls talloc_free(req), so we can't >access it directly as callback data. > >The next commit will NULL out the vfs_gluster_fsync_state->req pointer >when a caller calls talloc_free(req), and the request is still in >flight. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index d5d402d72ab..4706e6f9189 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, req); >+ tevent_req_set_callback(subreq, vfs_gluster_fsync_done, state); > > talloc_set_destructor(state, vfs_gluster_fsync_state_destructor); > >@@ -1202,10 +1202,9 @@ static int vfs_gluster_fsync_state_destructor(struct vfs_gluster_fsync_state *st > > static void vfs_gluster_fsync_done(struct tevent_req *subreq) > { >- 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); >+ struct vfs_gluster_fsync_state *state = tevent_req_callback_data( >+ subreq, struct vfs_gluster_fsync_state); >+ struct tevent_req *req = state->req; > int ret; > > ret = pthreadpool_tevent_job_recv(subreq); >-- >2.25.0.265.gbab2e86ba0-goog > > >From 3f39badb23de37c87e77d56ac71d80007f715f38 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Fri, 28 Feb 2020 16:01:11 -0800 >Subject: [PATCH 18/24] s3: VFS: vfs_glusterfs. Protect > vfs_gluster_fsync_done() from accessing a freed req pointer. > >If the fsp is forced closed by a SHUTDOWN_CLOSE whilst the >request is in flight (share forced closed by smbcontrol), >then we set state->req = NULL in the state destructor. > >The existing state destructor prevents the state memory >from being freed, so when the thread completes and calls >vfs_gluster_fsync_done(), just throw away the result if >state->req == NULL. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/modules/vfs_glusterfs.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > >diff --git a/source3/modules/vfs_glusterfs.c b/source3/modules/vfs_glusterfs.c >index 4706e6f9189..b5300282b7b 100644 >--- a/source3/modules/vfs_glusterfs.c >+++ b/source3/modules/vfs_glusterfs.c >@@ -1197,6 +1197,15 @@ 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; > } > >@@ -1211,6 +1220,17 @@ 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.25.0.265.gbab2e86ba0-goog > > >From 49c8d1ea87e5597ab7b6d4a218d62fcf6779dcd6 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Mon, 2 Mar 2020 13:11:06 -0800 >Subject: [PATCH 19/24] s3: smbd: Make sure we correctly reply to outstanding > aio requests with an error on SHUTDOWN_CLOSE. > >SHUTDOWN_CLOSE can be called when smbcontrol close-share >is used to terminate active connections. > >Previously we just called talloc_free() >on the outstanding request, but this >caused crashes (before the async callback >functions were fixed not to reference req >directly) and also leaves the SMB2 request >outstanding on the processing queue. > >Using tevent_req_error() instead >causes the outstanding SMB1/2/3 request to >return with NT_STATUS_INVALID_HANDLE >and removes it from the processing queue. > >The callback function called from this >calls talloc_free(req). The destructor will remove >itself from the fsp and the aio_requests array. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14301 > >Signed-off-by: Jeremy Allison <jra@samba.org> >--- > source3/smbd/close.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > >diff --git a/source3/smbd/close.c b/source3/smbd/close.c >index f45371e656c..c7be0c8d447 100644 >--- a/source3/smbd/close.c >+++ b/source3/smbd/close.c >@@ -652,6 +652,7 @@ static NTSTATUS close_normal_file(struct smb_request *req, files_struct *fsp, > bool is_durable = false; > > if (fsp->num_aio_requests != 0) { >+ unsigned num_requests = fsp->num_aio_requests; > > if (close_type != SHUTDOWN_CLOSE) { > /* >@@ -681,13 +682,30 @@ static NTSTATUS close_normal_file(struct smb_request *req, files_struct *fsp, > > 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]. >+ * Previously we just called talloc_free() >+ * on the outstanding request, but this >+ * caused crashes (before the async callback >+ * functions were fixed not to reference req >+ * directly) and also leaves the SMB2 request >+ * outstanding on the processing queue. >+ * >+ * Using tevent_req_error() instead >+ * causes the outstanding SMB1/2/3 request to >+ * return with NT_STATUS_INVALID_HANDLE >+ * and removes it from the processing queue. >+ * >+ * The callback function called from this >+ * calls talloc_free(req). The destructor will remove >+ * itself from the fsp and the aio_requests array. > */ >- talloc_free(fsp->aio_requests[0]); >+ tevent_req_error(fsp->aio_requests[0], EBADF); >+ >+ /* Paranoia to ensure we don't spin. */ >+ num_requests--; >+ if (fsp->num_aio_requests != num_requests) { >+ smb_panic("cannot cancel outstanding aio " >+ "requests"); >+ } > } > } > >-- >2.25.0.265.gbab2e86ba0-goog > > >From 7fcc5cf32b6c92eeaa5e393cb2de6d38ffa0cf8a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 4 Mar 2020 13:29:08 -0800 >Subject: [PATCH 20/24] 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> >--- > 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.25.0.265.gbab2e86ba0-goog > > >From 98ad33d2ab32c190db650366e4e9d385d3239e18 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 4 Mar 2020 13:47:13 -0800 >Subject: [PATCH 21/24] 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> >--- > 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.25.0.265.gbab2e86ba0-goog > > >From d1a5a94c681bee9c94970e3f3b953cf516ce719a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 4 Mar 2020 16:39:39 -0800 >Subject: [PATCH 22/24] 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> >--- > 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.25.0.265.gbab2e86ba0-goog > > >From 564d3c1a3ae1997fdb91130704457dec09564ba8 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 5 Mar 2020 10:22:00 -0800 >Subject: [PATCH 23/24] s3: VFS: vfs_aio_pthread: Make aio opens safe against > connection teardown. > >Allocate state off the awaiting fsp, and add a destructor >that catches deallocation of that fsp (caused by the >deallocation of the containing conn struct) and record >that fact in state. Moving to allocating off fsp instead >of the NULL context is needed so we can detect (by the >destructor firing) when the conn struct is torn down. >That allows us to NULL out the saved conn struct pointer >so we know not to access deallocated memory. > >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> >--- > source3/modules/vfs_aio_pthread.c | 55 ++++++++++++++++++++++++++++++- > 1 file changed, 54 insertions(+), 1 deletion(-) > >diff --git a/source3/modules/vfs_aio_pthread.c b/source3/modules/vfs_aio_pthread.c >index d5919f83b3f..2ad39107e64 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; > >@@ -221,7 +252,7 @@ static struct aio_open_private_data *create_private_open_data(const files_struct > int flags, > mode_t mode) > { >- struct aio_open_private_data *opd = talloc_zero(NULL, >+ struct aio_open_private_data *opd = talloc_zero(fsp, > struct aio_open_private_data); > const char *fname = NULL; > >@@ -285,6 +316,22 @@ static struct aio_open_private_data *create_private_open_data(const files_struct > 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. > *****************************************************************/ >@@ -317,6 +364,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.25.0.265.gbab2e86ba0-goog > > >From 341e516697506fef2b6061ff949ecb7acb38562a Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Tue, 3 Mar 2020 13:31:18 -0800 >Subject: [PATCH 24/24] 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> >--- > .../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..ebfff5af77c >--- /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 10MB /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 6e9d3ddb144..ff6ab1a6b15 100755 >--- a/source3/selftest/tests.py >+++ b/source3/selftest/tests.py >@@ -816,6 +816,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.25.0.265.gbab2e86ba0-goog >
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:
jra
:
ci-passed-
Actions:
View
Attachments on
bug 14301
:
15825
|
15837
|
15844
|
15847
|
15854
|
15858
|
15861
|
15867
|
15870
|
16043
|
16044
|
16054
|
16055
|
16064
|
16065
|
16069
|
16091