From 4c71647de98e632140a8780d1aee03bf5482c0ae 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.4.1 From 18e9f6bce0aa754e73c74b4abdbb8ea9a01a14b9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 26 Aug 2011 14:23:26 -0700 Subject: [PATCH 2/3] Based on metze's fix for Bug 8407 - SMB2 server can return requests out-of-order when processing a compound request. (cherry picked from commit 726b4685aa25b0b3b4470bfec5d514fb2db7a95e) --- source3/smbd/globals.h | 1 + source3/smbd/smb2_server.c | 40 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 58e03a5..e39ea3e 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -601,6 +601,7 @@ struct smbd_server_connection { uint32_t credits_granted; uint32_t max_credits; struct bitmap *credits_bitmap; + bool compound_related_in_progress; } smb2; }; diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index b121a29..5661c47 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -884,6 +884,20 @@ 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_related_in_progress" + * state, and this is no longer a compound + * request. + */ + req->compound_related = false; + req->sconn->smb2.compound_related_in_progress = false; } /* Don't return an intermediate packet on a pipe read/write. */ @@ -1175,6 +1189,10 @@ NTSTATUS smbd_smb2_request_dispatch(struct smbd_smb2_request *req) req->compat_chain_fsp = NULL; } + if (req->compound_related) { + req->sconn->smb2.compound_related_in_progress = true; + } + switch (opcode) { case SMB2_OP_NEGPROT: /* This call needs to be run as root */ @@ -1621,6 +1639,10 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) return NT_STATUS_OK; } + if (req->compound_related) { + req->sconn->smb2.compound_related_in_progress = false; + } + smb2_setup_nbt_length(req->out.vector, req->out.vector_count); /* Set credit for this operation (zero credits if this @@ -1671,6 +1693,8 @@ static NTSTATUS smbd_smb2_request_reply(struct smbd_smb2_request *req) return NT_STATUS_OK; } +static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *sconn); + void smbd_smb2_request_dispatch_immediate(struct tevent_context *ctx, struct tevent_immediate *im, void *private_data) @@ -1693,9 +1717,13 @@ void smbd_smb2_request_dispatch_immediate(struct tevent_context *ctx, smbd_server_connection_terminate(sconn, nt_errstr(status)); return; } -} -static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *sconn); + status = smbd_smb2_request_next_incoming(sconn); + if (!NT_STATUS_IS_OK(status)) { + smbd_server_connection_terminate(sconn, nt_errstr(status)); + return; + } +} static void smbd_smb2_request_writev_done(struct tevent_req *subreq) { @@ -2332,6 +2360,14 @@ static NTSTATUS smbd_smb2_request_next_incoming(struct smbd_server_connection *s size_t cur_send_queue_len; struct tevent_req *subreq; + if (sconn->smb2.compound_related_in_progress) { + /* + * Can't read another until the related + * compound is done. + */ + return NT_STATUS_OK; + } + if (tevent_queue_length(sconn->smb2.recv_queue) > 0) { /* * if there is already a smbd_smb2_request_read -- 1.7.4.1 From 48902d31d60335fefe7ca35b349a61a744f1791b Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 25 Aug 2011 23:33:41 +0200 Subject: [PATCH 3/3] s3:smb2_server: keep compound_related on struct smbd_smb2_request metze (cherry picked from commit 0d450d166bab952daf37d922e5c2e5cac16f1cc3) --- source3/smbd/globals.h | 1 + source3/smbd/smb2_server.c | 7 +++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index e39ea3e..abeaed4 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -358,6 +358,7 @@ struct smbd_smb2_request { bool do_signing; bool async; bool cancelled; + bool compound_related; /* fake smb1 request. */ struct smb_request *smb1req; diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 5661c47..d7a40ed 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -357,7 +357,6 @@ static NTSTATUS smbd_smb2_request_validate(struct smbd_smb2_request *req) { int count; int idx; - bool compound_related = false; count = req->in.vector_count; @@ -405,7 +404,7 @@ static NTSTATUS smbd_smb2_request_validate(struct smbd_smb2_request *req) * compounded requests */ if (flags & SMB2_HDR_FLAG_CHAINED) { - compound_related = true; + req->compound_related = true; } } else if (idx > 4) { #if 0 @@ -418,13 +417,13 @@ static NTSTATUS smbd_smb2_request_validate(struct smbd_smb2_request *req) * all other requests should match the 2nd one */ if (flags & SMB2_HDR_FLAG_CHAINED) { - if (!compound_related) { + if (!req->compound_related) { req->next_status = NT_STATUS_INVALID_PARAMETER; return NT_STATUS_OK; } } else { - if (compound_related) { + if (req->compound_related) { req->next_status = NT_STATUS_INVALID_PARAMETER; return NT_STATUS_OK; -- 1.7.4.1