From 076412390210a9abbd08350a6458aaec375cefe5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Fri, 5 Aug 2011 20:34:43 +0200 Subject: [PATCH 1/3] s3:smb2_server: make sure we prefer responses over requests on the client socket metze Autobuild-User: Stefan Metzmacher Autobuild-Date: Fri Aug 12 16:46:43 CEST 2011 on sn-devel-104 (cherry picked from commit 42cde0480bd6a5e2dddaa66917e1fa71e6a4edcd) --- source3/smbd/smb2_server.c | 64 +++++++++++++++++++++++++++++++++++-------- 1 files changed, 52 insertions(+), 12 deletions(-) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 0cc80ed..b121a29 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -1695,6 +1695,8 @@ void smbd_smb2_request_dispatch_immediate(struct tevent_context *ctx, } } +static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *sconn); + static void smbd_smb2_request_writev_done(struct tevent_req *subreq) { struct smbd_smb2_request *req = tevent_req_callback_data(subreq, @@ -1702,17 +1704,24 @@ static void smbd_smb2_request_writev_done(struct tevent_req *subreq) struct smbd_server_connection *sconn = req->sconn; int ret; int sys_errno; + NTSTATUS status; ret = tstream_writev_queue_recv(subreq, &sys_errno); TALLOC_FREE(subreq); TALLOC_FREE(req); if (ret == -1) { - NTSTATUS status = map_nt_error_from_unix(sys_errno); + status = map_nt_error_from_unix(sys_errno); DEBUG(2,("smbd_smb2_request_writev_done: client write error %s\n", nt_errstr(status))); smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } + + status = smbd_smb2_request_next_incoming(sconn); + if (!NT_STATUS_IS_OK(status)) { + smbd_server_connection_terminate(sconn, nt_errstr(status)); + return; + } } NTSTATUS smbd_smb2_request_done_ex(struct smbd_smb2_request *req, @@ -2317,12 +2326,47 @@ static NTSTATUS smbd_smb2_request_read_recv(struct tevent_req *req, static void smbd_smb2_request_incoming(struct tevent_req *subreq); +static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *sconn) +{ + size_t max_send_queue_len; + size_t cur_send_queue_len; + struct tevent_req *subreq; + + if (tevent_queue_length(sconn->smb2.recv_queue) > 0) { + /* + * if there is already a smbd_smb2_request_read + * pending, we are done. + */ + return NT_STATUS_OK; + } + + max_send_queue_len = MAX(1, sconn->smb2.max_credits/16); + cur_send_queue_len = tevent_queue_length(sconn->smb2.send_queue); + + if (cur_send_queue_len > max_send_queue_len) { + /* + * if we have a lot of requests to send, + * we wait until they are on the wire until we + * ask for the next request. + */ + return NT_STATUS_OK; + } + + /* ask for the next request */ + subreq = smbd_smb2_request_read_send(sconn, sconn->smb2.event_ctx, sconn); + if (subreq == NULL) { + return NT_STATUS_NO_MEMORY; + } + tevent_req_set_callback(subreq, smbd_smb2_request_incoming, sconn); + + return NT_STATUS_OK; +} + void smbd_smb2_first_negprot(struct smbd_server_connection *sconn, const uint8_t *inbuf, size_t size) { NTSTATUS status; struct smbd_smb2_request *req = NULL; - struct tevent_req *subreq; DEBUG(10,("smbd_smb2_first_negprot: packet length %u\n", (unsigned int)size)); @@ -2351,13 +2395,11 @@ void smbd_smb2_first_negprot(struct smbd_server_connection *sconn, return; } - /* ask for the next request */ - subreq = smbd_smb2_request_read_send(sconn, sconn->smb2.event_ctx, sconn); - if (subreq == NULL) { - smbd_server_connection_terminate(sconn, "no memory for reading"); + status = smbd_smb2_request_next_incoming(sconn); + if (!NT_STATUS_IS_OK(status)) { + smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } - tevent_req_set_callback(subreq, smbd_smb2_request_incoming, sconn); sconn->num_requests++; } @@ -2409,13 +2451,11 @@ static void smbd_smb2_request_incoming(struct tevent_req *subreq) } next: - /* ask for the next request (this constructs the main loop) */ - subreq = smbd_smb2_request_read_send(sconn, sconn->smb2.event_ctx, sconn); - if (subreq == NULL) { - smbd_server_connection_terminate(sconn, "no memory for reading"); + status = smbd_smb2_request_next_incoming(sconn); + if (!NT_STATUS_IS_OK(status)) { + smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } - tevent_req_set_callback(subreq, smbd_smb2_request_incoming, sconn); sconn->num_requests++; -- 1.7.3.1 From 6cc7f4e3ec181cce1396485fc3ab5aa9ec202f18 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 25 Aug 2011 14:01:01 -0700 Subject: [PATCH 2/3] Fix bug 8407 - SMB2 server can return requests out-of-order when processing a compound request. (cherry picked from commit b19aca715a951cd64cdd9f1e4849174b5759860b) --- source3/smbd/globals.h | 5 +- source3/smbd/smb2_server.c | 113 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 58e03a5..6d208fd 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -299,9 +299,6 @@ NTSTATUS smbd_smb2_request_process_getinfo(struct smbd_smb2_request *req); NTSTATUS smbd_smb2_request_process_setinfo(struct smbd_smb2_request *req); NTSTATUS smbd_smb2_request_process_break(struct smbd_smb2_request *req); NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req); -void smbd_smb2_request_dispatch_immediate(struct tevent_context *ctx, - struct tevent_immediate *im, - void *private_data); /* SMB1 -> SMB2 glue. */ void send_break_message_smb2(files_struct *fsp, int level); @@ -358,6 +355,7 @@ struct smbd_smb2_request { bool do_signing; bool async; bool cancelled; + bool dispatched; /* fake smb1 request. */ struct smb_request *smb1req; @@ -601,6 +599,7 @@ struct smbd_server_connection { uint32_t credits_granted; uint32_t max_credits; struct bitmap *credits_bitmap; + bool compound_in_progress; } smb2; }; diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index b121a29..7b40b08 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -27,6 +27,10 @@ #include "../lib/util/tevent_ntstatus.h" #include "smbprofile.h" +static void smbd_smb2_request_dispatch_immediate(struct tevent_context *ctx, + struct tevent_immediate *im, + void *private_data); + #define OUTVEC_ALLOC_SIZE (SMB2_HDR_BODY + 9) static const char *smb2_names[] = { @@ -864,6 +868,10 @@ NTSTATUS smbd_smb2_request_pending_queue(struct smbd_smb2_request *req, * Cancel the outstanding request. */ tevent_req_cancel(req->subreq); + + /* The following call takes care + of the compound_in_progress flag + when it calls smbd_smb2_request_reply(). */ return smbd_smb2_request_error(req, NT_STATUS_INSUFFICIENT_RESOURCES); } @@ -884,6 +892,15 @@ NTSTATUS smbd_smb2_request_pending_queue(struct smbd_smb2_request *req, if (!NT_STATUS_IS_OK(status)) { return status; } + + /* We're splitting off the last SMB2 + request in a compound set, and the + smb2_send_async_interim_response() + call above just sent all the replies + for the previous SMB2 requests in + this compound set. So we're no longer + in the "compound_in_progress" state. */ + req->sconn->smb2.compound_in_progress = false; } /* Don't return an intermediate packet on a pipe read/write. */ @@ -1113,6 +1130,9 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) uint32_t allowed_flags; NTSTATUS return_value; + /* Ensure we know this is in flight. */ + req->dispatched = true; + inhdr = (const uint8_t *)req->in.vector[i].iov_base; /* TODO: verify more things */ @@ -1592,6 +1612,48 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) return return_value; } +static struct smbd_smb2_request *smbd_smb2_find_next_request_to_dispatch(struct smbd_server_connection *sconn) +{ + struct smbd_smb2_request *req; + + for (req = sconn->smb2.requests; req; req = req->next) { + /* Return the first undispatched entry on the queue. */ + if (!req->dispatched) { + return req; + } + } + return NULL; +} + +static void smbd_smb2_request_dispatch_from_queue(struct tevent_context *ctx, + struct tevent_immediate *im, + void *private_data) +{ + NTSTATUS status; + struct smbd_server_connection *sconn = talloc_get_type_abort(private_data, + struct smbd_server_connection); + struct smbd_smb2_request *req = smbd_smb2_find_next_request_to_dispatch(sconn); + + TALLOC_FREE(im); + + if (req == NULL) { + /* Someone else already dispatched - ignore. */ + return; + } + + if (DEBUGLEVEL >= 10) { + DEBUG(10,("smbd_smb2_request_dispatch_from_queue: idx[%d] of %d vectors\n", + req->current_idx, req->in.vector_count)); + print_req_vectors(req); + } + + status = smbd_smb2_request_dispatch(req); + if (!NT_STATUS_IS_OK(status)) { + smbd_server_connection_terminate(sconn, nt_errstr(status)); + } +} + + static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) { struct tevent_req *subreq; @@ -1614,6 +1676,7 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) if (!im) { return NT_STATUS_NO_MEMORY; } + req->sconn->smb2.compound_in_progress = true; tevent_schedule_immediate(im, req->sconn->smb2.event_ctx, smbd_smb2_request_dispatch_immediate, @@ -1621,6 +1684,10 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) return NT_STATUS_OK; } + /* We're sending out a full PDU reply - compound must + be finished. */ + req->sconn->smb2.compound_in_progress = false; + smb2_setup_nbt_length(req->out.vector, req->out.vector_count); /* Set credit for this operation (zero credits if this @@ -1668,10 +1735,32 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) */ DLIST_REMOVE(req->sconn->smb2.requests, req); + /* If there is an undispatched request on + the queue, schedule it to be dispatched. + We have to do it here in addition to + calling dispatch in smbd_smb2_request_incoming() + because not every incoming SMB2 PDU will + trigger a dispatch, we might have had to + wait for a compound request to finish. + We could just call smbd_smb2_request_dispatch() + but that would lead to a deep call stack. JRA. */ + + req = smbd_smb2_find_next_request_to_dispatch(req->sconn); + if (req) { + struct tevent_immediate *im = tevent_create_immediate(req->sconn); + if (!im) { + return NT_STATUS_NO_MEMORY; + } + tevent_schedule_immediate(im, + req->sconn->smb2.event_ctx, + smbd_smb2_request_dispatch_from_queue, + req->sconn); + } + return NT_STATUS_OK; } -void smbd_smb2_request_dispatch_immediate(struct tevent_context *ctx, +static void smbd_smb2_request_dispatch_immediate(struct tevent_context *ctx, struct tevent_immediate *im, void *private_data) { @@ -2444,13 +2533,23 @@ static void smbd_smb2_request_incoming(struct tevent_req *subreq) return; } - status = smbd_smb2_request_dispatch(req); - if (!NT_STATUS_IS_OK(status)) { - smbd_server_connection_terminate(sconn, nt_errstr(status)); - return; - } - next: + /* Only dispatch a new request if we don't have a compound + request in flight. */ + if (!sconn->smb2.compound_in_progress) { + req = smbd_smb2_find_next_request_to_dispatch(sconn); + if (req) { + status = smbd_smb2_request_dispatch(req); + if (!NT_STATUS_IS_OK(status)) { + smbd_server_connection_terminate(sconn, nt_errstr(status)); + return; + } + } + } else { + DEBUG(10,("smbd_smb2_request_incoming: compound_in_progress - " + "deferring dispatch\n")); + } + status = smbd_smb2_request_next_incoming(sconn); if (!NT_STATUS_IS_OK(status)) { smbd_server_connection_terminate(sconn, nt_errstr(status)); -- 1.7.3.1 From 866142d3db6ec7edaa268fc45a69f6a2679f43af Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 25 Aug 2011 15:04:36 -0700 Subject: [PATCH 3/3] Remove the "dispatched" bool from the request struct. Split out the queues into "pending_requests" and "requests". (cherry picked from commit dc11b912f13e862a957499cec7d2557f1a4d13b5) --- source3/smbd/globals.h | 2 +- source3/smbd/smb2_server.c | 34 ++++++++++++++++++---------------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 6d208fd..59e0be4 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -355,7 +355,6 @@ struct smbd_smb2_request { bool do_signing; bool async; bool cancelled; - bool dispatched; /* fake smb1 request. */ struct smb_request *smb1req; @@ -594,6 +593,7 @@ struct smbd_server_connection { struct timed_event *brl_timeout; bool blocking_lock_unlock_state; } locks; + struct smbd_smb2_request *pending_requests; struct smbd_smb2_request *requests; uint64_t seqnum_low; uint32_t credits_granted; diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 7b40b08..a25b7e8 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -603,7 +603,8 @@ static NTSTATUS smbd_smb2_request_setup_out(struct smbd_smb2_request *req) /* setup the length of the NBT packet */ smb2_setup_nbt_length(req->out.vector, req->out.vector_count); - DLIST_ADD_END(req->sconn->smb2.requests, req, struct smbd_smb2_request *); + /* Put it on the pending queue - not dispatched yet. */ + DLIST_ADD_END(req->sconn->smb2.pending_requests, req, struct smbd_smb2_request *); return NT_STATUS_OK; } @@ -1130,9 +1131,6 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) uint32_t allowed_flags; NTSTATUS return_value; - /* Ensure we know this is in flight. */ - req->dispatched = true; - inhdr = (const uint8_t *)req->in.vector[i].iov_base; /* TODO: verify more things */ @@ -1612,17 +1610,20 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) return return_value; } -static struct smbd_smb2_request *smbd_smb2_find_next_request_to_dispatch(struct smbd_server_connection *sconn) +static struct smbd_smb2_request *smbd_smb2_get_next_request_to_dispatch(struct smbd_server_connection *sconn) { - struct smbd_smb2_request *req; + struct smbd_smb2_request *req = sconn->smb2.pending_requests; - for (req = sconn->smb2.requests; req; req = req->next) { - /* Return the first undispatched entry on the queue. */ - if (!req->dispatched) { - return req; - } + if (req == NULL) { + return NULL; } - return NULL; + + /* Remove from the pending queue. */ + DLIST_REMOVE(req->sconn->smb2.pending_requests, req); + /* And add to the dispatched queue. */ + DLIST_ADD_END(req->sconn->smb2.requests, req, struct smbd_smb2_request *); + + return req; } static void smbd_smb2_request_dispatch_from_queue(struct tevent_context *ctx, @@ -1632,7 +1633,7 @@ static void smbd_smb2_request_dispatch_from_queue(struct tevent_context *ctx, NTSTATUS status; struct smbd_server_connection *sconn = talloc_get_type_abort(private_data, struct smbd_server_connection); - struct smbd_smb2_request *req = smbd_smb2_find_next_request_to_dispatch(sconn); + struct smbd_smb2_request *req = smbd_smb2_get_next_request_to_dispatch(sconn); TALLOC_FREE(im); @@ -1745,8 +1746,7 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) We could just call smbd_smb2_request_dispatch() but that would lead to a deep call stack. JRA. */ - req = smbd_smb2_find_next_request_to_dispatch(req->sconn); - if (req) { + if (req->sconn->smb2.pending_requests) { struct tevent_immediate *im = tevent_create_immediate(req->sconn); if (!im) { return NT_STATUS_NO_MEMORY; @@ -2478,6 +2478,8 @@ void smbd_smb2_first_negprot(struct smbd_server_connection *sconn, return; } + req = smbd_smb2_get_next_request_to_dispatch(sconn); + status = smbd_smb2_request_dispatch(req); if (!NT_STATUS_IS_OK(status)) { smbd_server_connection_terminate(sconn, nt_errstr(status)); @@ -2537,7 +2539,7 @@ next: /* Only dispatch a new request if we don't have a compound request in flight. */ if (!sconn->smb2.compound_in_progress) { - req = smbd_smb2_find_next_request_to_dispatch(sconn); + req = smbd_smb2_get_next_request_to_dispatch(sconn); if (req) { status = smbd_smb2_request_dispatch(req); if (!NT_STATUS_IS_OK(status)) { -- 1.7.3.1