The Samba-Bugzilla – Attachment 6809 Details for
Bug 8407
SMB2 server can return requests out-of-order when processing a compound request.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
git-am patch for master
look (text/plain), 11.62 KB, created by
Jeremy Allison
on 2011-08-25 22:32:09 UTC
(
hide
)
Description:
git-am patch for master
Filename:
MIME Type:
Creator:
Jeremy Allison
Created:
2011-08-25 22:32:09 UTC
Size:
11.62 KB
patch
obsolete
>From b19aca715a951cd64cdd9f1e4849174b5759860b Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 25 Aug 2011 14:01:01 -0700 >Subject: [PATCH 1/2] Fix bug 8407 - SMB2 server can return requests out-of-order when processing a compound request. > >--- > 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 b9bd212..2b150a1 100644 >--- a/source3/smbd/globals.h >+++ b/source3/smbd/globals.h >@@ -296,9 +296,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); >@@ -355,6 +352,7 @@ struct smbd_smb2_request { > bool do_signing; > bool async; > bool cancelled; >+ bool dispatched; > > /* fake smb1 request. */ > struct smb_request *smb1req; >@@ -605,6 +603,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 6fc4b5d..fe3f8b0 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -30,6 +30,10 @@ > #include "../librpc/gen_ndr/krb5pac.h" > #include "auth.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[] = { >@@ -867,6 +871,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); > } >@@ -887,6 +895,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. */ >@@ -1244,6 +1261,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 */ >@@ -1723,6 +1743,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; >@@ -1745,6 +1807,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, >@@ -1752,6 +1815,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 >@@ -1799,10 +1866,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) > { >@@ -2577,13 +2666,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 dc11b912f13e862a957499cec7d2557f1a4d13b5 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 25 Aug 2011 15:04:36 -0700 >Subject: [PATCH 2/2] Remove the "dispatched" bool from the request struct. Split out the > queues into "pending_requests" and "requests". > >--- > 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 2b150a1..7abbf67 100644 >--- a/source3/smbd/globals.h >+++ b/source3/smbd/globals.h >@@ -352,7 +352,6 @@ struct smbd_smb2_request { > bool do_signing; > bool async; > bool cancelled; >- bool dispatched; > > /* fake smb1 request. */ > struct smb_request *smb1req; >@@ -598,6 +597,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 fe3f8b0..58230b4 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -606,7 +606,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; > } >@@ -1261,9 +1262,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 */ >@@ -1743,17 +1741,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, >@@ -1763,7 +1764,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); > >@@ -1876,8 +1877,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; >@@ -2611,6 +2611,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)); >@@ -2670,7 +2672,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 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 8407
:
6807
|
6808
|
6809
|
6810
|
6811
|
6812
|
6814
|
6815
|
6855