The Samba-Bugzilla – Attachment 15837 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 (text/plain), 33.42 KB, created by
Jeremy Allison
on 2020-03-02 21:46:05 UTC
(
hide
)
Description:
git-am fix for master
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2020-03-02 21:46:05 UTC
Size:
33.42 KB
patch
obsolete
>From 3a903adaf8f0cca25d557972bc7689a7898a9ccc 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/19] 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 5fd132bc072e32f3441f2adff915493af7a86c98 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/19] 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 792bafe43b80313030eb02af3d4f702508cb3203 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/19] 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 4d79e71ae0289df4ce3f786ed0216c896df3d781 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/19] 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 a1d4770cb91678e0e09c25602d827e6cf506da07 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/19] 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 a72da31e76e848153c5ddd80309a8407e38cee11 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/19] 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 4c522ffe56a6a43761f2acd3b093eaf045d1c998 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/19] 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 4c86b16d315daff914f9a36a8c52314e03cbd9f8 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/19] 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 fc4963a15f0a9d607d9d1f6e1d00d4cf91d04167 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/19] 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 b75302b414bffe0aa7127af58444c62f22e8f780 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/19] 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 5d908bb2b28ff2c39f88548d72523d04fc62332c 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/19] 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 89133b357f1c0900356302d4c01755d85a1151b0 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/19] 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 f54e312e7ff389dc0cde9419e480ad79629705a1 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/19] 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 e4b59851161d5c639a565e2a03952bf52a7e5b5b 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/19] 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 6bfe04ec2dc4fe2790ad72169c0f61dcc6081763 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/19] 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 972857e66c767d2fe1485a3ad76165790264a37c 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/19] 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 47a0a26968b69d09717f0f4b4e07881e33027113 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/19] 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 ec69274a71c4a4a4271b0225f541fdf0b9dd92fb 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/19] 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 d538874e69dcd32c6e59cd1819d4b2507667eaf5 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/19] 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 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=15837">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 14301
:
15825
|
15837
|
15844
|
15847
|
15854
|
15858
|
15861
|
15867
|
15870
|
16043
|
16044
|
16054
|
16055
|
16064
|
16065
|
16069
|
16091