[PATCH] Version 2: Patchset for bug #10344 - SessionLogoff on a signed connection with an outstanding notify request crashes smbd.
Jeremy Allison
jra at samba.org
Tue Mar 11 15:59:06 MDT 2014
On Tue, Mar 11, 2014 at 01:59:08PM -0700, Jeremy Allison wrote:
>
> CONCLUSION (if anyone actually gets down as far as this :-).
> ------------------------------------------------------------
>
> I agree on tmp1.diff and tmp2.diff, with the 'Signed-off-by'
> change mentioned above.
>
> I hate it, but the tevent_queue_wait_send() already exists
> and is in use inside our code, and even though I think my
> code is easier to understand people can disagree, so in
> the interests of not adding extra API's I'm willing to accept
> metze's fix in place of mine.
>
> I also think (at least for the smb2_sesssetup.c
> and smb2_tcon.c changes that we should change the
> 'Signed-off-by' lines to include both Metze and
> myself, as this has been a herculean effort from
> both of us.
Here is the proposed patchset for master. Even
though I like my API better, I recognise when it's
a reinvention of what is already there :-).
Metze, if you're OK with this I'll push to
master and work on back-ports for 4.1.x,
4.0.x for the bug report.
Cheers,
Jeremy.
-------------- next part --------------
From c4d0f42576f2e56f0aac1ceda7f581499c7a42a2 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 7 Mar 2014 12:31:19 +0100
Subject: [PATCH 1/9] s4:torture/smb2: accept NT_STATUS_RANGE_NOT_LOCKED after
smb2_logoff/tdis
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source4/torture/smb2/lock.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/source4/torture/smb2/lock.c b/source4/torture/smb2/lock.c
index 9350c13..a27ae90 100644
--- a/source4/torture/smb2/lock.c
+++ b/source4/torture/smb2/lock.c
@@ -1044,11 +1044,12 @@ static bool test_cancel_tdis(struct torture_context *torture,
torture_comment(torture, " Check pending lock reply\n");
status = smb2_lock_recv(req, &lck);
- if (torture_setting_bool(torture, "samba4", false)) {
- /* saying that this lock succeeded is nonsense - the
- * tree is gone!! */
- CHECK_STATUS(status, NT_STATUS_RANGE_NOT_LOCKED);
- } else {
+ if (!NT_STATUS_EQUAL(status, NT_STATUS_RANGE_NOT_LOCKED)) {
+ /*
+ * The status depends on the server internals
+ * the order in which the files are closed
+ * by smb2_tdis().
+ */
CHECK_STATUS(status, NT_STATUS_OK);
}
@@ -1087,7 +1088,7 @@ static bool test_cancel_logoff(struct torture_context *torture,
struct smb2_lock_element el[2];
struct smb2_request *req = NULL;
- const char *fname = BASEDIR "\\cancel_tdis.txt";
+ const char *fname = BASEDIR "\\cancel_logoff.txt";
status = torture_smb2_testdir(tree, BASEDIR, &h);
CHECK_STATUS(status, NT_STATUS_OK);
@@ -1130,11 +1131,12 @@ static bool test_cancel_logoff(struct torture_context *torture,
torture_comment(torture, " Check pending lock reply\n");
status = smb2_lock_recv(req, &lck);
- if (torture_setting_bool(torture, "samba4", false)) {
- /* another bogus 'success' code from windows. The lock
- * cannot have succeeded, as we are now logged off */
- CHECK_STATUS(status, NT_STATUS_RANGE_NOT_LOCKED);
- } else {
+ if (!NT_STATUS_EQUAL(status, NT_STATUS_RANGE_NOT_LOCKED)) {
+ /*
+ * The status depends on the server internals
+ * the order in which the files are closed
+ * by smb2_logoff().
+ */
CHECK_STATUS(status, NT_STATUS_OK);
}
--
1.9.0.279.gdc9e3eb
From 894110760363a3541e79a4e034c709e15f94d3ed Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 10 Mar 2014 09:43:35 +0100
Subject: [PATCH 2/9] s3:smb2_lock: fix whitespaces/tabs in
smbd_smb2_lock_cancel()
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_lock.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c
index 4af3d61..69bc09a 100644
--- a/source3/smbd/smb2_lock.c
+++ b/source3/smbd/smb2_lock.c
@@ -368,23 +368,23 @@ static NTSTATUS smbd_smb2_lock_recv(struct tevent_req *req)
static bool smbd_smb2_lock_cancel(struct tevent_req *req)
{
- struct smbd_smb2_request *smb2req = NULL;
- struct smbd_smb2_lock_state *state = tevent_req_data(req,
- struct smbd_smb2_lock_state);
- if (!state) {
- return false;
- }
+ struct smbd_smb2_request *smb2req = NULL;
+ struct smbd_smb2_lock_state *state = tevent_req_data(req,
+ struct smbd_smb2_lock_state);
+ if (!state) {
+ return false;
+ }
- if (!state->smb2req) {
- return false;
- }
+ if (!state->smb2req) {
+ return false;
+ }
- smb2req = state->smb2req;
+ smb2req = state->smb2req;
remove_pending_lock(state, state->blr);
tevent_req_defer_callback(req, smb2req->sconn->ev_ctx);
tevent_req_nterror(req, NT_STATUS_CANCELLED);
- return true;
+ return true;
}
/****************************************************************
--
1.9.0.279.gdc9e3eb
From da1c3f868934c3fc652634417dcda0db4c8c985b Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 10 Mar 2014 09:47:11 +0100
Subject: [PATCH 3/9] s3:smb2_lock: return RANGE_NOT_LOCKED instead of
CANCELLED for logoff and tdis
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_lock.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/source3/smbd/smb2_lock.c b/source3/smbd/smb2_lock.c
index 69bc09a..5da14e9 100644
--- a/source3/smbd/smb2_lock.c
+++ b/source3/smbd/smb2_lock.c
@@ -383,6 +383,26 @@ static bool smbd_smb2_lock_cancel(struct tevent_req *req)
remove_pending_lock(state, state->blr);
tevent_req_defer_callback(req, smb2req->sconn->ev_ctx);
+
+ /*
+ * If the request is canceled because of logoff, tdis or close
+ * the status is NT_STATUS_RANGE_NOT_LOCKED instead of
+ * NT_STATUS_CANCELLED.
+ *
+ * Note that the close case is handled in
+ * cancel_pending_lock_requests_by_fid_smb2(SHUTDOWN_CLOSE)
+ * for now.
+ */
+ if (!NT_STATUS_IS_OK(smb2req->session->status)) {
+ tevent_req_nterror(req, NT_STATUS_RANGE_NOT_LOCKED);
+ return true;
+ }
+
+ if (!NT_STATUS_IS_OK(smb2req->tcon->status)) {
+ tevent_req_nterror(req, NT_STATUS_RANGE_NOT_LOCKED);
+ return true;
+ }
+
tevent_req_nterror(req, NT_STATUS_CANCELLED);
return true;
}
--
1.9.0.279.gdc9e3eb
From 1d69b52e2b1f27212ac91b44ef4a714b76c5fed3 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 10 Mar 2014 09:53:18 +0100
Subject: [PATCH 4/9] s3:smb2_sesssetup: split smbd_smb2_logoff into an async
*_send/recv pair.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=10344
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_sesssetup.c | 112 ++++++++++++++++++++++++++++++++++--------
1 file changed, 92 insertions(+), 20 deletions(-)
diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index 9d7193b..786bfe8 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -788,44 +788,116 @@ static NTSTATUS smbd_smb2_session_setup_recv(struct tevent_req *req,
return status;
}
+static struct tevent_req *smbd_smb2_logoff_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct smbd_smb2_request *smb2req);
+static NTSTATUS smbd_smb2_logoff_recv(struct tevent_req *req);
+static void smbd_smb2_request_logoff_done(struct tevent_req *subreq);
+
NTSTATUS smbd_smb2_request_process_logoff(struct smbd_smb2_request *req)
{
NTSTATUS status;
- DATA_BLOB outbody;
+ struct tevent_req *subreq = NULL;
status = smbd_smb2_request_verify_sizes(req, 0x04);
if (!NT_STATUS_IS_OK(status)) {
return smbd_smb2_request_error(req, status);
}
+ subreq = smbd_smb2_logoff_send(req, req->sconn->ev_ctx, req);
+ if (subreq == NULL) {
+ return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
+ }
+ tevent_req_set_callback(subreq, smbd_smb2_request_logoff_done, req);
+
/*
- * TODO: cancel all outstanding requests on the session
+ * Wait a long time before going async on this to allow
+ * requests we're waiting on to finish. Set timeout to 10 secs.
*/
- status = smbXsrv_session_logoff(req->session);
+ return smbd_smb2_request_pending_queue(req, subreq, 10000000);
+}
+
+static void smbd_smb2_request_logoff_done(struct tevent_req *subreq)
+{
+ struct smbd_smb2_request *smb2req =
+ tevent_req_callback_data(subreq,
+ struct smbd_smb2_request);
+ DATA_BLOB outbody;
+ NTSTATUS status;
+ NTSTATUS error;
+
+ status = smbd_smb2_logoff_recv(subreq);
+ TALLOC_FREE(subreq);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(0, ("smbd_smb2_request_process_logoff: "
- "smbXsrv_session_logoff() failed: %s\n",
- nt_errstr(status)));
- /*
- * If we hit this case, there is something completely
- * wrong, so we better disconnect the transport connection.
- */
- return status;
+ error = smbd_smb2_request_error(smb2req, status);
+ if (!NT_STATUS_IS_OK(error)) {
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(error));
+ return;
+ }
+ return;
+ }
+
+ outbody = smbd_smb2_generate_outbody(smb2req, 0x04);
+ if (outbody.data == NULL) {
+ error = smbd_smb2_request_error(smb2req, NT_STATUS_NO_MEMORY);
+ if (!NT_STATUS_IS_OK(error)) {
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(error));
+ return;
+ }
+ return;
+ }
+
+ SSVAL(outbody.data, 0x00, 0x04); /* struct size */
+ SSVAL(outbody.data, 0x02, 0); /* reserved */
+
+ error = smbd_smb2_request_done(smb2req, outbody, NULL);
+ if (!NT_STATUS_IS_OK(error)) {
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(error));
+ return;
+ }
+}
+
+struct smbd_smb2_logout_state {
+ struct smbd_smb2_request *smb2req;
+};
+
+static struct tevent_req *smbd_smb2_logoff_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct smbd_smb2_request *smb2req)
+{
+ struct tevent_req *req;
+ struct smbd_smb2_logout_state *state;
+ NTSTATUS status;
+
+ req = tevent_req_create(mem_ctx, &state,
+ struct smbd_smb2_logout_state);
+ if (req == NULL) {
+ return NULL;
+ }
+ state->smb2req = smb2req;
+
+ /*
+ * TODO: cancel all outstanding requests on the session
+ */
+ status = smbXsrv_session_logoff(state->smb2req->session);
+ if (tevent_req_nterror(req, status)) {
+ return tevent_req_post(req, ev);
}
/*
* we may need to sign the response, so we need to keep
* the session until the response is sent to the wire.
*/
- talloc_steal(req, req->session);
-
- outbody = smbd_smb2_generate_outbody(req, 0x04);
- if (outbody.data == NULL) {
- return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
- }
+ talloc_steal(state->smb2req, state->smb2req->session);
- SSVAL(outbody.data, 0x00, 0x04); /* struct size */
- SSVAL(outbody.data, 0x02, 0); /* reserved */
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+}
- return smbd_smb2_request_done(req, outbody, NULL);
+static NTSTATUS smbd_smb2_logoff_recv(struct tevent_req *req)
+{
+ return tevent_req_simple_recv_ntstatus(req);
}
--
1.9.0.279.gdc9e3eb
From f26a23b543505a6d4be5e9e5f37698b5d1c2f5ed Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 10 Mar 2014 09:53:18 +0100
Subject: [PATCH 5/9] s3:smb2_tcon: split smbd_smb2_tdis into an async
*_send/recv pair.
Bug: https://bugzilla.samba.org/show_bug.cgi?id=10344
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_tcon.c | 105 +++++++++++++++++++++++++++++++++++++++--------
1 file changed, 89 insertions(+), 16 deletions(-)
diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c
index e1f0987..0e9c6f6 100644
--- a/source3/smbd/smb2_tcon.c
+++ b/source3/smbd/smb2_tcon.c
@@ -411,40 +411,113 @@ static NTSTATUS smbd_smb2_tree_connect_recv(struct tevent_req *req,
return NT_STATUS_OK;
}
+static struct tevent_req *smbd_smb2_tdis_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct smbd_smb2_request *smb2req);
+static NTSTATUS smbd_smb2_tdis_recv(struct tevent_req *req);
+static void smbd_smb2_request_tdis_done(struct tevent_req *subreq);
+
NTSTATUS smbd_smb2_request_process_tdis(struct smbd_smb2_request *req)
{
NTSTATUS status;
- DATA_BLOB outbody;
+ struct tevent_req *subreq = NULL;
status = smbd_smb2_request_verify_sizes(req, 0x04);
if (!NT_STATUS_IS_OK(status)) {
return smbd_smb2_request_error(req, status);
}
+ subreq = smbd_smb2_tdis_send(req, req->sconn->ev_ctx, req);
+ if (subreq == NULL) {
+ return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
+ }
+ tevent_req_set_callback(subreq, smbd_smb2_request_tdis_done, req);
+
/*
- * TODO: cancel all outstanding requests on the tcon
+ * Wait a long time before going async on this to allow
+ * requests we're waiting on to finish. Set timeout to 10 secs.
*/
- status = smbXsrv_tcon_disconnect(req->tcon, req->tcon->compat->vuid);
+ return smbd_smb2_request_pending_queue(req, subreq, 10000000);
+}
+
+static void smbd_smb2_request_tdis_done(struct tevent_req *subreq)
+{
+ struct smbd_smb2_request *smb2req =
+ tevent_req_callback_data(subreq,
+ struct smbd_smb2_request);
+ DATA_BLOB outbody;
+ NTSTATUS status;
+ NTSTATUS error;
+
+ status = smbd_smb2_tdis_recv(subreq);
+ TALLOC_FREE(subreq);
if (!NT_STATUS_IS_OK(status)) {
- DEBUG(0, ("smbd_smb2_request_process_tdis: "
- "smbXsrv_tcon_disconnect() failed: %s\n",
- nt_errstr(status)));
- /*
- * If we hit this case, there is something completely
- * wrong, so we better disconnect the transport connection.
- */
- return status;
+ error = smbd_smb2_request_error(smb2req, status);
+ if (!NT_STATUS_IS_OK(error)) {
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(error));
+ return;
+ }
+ return;
}
- TALLOC_FREE(req->tcon);
-
- outbody = smbd_smb2_generate_outbody(req, 0x04);
+ outbody = smbd_smb2_generate_outbody(smb2req, 0x04);
if (outbody.data == NULL) {
- return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
+ error = smbd_smb2_request_error(smb2req, NT_STATUS_NO_MEMORY);
+ if (!NT_STATUS_IS_OK(error)) {
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(error));
+ return;
+ }
+ return;
}
SSVAL(outbody.data, 0x00, 0x04); /* struct size */
SSVAL(outbody.data, 0x02, 0); /* reserved */
- return smbd_smb2_request_done(req, outbody, NULL);
+ error = smbd_smb2_request_done(smb2req, outbody, NULL);
+ if (!NT_STATUS_IS_OK(error)) {
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(error));
+ return;
+ }
+}
+
+struct smbd_smb2_tdis_state {
+ struct smbd_smb2_request *smb2req;
+};
+
+static struct tevent_req *smbd_smb2_tdis_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct smbd_smb2_request *smb2req)
+{
+ struct tevent_req *req;
+ struct smbd_smb2_tdis_state *state;
+ NTSTATUS status;
+
+ req = tevent_req_create(mem_ctx, &state,
+ struct smbd_smb2_tdis_state);
+ if (req == NULL) {
+ return NULL;
+ }
+ state->smb2req = smb2req;
+
+ /*
+ * TODO: cancel all outstanding requests on the tcon
+ */
+ status = smbXsrv_tcon_disconnect(state->smb2req->tcon,
+ state->smb2req->tcon->compat->vuid);
+ if (tevent_req_nterror(req, status)) {
+ return tevent_req_post(req, ev);
+ }
+
+ /* We did tear down the tcon. */
+ TALLOC_FREE(state->smb2req->tcon);
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+}
+
+static NTSTATUS smbd_smb2_tdis_recv(struct tevent_req *req)
+{
+ return tevent_req_simple_recv_ntstatus(req);
}
--
1.9.0.279.gdc9e3eb
From a6644eb333e27c0d3dfc6fee82e7bed9f1fad7ba Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 10 Mar 2014 09:53:18 +0100
Subject: [PATCH 6/9] s3:smb2_sesssetup: cancel and wait for pending requests
on logoff
Bug: https://bugzilla.samba.org/show_bug.cgi?id=10344
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_sesssetup.c | 82 ++++++++++++++++++++++++++++++++++++++++---
1 file changed, 78 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index 786bfe8..391b100 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -862,15 +862,19 @@ static void smbd_smb2_request_logoff_done(struct tevent_req *subreq)
struct smbd_smb2_logout_state {
struct smbd_smb2_request *smb2req;
+ struct tevent_queue *wait_queue;
};
+static void smbd_smb2_logoff_wait_done(struct tevent_req *subreq);
+
static struct tevent_req *smbd_smb2_logoff_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct smbd_smb2_request *smb2req)
{
struct tevent_req *req;
struct smbd_smb2_logout_state *state;
- NTSTATUS status;
+ struct tevent_req *subreq;
+ struct smbd_smb2_request *preq;
req = tevent_req_create(mem_ctx, &state,
struct smbd_smb2_logout_state);
@@ -879,12 +883,83 @@ static struct tevent_req *smbd_smb2_logoff_send(TALLOC_CTX *mem_ctx,
}
state->smb2req = smb2req;
+ state->wait_queue = tevent_queue_create(state, "logoff_wait_queue");
+ if (tevent_req_nomem(state->wait_queue, req)) {
+ return tevent_req_post(req, ev);
+ }
+
/*
- * TODO: cancel all outstanding requests on the session
+ * Make sure that no new request will be able to use this session.
*/
+ smb2req->session->status = NT_STATUS_USER_SESSION_DELETED;
+
+ for (preq = smb2req->sconn->smb2.requests; preq != NULL; preq = preq->next) {
+ if (preq == smb2req) {
+ /* Can't cancel current request. */
+ continue;
+ }
+ if (preq->session != smb2req->session) {
+ /* Request on different session. */
+ continue;
+ }
+
+ /*
+ * Never cancel anything in a compound
+ * request. Way too hard to deal with
+ * the result.
+ */
+ if (!preq->compound_related && preq->subreq != NULL) {
+ tevent_req_cancel(preq->subreq);
+ }
+
+ /*
+ * Now wait until the request is finished.
+ *
+ * We don't set a callback, as we just want to block the
+ * wait queue and the talloc_free() of the request will
+ * remove the item from the wait queue.
+ */
+ subreq = tevent_queue_wait_send(preq, ev, state->wait_queue);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ }
+
+ /*
+ * Now we add our own waiter to the end of the queue,
+ * this way we get notified when all pending requests are finished
+ * and send to the socket.
+ */
+ subreq = tevent_queue_wait_send(state, ev, state->wait_queue);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, smbd_smb2_logoff_wait_done, req);
+
+ return req;
+}
+
+static void smbd_smb2_logoff_wait_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct smbd_smb2_logout_state *state = tevent_req_data(
+ req, struct smbd_smb2_logout_state);
+ NTSTATUS status;
+
+ tevent_queue_wait_recv(subreq);
+ TALLOC_FREE(subreq);
+
+ /*
+ * As we've been awoken, we may have changed
+ * uid in the meantime. Ensure we're still
+ * root (SMB2_OP_LOGOFF has .as_root = true).
+ */
+ change_to_root_user();
+
status = smbXsrv_session_logoff(state->smb2req->session);
if (tevent_req_nterror(req, status)) {
- return tevent_req_post(req, ev);
+ return;
}
/*
@@ -894,7 +969,6 @@ static struct tevent_req *smbd_smb2_logoff_send(TALLOC_CTX *mem_ctx,
talloc_steal(state->smb2req, state->smb2req->session);
tevent_req_done(req);
- return tevent_req_post(req, ev);
}
static NTSTATUS smbd_smb2_logoff_recv(struct tevent_req *req)
--
1.9.0.279.gdc9e3eb
From 0966c24ebee7df5d1ca3b463d674535618cb8b67 Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Mon, 10 Mar 2014 09:53:18 +0100
Subject: [PATCH 7/9] s3:smb2_tcon: cancel and wait for pending requests on
tdis
Bug: https://bugzilla.samba.org/show_bug.cgi?id=10344
Signed-off-by: Stefan Metzmacher <metze at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
source3/smbd/smb2_tcon.c | 82 +++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 78 insertions(+), 4 deletions(-)
diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c
index 0e9c6f6..93f62fd 100644
--- a/source3/smbd/smb2_tcon.c
+++ b/source3/smbd/smb2_tcon.c
@@ -485,15 +485,19 @@ static void smbd_smb2_request_tdis_done(struct tevent_req *subreq)
struct smbd_smb2_tdis_state {
struct smbd_smb2_request *smb2req;
+ struct tevent_queue *wait_queue;
};
+static void smbd_smb2_tdis_wait_done(struct tevent_req *subreq);
+
static struct tevent_req *smbd_smb2_tdis_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct smbd_smb2_request *smb2req)
{
struct tevent_req *req;
struct smbd_smb2_tdis_state *state;
- NTSTATUS status;
+ struct tevent_req *subreq;
+ struct smbd_smb2_request *preq;
req = tevent_req_create(mem_ctx, &state,
struct smbd_smb2_tdis_state);
@@ -502,19 +506,89 @@ static struct tevent_req *smbd_smb2_tdis_send(TALLOC_CTX *mem_ctx,
}
state->smb2req = smb2req;
+ state->wait_queue = tevent_queue_create(state, "tdis_wait_queue");
+ if (tevent_req_nomem(state->wait_queue, req)) {
+ return tevent_req_post(req, ev);
+ }
+
+ /*
+ * Make sure that no new request will be able to use this tcon.
+ */
+ smb2req->tcon->status = NT_STATUS_NETWORK_NAME_DELETED;
+
+ for (preq = smb2req->sconn->smb2.requests; preq != NULL; preq = preq->next) {
+ if (preq == smb2req) {
+ /* Can't cancel current request. */
+ continue;
+ }
+ if (preq->tcon != smb2req->tcon) {
+ /* Request on different tcon. */
+ continue;
+ }
+
+ /*
+ * Never cancel anything in a compound
+ * request. Way too hard to deal with
+ * the result.
+ */
+ if (!preq->compound_related && preq->subreq != NULL) {
+ tevent_req_cancel(preq->subreq);
+ }
+
+ /*
+ * Now wait until the request is finished.
+ *
+ * We don't set a callback, as we just want to block the
+ * wait queue and the talloc_free() of the request will
+ * remove the item from the wait queue.
+ */
+ subreq = tevent_queue_wait_send(preq, ev, state->wait_queue);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ }
+
/*
- * TODO: cancel all outstanding requests on the tcon
+ * Now we add our own waiter to the end of the queue,
+ * this way we get notified when all pending requests are finished
+ * and send to the socket.
*/
+ subreq = tevent_queue_wait_send(state, ev, state->wait_queue);
+ if (tevent_req_nomem(subreq, req)) {
+ return tevent_req_post(req, ev);
+ }
+ tevent_req_set_callback(subreq, smbd_smb2_tdis_wait_done, req);
+
+ return req;
+}
+
+static void smbd_smb2_tdis_wait_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct smbd_smb2_tdis_state *state = tevent_req_data(
+ req, struct smbd_smb2_tdis_state);
+ NTSTATUS status;
+
+ tevent_queue_wait_recv(subreq);
+ TALLOC_FREE(subreq);
+
+ /*
+ * As we've been awoken, we may have changed
+ * uid in the meantime. Ensure we're still
+ * root (SMB2_OP_TDIS has .as_root = true).
+ */
+ change_to_root_user();
+
status = smbXsrv_tcon_disconnect(state->smb2req->tcon,
state->smb2req->tcon->compat->vuid);
if (tevent_req_nterror(req, status)) {
- return tevent_req_post(req, ev);
+ return;
}
/* We did tear down the tcon. */
TALLOC_FREE(state->smb2req->tcon);
tevent_req_done(req);
- return tevent_req_post(req, ev);
}
static NTSTATUS smbd_smb2_tdis_recv(struct tevent_req *req)
--
1.9.0.279.gdc9e3eb
From 3148ce076382e2b14c6b8b8f274f6e9702d4e8bc Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 28 Jan 2014 14:07:26 -0800
Subject: [PATCH 8/9] s4: smbtorture: Update the torture_smb2_notify_ulogoff
test to demonstrate the problem.
[Bug 10344] SessionLogoff on a signed connection with an outstanding notify request crashes smbd.
https://bugzilla.samba.org/show_bug.cgi?id=10344
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source4/torture/smb2/notify.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c
index e83b099..e9664d3 100644
--- a/source4/torture/smb2/notify.c
+++ b/source4/torture/smb2/notify.c
@@ -69,6 +69,13 @@
goto done; \
}} while (0)
+#define WAIT_FOR_ASYNC_RESPONSE(req) \
+ while (!req->cancel.can_cancel && req->state <= SMB2_REQUEST_RECV) { \
+ if (tevent_loop_once(torture->ev) != 0) { \
+ break; \
+ } \
+ }
+
#define BASEDIR "test_notify"
#define FNAME "smb2-notify01.dat"
@@ -1280,7 +1287,7 @@ static bool torture_smb2_notify_ulogoff(struct torture_context *torture,
CHECK_STATUS(status, NT_STATUS_OK);
io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN;
- status = smb2_create(tree2, torture, &(io.smb2));
+ status = smb2_create(tree1, torture, &(io.smb2));
CHECK_STATUS(status, NT_STATUS_OK);
h1 = io.smb2.out.file.handle;
@@ -1295,7 +1302,9 @@ static bool torture_smb2_notify_ulogoff(struct torture_context *torture,
req = smb2_notify_send(tree1, &(notify.smb2));
- status = smb2_logoff(tree2->session);
+ WAIT_FOR_ASYNC_RESPONSE(req);
+
+ status = smb2_logoff(tree1->session);
CHECK_STATUS(status, NT_STATUS_OK);
status = smb2_notify_recv(req, torture, &(notify.smb2));
--
1.9.0.279.gdc9e3eb
From c0b571756a8a8b44eef1056ba46c72d8082d7237 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 24 Feb 2014 10:44:59 -0800
Subject: [PATCH 9/9] s4: smbtorture: Add a proper change_notify going async
followed by tdis test.
[Bug 10344] SessionLogoff on a signed connection with an outstanding notify request crashes smbd.
https://bugzilla.samba.org/show_bug.cgi?id=10344
Signed-off-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Stefan Metzmacher <metze at samba.org>
---
source4/torture/smb2/notify.c | 68 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c
index e9664d3..650337e 100644
--- a/source4/torture/smb2/notify.c
+++ b/source4/torture/smb2/notify.c
@@ -1195,7 +1195,7 @@ static bool torture_smb2_notify_tree_disconnect(
smb2_deltree(tree, BASEDIR);
smb2_util_rmdir(tree, BASEDIR);
- torture_comment(torture, "TESTING CHANGE NOTIFY FOLLOWED BY "
+ torture_comment(torture, "TESTING CHANGE NOTIFY+CANCEL FOLLOWED BY "
"TREE-DISCONNECT\n");
/*
@@ -1247,6 +1247,71 @@ done:
}
/*
+ testing of change notifies followed by a tdis - no cancel
+*/
+
+static bool torture_smb2_notify_tree_disconnect_1(
+ struct torture_context *torture,
+ struct smb2_tree *tree)
+{
+ bool ret = true;
+ NTSTATUS status;
+ union smb_notify notify;
+ union smb_open io;
+ struct smb2_handle h1;
+ struct smb2_request *req;
+
+ smb2_deltree(tree, BASEDIR);
+ smb2_util_rmdir(tree, BASEDIR);
+
+ torture_comment(torture, "TESTING CHANGE NOTIFY ASYNC FOLLOWED BY "
+ "TREE-DISCONNECT\n");
+
+ /*
+ get a handle on the directory
+ */
+ ZERO_STRUCT(io.smb2);
+ io.generic.level = RAW_OPEN_SMB2;
+ io.smb2.in.create_flags = 0;
+ io.smb2.in.desired_access = SEC_FILE_ALL;
+ io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
+ io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
+ io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE;
+ io.smb2.in.alloc_size = 0;
+ io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE;
+ io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS;
+ io.smb2.in.security_flags = 0;
+ io.smb2.in.fname = BASEDIR;
+
+ status = smb2_create(tree, torture, &(io.smb2));
+ CHECK_STATUS(status, NT_STATUS_OK);
+ h1 = io.smb2.out.file.handle;
+
+ /* ask for a change notify,
+ on file or directory name changes */
+ ZERO_STRUCT(notify.smb2);
+ notify.smb2.level = RAW_NOTIFY_SMB2;
+ notify.smb2.in.buffer_size = 1000;
+ notify.smb2.in.completion_filter = FILE_NOTIFY_CHANGE_NAME;
+ notify.smb2.in.file.handle = h1;
+ notify.smb2.in.recursive = true;
+
+ req = smb2_notify_send(tree, &(notify.smb2));
+ WAIT_FOR_ASYNC_RESPONSE(req);
+
+ status = smb2_tdis(tree);
+ CHECK_STATUS(status, NT_STATUS_OK);
+
+ status = smb2_notify_recv(req, torture, &(notify.smb2));
+ CHECK_VAL(notify.smb2.out.num_changes, 0);
+
+done:
+ smb2_deltree(tree, BASEDIR);
+ return ret;
+}
+
+/*
basic testing of change notifies followed by a ulogoff
*/
@@ -2000,6 +2065,7 @@ struct torture_suite *torture_smb2_notify_init(void)
torture_suite_add_2smb2_test(suite, "dir", torture_smb2_notify_dir);
torture_suite_add_2smb2_test(suite, "mask", torture_smb2_notify_mask);
torture_suite_add_1smb2_test(suite, "tdis", torture_smb2_notify_tree_disconnect);
+ torture_suite_add_1smb2_test(suite, "tdis1", torture_smb2_notify_tree_disconnect_1);
torture_suite_add_2smb2_test(suite, "mask-change", torture_smb2_notify_mask_change);
torture_suite_add_2smb2_test(suite, "logoff", torture_smb2_notify_ulogoff);
torture_suite_add_1smb2_test(suite, "tree", torture_smb2_notify_tree);
--
1.9.0.279.gdc9e3eb
More information about the samba-technical
mailing list