From c45279a9ba62a9cc84ff0cb95182f3180cef5045 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 13 Jan 2014 14:12:18 -0800 Subject: [PATCH 1/5] s3: SMB2 sessionsetup - factor code out into a function remove_outstanding_session_refs(). We will be reusing this. Signed-off-by: Jeremy Allison --- 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.8.5.3 From 4490f74982422f2267c78eeac2bb8475741621bf Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 13 Jan 2014 14:31:31 -0800 Subject: [PATCH 2/5] 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. Signed-off-by: Jeremy Allison --- 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.8.5.3 From 8482510f04a788e5c4d0f936d908d5764fbd2d0a Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 27 Jan 2014 15:19:35 -0800 Subject: [PATCH 3/5] 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. Signed-off-by: Jeremy Allison --- source3/smbd/smb2_sesssetup.c | 170 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 150 insertions(+), 20 deletions(-) diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c index 7dbf603..44422c3 100644 --- a/source3/smbd/smb2_sesssetup.c +++ b/source3/smbd/smb2_sesssetup.c @@ -880,44 +880,174 @@ 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); + return smbd_smb2_request_pending_queue(req, subreq, 500); +} + +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) { + 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(&state->end_logout, &curr) >= 0) { + /* Ran out of time - terminate. */ + smbd_server_connection_terminate(smb2req->sconn, + nt_errstr(NT_STATUS_DRIVER_CANCEL_TIMEOUT)); + return 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.8.5.3 From f770381a1bce3cef17cb072da2137130dab0a7ee Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 28 Jan 2014 13:33:06 -0800 Subject: [PATCH 4/5] 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. Signed-off-by: Jeremy Allison --- source3/smbd/smb2_tcon.c | 229 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 217 insertions(+), 12 deletions(-) diff --git a/source3/smbd/smb2_tcon.c b/source3/smbd/smb2_tcon.c index ca67461..65e196f 100644 --- a/source3/smbd/smb2_tcon.c +++ b/source3/smbd/smb2_tcon.c @@ -411,20 +411,218 @@ 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); + return smbd_smb2_request_pending_queue(req, subreq, 500); +} + +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) { + 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(&state->end_logout, &curr) >= 0) { + /* Ran out of time - terminate. */ + smbd_server_connection_terminate(smb2req->sconn, + nt_errstr(NT_STATUS_DRIVER_CANCEL_TIMEOUT)); + return 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 +631,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.8.5.3 From 9cba082ae907ec0a801a4237cb297f9d975ee8c3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 28 Jan 2014 14:07:26 -0800 Subject: [PATCH 5/5] Update the torture_smb2_notify_ulogoff test to demonstrate the problem. Signed-off-by: Jeremy Allison --- source4/torture/smb2/notify.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/source4/torture/smb2/notify.c b/source4/torture/smb2/notify.c index e83b099..df42721 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" @@ -1295,6 +1302,8 @@ static bool torture_smb2_notify_ulogoff(struct torture_context *torture, req = smb2_notify_send(tree1, &(notify.smb2)); + WAIT_FOR_ASYNC_RESPONSE(req); + status = smb2_logoff(tree2->session); CHECK_STATUS(status, NT_STATUS_OK); -- 1.8.5.3