[PATCH] Patchset for bug #10344 - SessionLogoff on a signed connection with an outstanding notify request crashes smbd.
Jeremy Allison
jra at samba.org
Tue Feb 25 15:56:53 MST 2014
Hi all,
Here is a patchset to fix a bug Codenomicon
scans found in our SMB2/3 implementation.
[Bug 10344] SessionLogoff on a signed connection with an outstanding notify request crashes smbd.
https://bugzilla.samba.org/show_bug.cgi?id=10344
Sending a SMB2 Notify that goes async
followed by a SessionLogoff on a signed
connection will crash the server.
The problem is we don't cancel the
outstanding requests for a session
when we receive a SessionLogoff or
a TreeDisconnect.
This patchset implements this by
making both SessionLogoff and
TreeDisconnect in SMB2/3 asynchronous.
On receipt of a SessionLogoff or
TreeDisconnect is looks over the
pending request list and calls
tevent_req_cancel() on any cancelable
requests (it ignores any that are
not cancelable, but all requests
that can go async are cancelable).
It then reschedules itself and
re-scans when it's rescheduled,
and if all cancelable requests
are finished it then replies.
If there are no pending requests
it just replies as normal with
a tevent_req_done(req); return tevent_req_post(req, ev);
pair at the end of the _send
function.
Finally I've added two tests
that excersise these functions
with pending requests outstanding
in the smb2.notify smbtorture
test.
I'm pretty happy with this patchset,
as it fixes a longstanding issue.
Reviews appreciated !
Thanks,
Jeremy
-------------- next part --------------
From 078b999ec6b3b6a82c72dd63f973ff90af9fe413 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 13 Jan 2014 14:12:18 -0800
Subject: [PATCH 1/6] s3: SMB2 sessionsetup - factor code out into a function
remove_outstanding_session_refs().
We will be reusing this.
[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>
---
source3/smbd/smb2_sesssetup.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index edef0cc..e63a244 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -455,10 +455,30 @@ static int pp_self_ref_destructor(struct smbd_smb2_session_setup_state **pp_stat
return 0;
}
-static int smbd_smb2_session_setup_state_destructor(struct smbd_smb2_session_setup_state *state)
+static void remove_outstanding_session_refs(struct smbd_smb2_request *smb2req)
{
struct smbd_smb2_request *preq;
+ for (preq = smb2req->sconn->smb2.requests; preq != NULL; preq = preq->next) {
+ if (preq == smb2req) {
+ /* Don't remove the session from the current
+ request in flight. */
+ continue;
+ }
+ if (preq->session == smb2req->session) {
+ preq->session = NULL;
+ /*
+ * If we no longer have a session we can't
+ * sign or encrypt replies.
+ */
+ preq->do_signing = false;
+ preq->do_encryption = false;
+ }
+ }
+}
+
+static int smbd_smb2_session_setup_state_destructor(struct smbd_smb2_session_setup_state *state)
+{
/*
* If state->session is not NULL,
* we move the session from the session table to the request on failure
@@ -479,21 +499,7 @@ static int smbd_smb2_session_setup_state_destructor(struct smbd_smb2_session_set
* to it.
*/
- for (preq = state->smb2req->sconn->smb2.requests; preq != NULL; preq = preq->next) {
- if (preq == state->smb2req) {
- continue;
- }
- if (preq->session == state->smb2req->session) {
- preq->session = NULL;
- /*
- * If we no longer have a session we can't
- * sign or encrypt replies.
- */
- preq->do_signing = false;
- preq->do_encryption = false;
- }
- }
-
+ remove_outstanding_session_refs(state->smb2req);
return 0;
}
--
1.9.0.rc1.175.g0b1dcb5
From 07039a9ef0d6ac25d9afd8b0445955e3dc95457d Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 13 Jan 2014 14:31:31 -0800
Subject: [PATCH 2/6] s3: SMB2 sessionsetup - add
cancel_outstanding_session_requests().
Walks the outstanding requests on this session (not including
the current request being processed) and call cancel on the
first one we haven't already cancelled. Returns true if there
were any requests cancelled or if there are still outstanding
requests on the queue that we have alreadt called cancel on,
false if not. Stores the cancelled request in an array.
Not yet used.
[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>
---
source3/smbd/smb2_sesssetup.c | 86 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index e63a244..7dbf603 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -794,6 +794,92 @@ static NTSTATUS smbd_smb2_session_setup_recv(struct tevent_req *req,
return status;
}
+/********************************************************
+ Walk the outstanding requests on this session (not including
+ the current request being processed) and call cancel on
+ the first request that has not yet been cancelled. Store the
+ cancelled message-id. Returns true if a request was cancelled,
+ or there are still unfinished requests on the queue that were
+ cancelled, false otherwise.
+********************************************************/
+
+struct already_cancelled {
+ uint64_t *already_cancelled_array;
+ unsigned int num_already_cancelled;
+};
+
+static bool cancel_outstanding_session_requests(const struct smbd_smb2_request *smb2req,
+ struct already_cancelled *pc)
+{
+ bool ret = false;
+ struct smbd_smb2_request *preq;
+
+ if (smb2req->session == NULL) {
+ return false;
+ }
+ for (preq = smb2req->sconn->smb2.requests; preq != NULL; preq = preq->next) {
+ const uint8_t *outhdr;
+ uint64_t message_id;
+ unsigned i;
+
+ if (preq == smb2req) {
+ /* Can't cancel current request. */
+ continue;
+ }
+ if (preq->session != smb2req->session) {
+ /* Request on different session. */
+ continue;
+ }
+
+ if (preq->compound_related) {
+ /*
+ * Never cancel anything in a compound
+ * request. Way too hard to deal with
+ * the result.
+ */
+ continue;
+ }
+
+ /* If we get here we've either found a request
+ that we already cancelled, or a request we're
+ going to cancel. Either way we return true
+ so the caller will wait for completion. */
+
+ ret = true;
+
+ outhdr = SMBD_SMB2_OUT_HDR_PTR(preq);
+ message_id = BVAL(outhdr, SMB2_HDR_MESSAGE_ID);
+ for (i = 0; i < pc->num_already_cancelled; i++) {
+ if (message_id == pc->already_cancelled_array[i]) {
+ break;
+ }
+ }
+ if (i < pc->num_already_cancelled) {
+ /* We've called tevent_req_cancel
+ on this request before. */
+ continue;
+ }
+
+ tevent_req_cancel(preq->subreq);
+
+ /* Add the newly cancelled message-id to the array. */
+ pc->already_cancelled_array = talloc_realloc(pc,
+ pc->already_cancelled_array,
+ uint64_t,
+ pc->num_already_cancelled + 1);
+ if (pc->already_cancelled_array == NULL) {
+ /* Fatal error. */
+ smb_panic("talloc fail\n");
+ /* NOTREACHED. */
+ return ret;
+ }
+ pc->already_cancelled_array[pc->num_already_cancelled] = message_id;
+ pc->num_already_cancelled++;
+ return ret;
+ }
+ return ret;
+}
+
NTSTATUS smbd_smb2_request_process_logoff(struct smbd_smb2_request *req)
{
NTSTATUS status;
--
1.9.0.rc1.175.g0b1dcb5
From 17be9fdb25e4e9ddf5eded8ecd6099a3b368a814 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 27 Jan 2014 15:19:35 -0800
Subject: [PATCH 3/6] Make smbd_smb2_request_process_logoff() async.
Add smbd_smb2_logoff_send()/smbd_smb2_logoff_recv()/smbd_smb2_request_logoff_done()
and make smbd_smb2_logoff_send() re-entrant. First call to smbd_smb2_logoff_send()
calls tevent_req_cancel() on any outstanding requests on this session
then re-schedules itself to be processed after the canceled requests
finish. It waits 30 seconds, re-scheduling itself until all events
have been canceled.
[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>
---
source3/smbd/smb2_sesssetup.c | 178 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 158 insertions(+), 20 deletions(-)
diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
index 7dbf603..3763a0c 100644
--- a/source3/smbd/smb2_sesssetup.c
+++ b/source3/smbd/smb2_sesssetup.c
@@ -880,44 +880,182 @@ static bool cancel_outstanding_session_requests(const struct smbd_smb2_request *
return ret;
}
+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);
+ /* Ensure we don't reply with an async message on this call -
+ set timeout to 120 secs. */
+ return smbd_smb2_request_pending_queue(req, subreq, 120000000);
+}
+
+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)) {
+ 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 = data_blob_talloc(smb2req->out.vector, NULL, 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 already_cancelled *acp;
+ struct timeval end_logout;
+};
+
+static struct tevent_req *smbd_smb2_logoff_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct smbd_smb2_request *smb2req)
+{
+ NTSTATUS status;
+ struct tevent_req *req;
+ struct smbd_smb2_logout_state *state;
+
+ req = smb2req->subreq;
+ if (req == NULL) {
+ /* New logoff call. */
+ req = tevent_req_create(mem_ctx, &state,
+ struct smbd_smb2_logout_state);
+ if (req == NULL) {
+ return NULL;
+ }
+
+ state->acp = talloc_zero(state, struct already_cancelled);
+ if (state->acp == NULL) {
+ return NULL;
+ }
+
+ /* Don't wait more than 30 seconds for cancel.
+ So remember time now + 30 seconds. If we are
+ re-scheduled after this the cancel requests
+ took too long and we exit. */
+ state->end_logout = tevent_timeval_current_ofs(30, 0);
+ } else {
+ /* Re-entrant logoff call - we are being rescheduled
+ after we're requested other requests are canceled. */
+ struct timeval curr = tevent_timeval_current();
+ state = tevent_req_data(req, struct smbd_smb2_logout_state);
+
+ if (tevent_timeval_compare(&curr, &state->end_logout) >= 0) {
+ /* Ran out of time - terminate. */
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(NT_STATUS_DRIVER_CANCEL_TIMEOUT));
+ return NULL;
+ }
+
+ /* We're re-entrant. We must clear the callback
+ data in case we finish here with tevent_done.
+ If we re-schedule, caller will reset callback
+ correctly. */
+ tevent_req_set_callback(req, NULL, NULL);
+ }
+
/*
- * TODO: cancel all outstanding requests on the session
+ * Try and cancel a request. If we succeed, reschedule ourselves.
*/
- status = smbXsrv_session_logoff(req->session);
- 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;
+ if (cancel_outstanding_session_requests(smb2req, state->acp)) {
+ struct tevent_immediate *im = tevent_create_immediate(smb2req);
+ if (!im) {
+ return NULL;
+ }
+ /* Try again later. */
+ tevent_schedule_immediate(im,
+ smb2req->sconn->ev_ctx,
+ smbd_smb2_request_dispatch_immediate,
+ smb2req);
+ return req;
+ }
+
+ /*
+ * When we get here, we are actually going to tear
+ * down the sesssion pointer.
+ */
+ status = smbXsrv_session_logoff(smb2req->session);
+ if (tevent_req_nterror(req, status)) {
+ DEBUG(0, ("smbXsrv_session_logoff() failed: %s\n",
+ nt_errstr(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);
+ talloc_steal(smb2req, smb2req->session);
- outbody = data_blob_talloc(req->out.vector, NULL, 0x04);
- if (outbody.data == NULL) {
- return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
- }
+ /*
+ * The return from smbXsrv_session_logoff()
+ * leaves all outstanding pointers to
+ * smb2req->session not on this request in an
+ * undefined state, ensure we don't try and
+ * access any req->session pointers once
+ * we return.
+ */
+ remove_outstanding_session_refs(smb2req);
- SSVAL(outbody.data, 0x00, 0x04); /* struct size */
- SSVAL(outbody.data, 0x02, 0); /* reserved */
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+}
+
+static NTSTATUS smbd_smb2_logoff_recv(struct tevent_req *req)
+{
+ NTSTATUS status;
- return smbd_smb2_request_done(req, outbody, NULL);
+ if (tevent_req_is_nterror(req, &status)) {
+ tevent_req_received(req);
+ return status;
+ }
+ tevent_req_received(req);
+ return NT_STATUS_OK;
}
--
1.9.0.rc1.175.g0b1dcb5
From c65f802a9b0f6734a2898dcb35e7539aabf9c2cd Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 28 Jan 2014 13:33:06 -0800
Subject: [PATCH 4/6] Make smbd_smb2_request_process_tdis() async.
Add smbd_smb2_tdis_send()/smbd_smb2_tids_recv()/smbd_smb2_request_tids_done()
and make smbd_smb2_tdis_send() re-entrant. First call to smbd_smb2_tids_send()
calls tevent_req_cancel() on any outstanding requests on this session
then re-schedules itself to be processed after the canceled requests
finish. It waits 30 seconds, re-scheduling itself until all events
have been canceled.
[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>
---
source3/smbd/smb2_tcon.c | 237 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 225 insertions(+), 12 deletions(-)
diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c
index c7dc928..3e072ff 100644
--- a/source3/smbd/smb2_tcon.c
+++ b/source3/smbd/smb2_tcon.c
@@ -411,20 +411,226 @@ 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);
+ /* Ensure we don't reply with an async message on this call -
+ set timeout to 120 secs. */
+ return smbd_smb2_request_pending_queue(req, subreq, 120000000);
+}
+
+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)) {
+ 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 = data_blob_talloc(smb2req->out.vector, NULL, 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 already_cancelled {
+ uint64_t *already_cancelled_array;
+ unsigned int num_already_cancelled;
+};
+
+static bool cancel_outstanding_tcon_requests(const struct smbd_smb2_request *smb2req,
+ struct already_cancelled *pc)
+{
+ bool ret = false;
+ struct smbd_smb2_request *preq;
+
+ if (smb2req->tcon == NULL) {
+ return false;
+ }
+ for (preq = smb2req->sconn->smb2.requests; preq != NULL; preq = preq->next) {
+ const uint8_t *outhdr;
+ uint64_t message_id;
+ unsigned i;
+
+ if (preq == smb2req) {
+ /* Can't cancel current request. */
+ continue;
+ }
+ if (preq->tcon != smb2req->tcon) {
+ /* Request on different tcon. */
+ continue;
+ }
+
+ if (preq->compound_related) {
+ /*
+ * Never cancel anything in a compound
+ * request. Way too hard to deal with
+ * the result.
+ */
+ continue;
+ }
+
+ /* If we get here we've either found a request
+ that we already cancelled, or a request we're
+ going to cancel. Either way we return true
+ so the caller will wait for completion. */
+
+ ret = true;
+
+ outhdr = SMBD_SMB2_OUT_HDR_PTR(preq);
+ message_id = BVAL(outhdr, SMB2_HDR_MESSAGE_ID);
+ for (i = 0; i < pc->num_already_cancelled; i++) {
+ if (message_id == pc->already_cancelled_array[i]) {
+ break;
+ }
+ }
+ if (i < pc->num_already_cancelled) {
+ /* We've called tevent_req_cancel
+ on this request before. */
+ continue;
+ }
+
+ tevent_req_cancel(preq->subreq);
+
+ /* Add the newly cancelled message-id to the array. */
+ pc->already_cancelled_array = talloc_realloc(pc,
+ pc->already_cancelled_array,
+ uint64_t,
+ pc->num_already_cancelled + 1);
+ if (pc->already_cancelled_array == NULL) {
+ /* Fatal error. */
+ smb_panic("talloc fail\n");
+ /* NOTREACHED. */
+ return ret;
+ }
+ pc->already_cancelled_array[pc->num_already_cancelled] = message_id;
+ pc->num_already_cancelled++;
+ return ret;
+ }
+ return ret;
+}
+
+struct smbd_smb2_tdis_state {
+ struct already_cancelled *acp;
+ struct timeval end_logout;
+};
+
+static struct tevent_req *smbd_smb2_tdis_send(TALLOC_CTX *mem_ctx,
+ struct tevent_context *ev,
+ struct smbd_smb2_request *smb2req)
+{
+ NTSTATUS status;
+ struct tevent_req *req;
+ struct smbd_smb2_tdis_state *state;
+
+ req = smb2req->subreq;
+ if (req == NULL) {
+ /* New tdis call. */
+ req = tevent_req_create(mem_ctx, &state,
+ struct smbd_smb2_tdis_state);
+ if (req == NULL) {
+ return NULL;
+ }
+
+ state->acp = talloc_zero(state, struct already_cancelled);
+ if (state->acp == NULL) {
+ return NULL;
+ }
+
+ /* Don't wait more than 30 seconds for cancel.
+ So remember time now + 30 seconds. If we are
+ re-scheduled after this the cancel requests
+ took too long and we exit. */
+ state->end_logout = tevent_timeval_current_ofs(30, 0);
+ } else {
+ /* Re-entrant tdis call - we are being rescheduled
+ after we're requested other requests are canceled. */
+ struct timeval curr = tevent_timeval_current();
+ state = tevent_req_data(req, struct smbd_smb2_tdis_state);
+
+ if (tevent_timeval_compare(&curr, &state->end_logout) >= 0) {
+ /* Ran out of time - terminate. */
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(NT_STATUS_DRIVER_CANCEL_TIMEOUT));
+ return NULL;
+ }
+
+ /* We're re-entrant. We must clear the callback
+ data in case we finish here with tevent_done.
+ If we re-schedule, caller will reset callback
+ correctly. */
+ tevent_req_set_callback(req, NULL, NULL);
+ }
+
/*
- * TODO: cancel all outstanding requests on the tcon
+ * Try and cancel a request. If we succeed, reschedule ourselves.
*/
- status = smbXsrv_tcon_disconnect(req->tcon, req->tcon->compat->vuid);
+ if (cancel_outstanding_tcon_requests(smb2req, state->acp)) {
+ struct tevent_immediate *im = tevent_create_immediate(smb2req);
+ if (!im) {
+ return NULL;
+ }
+ /* Try again later. */
+ tevent_schedule_immediate(im,
+ smb2req->sconn->ev_ctx,
+ smbd_smb2_request_dispatch_immediate,
+ smb2req);
+ return req;
+ }
+
+ /*
+ * When we get here, we are actually going to tear
+ * down the tcon pointer.
+ */
+
+ status = smbXsrv_tcon_disconnect(smb2req->tcon, smb2req->tcon->compat->vuid);
if (!NT_STATUS_IS_OK(status)) {
DEBUG(0, ("smbd_smb2_request_process_tdis: "
"smbXsrv_tcon_disconnect() failed: %s\n",
@@ -433,18 +639,25 @@ NTSTATUS smbd_smb2_request_process_tdis(struct smbd_smb2_request *req)
* If we hit this case, there is something completely
* wrong, so we better disconnect the transport connection.
*/
- return status;
+ smbd_server_connection_terminate(smb2req->sconn,
+ nt_errstr(status));
+ return NULL;
}
- TALLOC_FREE(req->tcon);
+ TALLOC_FREE(smb2req->tcon);
- outbody = data_blob_talloc(req->out.vector, NULL, 0x04);
- if (outbody.data == NULL) {
- return smbd_smb2_request_error(req, NT_STATUS_NO_MEMORY);
- }
+ tevent_req_done(req);
+ return tevent_req_post(req, ev);
+}
- SSVAL(outbody.data, 0x00, 0x04); /* struct size */
- SSVAL(outbody.data, 0x02, 0); /* reserved */
+static NTSTATUS smbd_smb2_tdis_recv(struct tevent_req *req)
+{
+ NTSTATUS status;
- return smbd_smb2_request_done(req, outbody, NULL);
+ if (tevent_req_is_nterror(req, &status)) {
+ tevent_req_received(req);
+ return status;
+ }
+ tevent_req_received(req);
+ return NT_STATUS_OK;
}
--
1.9.0.rc1.175.g0b1dcb5
From 4c14c6f49e9091ee05acdf3ea64ae9d392dd54ed 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 5/6] 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>
---
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.rc1.175.g0b1dcb5
From 3b5bee730237e861a51d04b766d894f4e2194aaa 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 6/6] 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>
---
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.rc1.175.g0b1dcb5
More information about the samba-technical
mailing list