From 0cb7622fc5ef57453362785ff9759edef4e39e9f Mon Sep 17 00:00:00 2001 From: Jeremy Allison 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- 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 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 --- .../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&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&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