The Samba-Bugzilla – Attachment 7716 Details for
Bug 9057
SMB2 credit handling code has bugs.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Patches for v3-6-test
v3-6-test-fix-credits-01.patches.txt (text/plain), 28.61 KB, created by
Stefan Metzmacher
on 2012-07-23 12:45:52 UTC
(
hide
)
Description:
Patches for v3-6-test
Filename:
MIME Type:
Creator:
Stefan Metzmacher
Created:
2012-07-23 12:45:52 UTC
Size:
28.61 KB
patch
obsolete
>From 9e5c82177910b8ef9ff6a7754b83545b703d4d0a Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 26 Jun 2012 14:23:12 +0200 >Subject: [PATCH 01/10] s3:smb2_server: start the connection with one credit granted to the client > >metze >(cherry picked from commit 0b8eac9b79197c4659a5738f1b9399b3c88f2f8d) >--- > source3/smbd/smb2_server.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index 8a5d81f..017fe7e 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -114,7 +114,7 @@ static NTSTATUS smbd_initialize_smb2(struct smbd_server_connection *sconn) > sconn->smb2.sessions.limit = 0x0000FFFE; > sconn->smb2.sessions.list = NULL; > sconn->smb2.seqnum_low = 0; >- sconn->smb2.credits_granted = 0; >+ sconn->smb2.credits_granted = 1; > sconn->smb2.max_credits = lp_smb2_max_credits(); > sconn->smb2.credits_bitmap = bitmap_talloc(sconn, > DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR*sconn->smb2.max_credits); >-- >1.7.4.1 > > >From 7fce1254ae10db07b82a1f07a5e2167b2ce091f8 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 25 Jun 2012 23:14:24 +0200 >Subject: [PATCH 02/10] s3:smb2_server: call smbd_smb2_request_validate() also in smbd_smb2_first_negprot() > >We need to consume message_id 0, for SMB1 negprot starts. > >metze >(cherry picked from commit 925994e42eba5b72ce605b68e8980adc1b5ecd83) >--- > source3/smbd/smb2_server.c | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index 017fe7e..1461d70 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -2522,6 +2522,12 @@ void smbd_smb2_first_negprot(struct smbd_server_connection *sconn, > return; > } > >+ status = smbd_smb2_request_validate(req); >+ if (!NT_STATUS_IS_OK(status)) { >+ smbd_server_connection_terminate(sconn, nt_errstr(status)); >+ return; >+ } >+ > status = smbd_smb2_request_setup_out(req); > if (!NT_STATUS_IS_OK(status)) { > smbd_server_connection_terminate(sconn, nt_errstr(status)); >-- >1.7.4.1 > > >From 864fd8447fcb16ee469a4c77262d06aaae9c4e9f Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 26 Jun 2012 09:11:23 +0200 >Subject: [PATCH 03/10] s3:smb2_server: remove unused and confusing DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR > >metze >(similar to commit d1ee774ed0b4b3882b4b85da16d9bb9c082a0c49) >--- > source3/include/local.h | 1 - > source3/smbd/smb2_server.c | 9 ++++----- > 2 files changed, 4 insertions(+), 6 deletions(-) > >diff --git a/source3/include/local.h b/source3/include/local.h >index d659522..499f468 100644 >--- a/source3/include/local.h >+++ b/source3/include/local.h >@@ -265,6 +265,5 @@ > #define DEFAULT_SMB2_MAX_WRITE (64*1024) > #define DEFAULT_SMB2_MAX_TRANSACT (64*1024) > #define DEFAULT_SMB2_MAX_CREDITS 8192 >-#define DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR 2 > > #endif >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index 1461d70..f4f9be8 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -117,7 +117,7 @@ static NTSTATUS smbd_initialize_smb2(struct smbd_server_connection *sconn) > sconn->smb2.credits_granted = 1; > sconn->smb2.max_credits = lp_smb2_max_credits(); > sconn->smb2.credits_bitmap = bitmap_talloc(sconn, >- DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR*sconn->smb2.max_credits); >+ sconn->smb2.max_credits); > if (sconn->smb2.credits_bitmap == NULL) { > return NT_STATUS_NO_MEMORY; > } >@@ -313,7 +313,7 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > > if (message_id < sconn->smb2.seqnum_low || > message_id > (sconn->smb2.seqnum_low + >- (sconn->smb2.max_credits * DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR))) { >+ (sconn->smb2.max_credits))) { > DEBUG(0,("smb2_validate_message_id: bad message_id " > "%llu (low = %llu, max = %lu)\n", > (unsigned long long)message_id, >@@ -333,8 +333,7 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > sconn->smb2.credits_granted -= 1; > > /* Mark the message_id as seen in the bitmap. */ >- bitmap_offset = (unsigned int)(message_id % >- (uint64_t)(sconn->smb2.max_credits * DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR)); >+ bitmap_offset = message_id % sconn->smb2.max_credits; > if (bitmap_query(credits_bm, bitmap_offset)) { > DEBUG(0,("smb2_validate_message_id: duplicate message_id " > "%llu (bm offset %u)\n", >@@ -355,7 +354,7 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > bitmap_clear(credits_bm, bitmap_offset); > sconn->smb2.seqnum_low += 1; > bitmap_offset = (bitmap_offset + 1) % >- (sconn->smb2.max_credits * DEFAULT_SMB2_MAX_CREDIT_BITMAP_FACTOR); >+ sconn->smb2.max_credits; > } > } > >-- >1.7.4.1 > > >From c1b0656971a80154c707a210cc27a60358f7e443 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 26 Jun 2012 09:12:44 +0200 >Subject: [PATCH 04/10] s3:smb2_server: fix calculation of the next bitmap_offset > >metze >(similar to commit bd6d415cae550e97e04830eecefa2881b497de89) >--- > source3/smbd/smb2_server.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index f4f9be8..5cb31b3 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -353,7 +353,7 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > bitmap_offset )); > bitmap_clear(credits_bm, bitmap_offset); > sconn->smb2.seqnum_low += 1; >- bitmap_offset = (bitmap_offset + 1) % >+ bitmap_offset = sconn->smb2.seqnum_low % > sconn->smb2.max_credits; > } > } >-- >1.7.4.1 > > >From ff75c321cc112e87243646e58fbf64e2b1b64c75 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 26 Jun 2012 08:08:37 +0200 >Subject: [PATCH 05/10] s3:smb2_server: clear sequence window if we got the lowest sequence id > >Otherwise we'll never consume sequence id '0'. > >metze >(similar to commit d6e7a76461ad7582efa510676aa2bea230ea9f02) >--- > source3/smbd/smb2_server.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index 5cb31b3..de4b93a 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -343,13 +343,13 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > } > bitmap_set(credits_bm, bitmap_offset); > >- if (message_id == sconn->smb2.seqnum_low + 1) { >+ if (message_id == sconn->smb2.seqnum_low) { > /* Move the window forward by all the message_id's > already seen. */ > while (bitmap_query(credits_bm, bitmap_offset)) { > DEBUG(10,("smb2_validate_message_id: clearing " > "id %llu (position %u) from bitmap\n", >- (unsigned long long)(sconn->smb2.seqnum_low + 1), >+ (unsigned long long)(sconn->smb2.seqnum_low), > bitmap_offset )); > bitmap_clear(credits_bm, bitmap_offset); > sconn->smb2.seqnum_low += 1; >-- >1.7.4.1 > > >From a204d0b303f3fa54b7c2eeb00eb088167a8afa90 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Mon, 25 Jun 2012 23:17:55 +0200 >Subject: [PATCH 06/10] s3:smb2_server: split out a smb2_validate_sequence_number() function > >metze >(similar to commit 984fdaf9149d96d0d28600443981d87d13eb355c) >--- > source3/smbd/smb2_server.c | 115 +++++++++++++++++++++++++++++-------------- > 1 files changed, 77 insertions(+), 38 deletions(-) > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index de4b93a..bbb342b 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -298,30 +298,83 @@ static NTSTATUS smbd_smb2_request_create(struct smbd_server_connection *sconn, > return NT_STATUS_OK; > } > >+static bool smb2_validate_sequence_number(struct smbd_server_connection *sconn, >+ uint64_t message_id, uint64_t seq_id) >+{ >+ struct bitmap *credits_bm = sconn->smb2.credits_bitmap; >+ unsigned int offset; >+ >+ if (seq_id < sconn->smb2.seqnum_low) { >+ DEBUG(0,("smb2_validate_sequence_number: bad message_id " >+ "%llu (sequence id %llu) (low = %llu, max = %lu)\n", >+ (unsigned long long)message_id, >+ (unsigned long long)seq_id, >+ (unsigned long long)sconn->smb2.seqnum_low, >+ (unsigned long)sconn->smb2.max_credits)); >+ return false; >+ } >+ >+ if (seq_id > (sconn->smb2.seqnum_low + sconn->smb2.max_credits)) { >+ DEBUG(0,("smb2_validate_sequence_number: bad message_id " >+ "%llu (sequence id %llu) (low = %llu, max = %lu)\n", >+ (unsigned long long)message_id, >+ (unsigned long long)seq_id, >+ (unsigned long long)sconn->smb2.seqnum_low, >+ (unsigned long)sconn->smb2.max_credits)); >+ return false; >+ } >+ >+ offset = seq_id % sconn->smb2.max_credits; >+ >+ if (bitmap_query(credits_bm, offset)) { >+ DEBUG(0,("smb2_validate_sequence_number: duplicate message_id " >+ "%llu (sequence id %llu) (low = %llu, max = %lu) " >+ "(bm offset %u)\n", >+ (unsigned long long)message_id, >+ (unsigned long long)seq_id, >+ (unsigned long long)sconn->smb2.seqnum_low, >+ (unsigned long)sconn->smb2.max_credits, >+ offset)); >+ return false; >+ } >+ >+ /* Mark the message_ids as seen in the bitmap. */ >+ bitmap_set(credits_bm, offset); >+ >+ if (seq_id != sconn->smb2.seqnum_low) { >+ return true; >+ } >+ >+ /* >+ * Move the window forward by all the message_id's >+ * already seen. >+ */ >+ while (bitmap_query(credits_bm, offset)) { >+ DEBUG(10,("smb2_validate_sequence_number: clearing " >+ "id %llu (position %u) from bitmap\n", >+ (unsigned long long)(sconn->smb2.seqnum_low), >+ offset)); >+ bitmap_clear(credits_bm, offset); >+ >+ sconn->smb2.seqnum_low += 1; >+ offset = sconn->smb2.seqnum_low % sconn->smb2.max_credits; >+ } >+ >+ return true; >+} >+ > static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > const uint8_t *inhdr) > { > uint64_t message_id = BVAL(inhdr, SMB2_HDR_MESSAGE_ID); >- struct bitmap *credits_bm = sconn->smb2.credits_bitmap; > uint16_t opcode = IVAL(inhdr, SMB2_HDR_OPCODE); >- unsigned int bitmap_offset; >+ bool ok; > > if (opcode == SMB2_OP_CANCEL) { > /* SMB2_CANCEL requests by definition resend messageids. */ > return true; > } > >- if (message_id < sconn->smb2.seqnum_low || >- message_id > (sconn->smb2.seqnum_low + >- (sconn->smb2.max_credits))) { >- DEBUG(0,("smb2_validate_message_id: bad message_id " >- "%llu (low = %llu, max = %lu)\n", >- (unsigned long long)message_id, >- (unsigned long long)sconn->smb2.seqnum_low, >- (unsigned long)sconn->smb2.max_credits )); >- return false; >- } >- > if (sconn->smb2.credits_granted == 0) { > DEBUG(0,("smb2_validate_message_id: client used more " > "credits than granted message_id (%llu)\n", >@@ -329,34 +382,20 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > return false; > } > >- /* client just used a credit. */ >- sconn->smb2.credits_granted -= 1; >+ DEBUG(11, ("smb2_validate_message_id: mid %llu, credits_granted %llu, " >+ "max_credits %llu, seqnum_low: %llu\n", >+ (unsigned long long) message_id, >+ (unsigned long long) sconn->smb2.credits_granted, >+ (unsigned long long) sconn->smb2.max_credits, >+ (unsigned long long) sconn->smb2.seqnum_low)); > >- /* Mark the message_id as seen in the bitmap. */ >- bitmap_offset = message_id % sconn->smb2.max_credits; >- if (bitmap_query(credits_bm, bitmap_offset)) { >- DEBUG(0,("smb2_validate_message_id: duplicate message_id " >- "%llu (bm offset %u)\n", >- (unsigned long long)message_id, >- bitmap_offset)); >+ ok = smb2_validate_sequence_number(sconn, message_id, message_id); >+ if (!ok) { > return false; > } >- bitmap_set(credits_bm, bitmap_offset); > >- if (message_id == sconn->smb2.seqnum_low) { >- /* Move the window forward by all the message_id's >- already seen. */ >- while (bitmap_query(credits_bm, bitmap_offset)) { >- DEBUG(10,("smb2_validate_message_id: clearing " >- "id %llu (position %u) from bitmap\n", >- (unsigned long long)(sconn->smb2.seqnum_low), >- bitmap_offset )); >- bitmap_clear(credits_bm, bitmap_offset); >- sconn->smb2.seqnum_low += 1; >- bitmap_offset = sconn->smb2.seqnum_low % >- sconn->smb2.max_credits; >- } >- } >+ /* substract used credits */ >+ sconn->smb2.credits_granted -= 1; > > return true; > } >-- >1.7.4.1 > > >From f3c22021c83d341c4c3b0e3a0ecc5909b5ec31d2 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Tue, 26 Jun 2012 14:28:07 +0200 >Subject: [PATCH 07/10] s3:smb2_server: check the already granted credits like in the master branch > >metze > >Backport of: s3:smb2_server: check the credit_charge against the already granted credits >(similar to commit 4fe41c0bb14f6ae7e52aa7f180e66c7695eb6fa0) >--- > source3/smbd/smb2_server.c | 18 +++++++++++------- > 1 files changed, 11 insertions(+), 7 deletions(-) > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index bbb342b..a73c2cc 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -375,13 +375,6 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > return true; > } > >- if (sconn->smb2.credits_granted == 0) { >- DEBUG(0,("smb2_validate_message_id: client used more " >- "credits than granted message_id (%llu)\n", >- (unsigned long long)message_id)); >- return false; >- } >- > DEBUG(11, ("smb2_validate_message_id: mid %llu, credits_granted %llu, " > "max_credits %llu, seqnum_low: %llu\n", > (unsigned long long) message_id, >@@ -389,6 +382,17 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > (unsigned long long) sconn->smb2.max_credits, > (unsigned long long) sconn->smb2.seqnum_low)); > >+ if (sconn->smb2.credits_granted < 1) { >+ DEBUG(0, ("smb2_validate_message_id: client used more " >+ "credits than granted, mid %llu, credits_granted %llu, " >+ "max_credits %llu, seqnum_low: %llu\n", >+ (unsigned long long) message_id, >+ (unsigned long long) sconn->smb2.credits_granted, >+ (unsigned long long) sconn->smb2.max_credits, >+ (unsigned long long) sconn->smb2.seqnum_low)); >+ return false; >+ } >+ > ok = smb2_validate_sequence_number(sconn, message_id, message_id); > if (!ok) { > return false; >-- >1.7.4.1 > > >From e3baeb0c61b2e10b198322dd1b426fc6af1577f9 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 27 Jun 2012 15:33:43 +0200 >Subject: [PATCH 08/10] s3:smb2_server: make sure we don't grant more credits than we allow > >If the client hasn't consumed the lowest seqnum, but the distance >between lowest and highest seqnum has reached max credits. > >In that case we should stop granting credits. > >metze >(similar to commit ee8ae459aea6879377b5510851a6dc673cf72aad) >--- > source3/smbd/globals.h | 41 ++++++++++++++++++++++-- > source3/smbd/smb2_server.c | 74 ++++++++++++++++++++++++++++---------------- > 2 files changed, 85 insertions(+), 30 deletions(-) > >diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h >index 663daa4..664370b 100644 >--- a/source3/smbd/globals.h >+++ b/source3/smbd/globals.h >@@ -603,13 +603,48 @@ struct smbd_server_connection { > bool blocking_lock_unlock_state; > } locks; > struct smbd_smb2_request *requests; >+ /* >+ * seqnum_low is the lowest sequence number >+ * we will accept. >+ */ > uint64_t seqnum_low; >- uint32_t credits_granted; >- uint32_t max_credits; >+ /* >+ * seqnum_range is the range of credits we have >+ * granted from the sequence windows starting >+ * at seqnum_low. >+ * >+ * This gets incremented when new credits are >+ * granted and gets decremented when the >+ * lowest sequence number is consumed >+ * (when seqnum_low gets incremented). >+ */ >+ uint16_t seqnum_range; >+ /* >+ * credits_grantedThe number of credits we have currently granted >+ * to the client. >+ * >+ * This gets incremented when new credits are >+ * granted and gets decremented when any credit >+ * is comsumed. >+ * >+ * Note: the decrementing is different compared >+ * to seqnum_range. >+ */ >+ uint16_t credits_granted; >+ /* >+ * The maximum number of credits we will ever >+ * grant to the client. >+ * >+ * This is the "server max credits" parameter. >+ */ >+ uint16_t max_credits; >+ /* >+ * a bitmap of size max_credits >+ */ >+ struct bitmap *credits_bitmap; > uint32_t max_trans; > uint32_t max_read; > uint32_t max_write; >- 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 a73c2cc..8f7a9a4 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -114,6 +114,7 @@ static NTSTATUS smbd_initialize_smb2(struct smbd_server_connection *sconn) > sconn->smb2.sessions.limit = 0x0000FFFE; > sconn->smb2.sessions.list = NULL; > sconn->smb2.seqnum_low = 0; >+ sconn->smb2.seqnum_range = 1; > sconn->smb2.credits_granted = 1; > sconn->smb2.max_credits = lp_smb2_max_credits(); > sconn->smb2.credits_bitmap = bitmap_talloc(sconn, >@@ -306,21 +307,25 @@ static bool smb2_validate_sequence_number(struct smbd_server_connection *sconn, > > if (seq_id < sconn->smb2.seqnum_low) { > DEBUG(0,("smb2_validate_sequence_number: bad message_id " >- "%llu (sequence id %llu) (low = %llu, max = %lu)\n", >+ "%llu (sequence id %llu) " >+ "(granted = %u, low = %llu, range = %u)\n", > (unsigned long long)message_id, > (unsigned long long)seq_id, >+ (unsigned int)sconn->smb2.credits_granted, > (unsigned long long)sconn->smb2.seqnum_low, >- (unsigned long)sconn->smb2.max_credits)); >+ (unsigned int)sconn->smb2.seqnum_range)); > return false; > } > >- if (seq_id > (sconn->smb2.seqnum_low + sconn->smb2.max_credits)) { >+ if (seq_id >= sconn->smb2.seqnum_low + sconn->smb2.seqnum_range) { > DEBUG(0,("smb2_validate_sequence_number: bad message_id " >- "%llu (sequence id %llu) (low = %llu, max = %lu)\n", >+ "%llu (sequence id %llu) " >+ "(granted = %u, low = %llu, range = %u)\n", > (unsigned long long)message_id, > (unsigned long long)seq_id, >+ (unsigned int)sconn->smb2.credits_granted, > (unsigned long long)sconn->smb2.seqnum_low, >- (unsigned long)sconn->smb2.max_credits)); >+ (unsigned int)sconn->smb2.seqnum_range)); > return false; > } > >@@ -328,12 +333,14 @@ static bool smb2_validate_sequence_number(struct smbd_server_connection *sconn, > > if (bitmap_query(credits_bm, offset)) { > DEBUG(0,("smb2_validate_sequence_number: duplicate message_id " >- "%llu (sequence id %llu) (low = %llu, max = %lu) " >+ "%llu (sequence id %llu) " >+ "(granted = %u, low = %llu, range = %u) " > "(bm offset %u)\n", > (unsigned long long)message_id, > (unsigned long long)seq_id, >+ (unsigned int)sconn->smb2.credits_granted, > (unsigned long long)sconn->smb2.seqnum_low, >- (unsigned long)sconn->smb2.max_credits, >+ (unsigned int)sconn->smb2.seqnum_range, > offset)); > return false; > } >@@ -357,6 +364,7 @@ static bool smb2_validate_sequence_number(struct smbd_server_connection *sconn, > bitmap_clear(credits_bm, offset); > > sconn->smb2.seqnum_low += 1; >+ sconn->smb2.seqnum_range -= 1; > offset = sconn->smb2.seqnum_low % sconn->smb2.max_credits; > } > >@@ -376,20 +384,20 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn, > } > > DEBUG(11, ("smb2_validate_message_id: mid %llu, credits_granted %llu, " >- "max_credits %llu, seqnum_low: %llu\n", >+ "seqnum low/range: %llu/%llu\n", > (unsigned long long) message_id, > (unsigned long long) sconn->smb2.credits_granted, >- (unsigned long long) sconn->smb2.max_credits, >- (unsigned long long) sconn->smb2.seqnum_low)); >+ (unsigned long long) sconn->smb2.seqnum_low, >+ (unsigned long long) sconn->smb2.seqnum_range)); > > if (sconn->smb2.credits_granted < 1) { > DEBUG(0, ("smb2_validate_message_id: client used more " > "credits than granted, mid %llu, credits_granted %llu, " >- "max_credits %llu, seqnum_low: %llu\n", >+ "seqnum low/range: %llu/%llu\n", > (unsigned long long) message_id, > (unsigned long long) sconn->smb2.credits_granted, >- (unsigned long long) sconn->smb2.max_credits, >- (unsigned long long) sconn->smb2.seqnum_low)); >+ (unsigned long long) sconn->smb2.seqnum_low, >+ (unsigned long long) sconn->smb2.seqnum_range)); > return false; > } > >@@ -496,6 +504,7 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, > uint16_t credits_requested; > uint32_t out_flags; > uint16_t credits_granted = 0; >+ uint64_t credits_possible; > > credits_requested = SVAL(inhdr, SMB2_HDR_CREDIT); > out_flags = IVAL(outhdr, SMB2_HDR_FLAGS); >@@ -508,10 +517,8 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, > * response, we should not grant > * credits on the final response. > */ >- credits_requested = 0; >- } >- >- if (credits_requested) { >+ credits_granted = 0; >+ } else if (credits_requested > 0) { > uint16_t modified_credits_requested; > uint32_t multiplier; > >@@ -531,25 +538,38 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, > modified_credits_requested = 1; > } > >- /* Remember what we gave out. */ >- credits_granted = MIN(modified_credits_requested, >- (sconn->smb2.max_credits - sconn->smb2.credits_granted)); >- } >- >- if (credits_granted == 0 && sconn->smb2.credits_granted == 0) { >- /* First negprot packet, or ensure the client credits can >- never drop to zero. */ >+ credits_granted = modified_credits_requested; >+ } else if (sconn->smb2.credits_granted == 0) { >+ /* >+ * Make sure the client has always at least one credit >+ */ > credits_granted = 1; > } > >+ /* >+ * remove the range we'll already granted to the client >+ * this makes sure the client consumes the lowest sequence >+ * number, before we can grant additional credits. >+ */ >+ credits_possible = sconn->smb2.max_credits; >+ credits_possible -= sconn->smb2.seqnum_range; >+ >+ credits_granted = MIN(credits_granted, credits_possible); >+ > SSVAL(outhdr, SMB2_HDR_CREDIT, credits_granted); > sconn->smb2.credits_granted += credits_granted; >+ sconn->smb2.seqnum_range += credits_granted; > > DEBUG(10,("smb2_set_operation_credit: requested %u, " >- "granted %u, total granted %u\n", >+ "granted %u, current possible %u, " >+ "total granted/max/low/range %u/%u/%llu/%u\n", > (unsigned int)credits_requested, > (unsigned int)credits_granted, >- (unsigned int)sconn->smb2.credits_granted )); >+ (unsigned int)credits_possible, >+ (unsigned int)sconn->smb2.credits_granted, >+ (unsigned int)sconn->smb2.max_credits, >+ (unsigned long long)sconn->smb2.seqnum_low, >+ (unsigned int)sconn->smb2.seqnum_range)); > } > > static void smb2_calculate_credits(const struct smbd_smb2_request *inreq, >-- >1.7.4.1 > > >From 558432fd39c011169c6c7070d00409bd20dad7fe Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 27 Jun 2012 15:33:43 +0200 >Subject: [PATCH 09/10] s3:smb2_server: make sure sequence numbers don't wrap at UINT64_MAX > >metze >(cherry picked from commit 82dc0b33b9af5094d78f3ecd855900e49c580343) >--- > source3/smbd/smb2_server.c | 19 +++++++++++++++++-- > 1 files changed, 17 insertions(+), 2 deletions(-) > >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index 8f7a9a4..d983527 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -547,11 +547,26 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, > } > > /* >- * remove the range we'll already granted to the client >+ * sequence numbers should not wrap >+ * >+ * 1. calculate the possible credits until >+ * the sequence numbers start to wrap on 64-bit. >+ * >+ * 2. UINT64_MAX is used for Break Notifications. >+ * >+ * 2. truncate the possible credits to the maximum >+ * credits we want to grant to the client in total. >+ * >+ * 3. remove the range we'll already granted to the client > * this makes sure the client consumes the lowest sequence > * number, before we can grant additional credits. > */ >- credits_possible = sconn->smb2.max_credits; >+ credits_possible = UINT64_MAX - sconn->smb2.seqnum_low; >+ if (credits_possible > 0) { >+ /* remove UINT64_MAX */ >+ credits_possible -= 1; >+ } >+ credits_possible = MIN(credits_possible, sconn->smb2.max_credits); > credits_possible -= sconn->smb2.seqnum_range; > > credits_granted = MIN(credits_granted, credits_possible); >-- >1.7.4.1 > > >From 274b2679b24b8399572b5aeb2264dbed73b7ba35 Mon Sep 17 00:00:00 2001 >From: Stefan Metzmacher <metze@samba.org> >Date: Wed, 27 Jun 2012 15:33:43 +0200 >Subject: [PATCH 10/10] s3:smb2_server: implement credit granting similar to windows > >This makes it much easier to compare traces. > >metze >(cherry picked from commit 648b959b13224105addaae483823bc422ed1cc21) >--- > source3/smbd/globals.h | 3 ++ > source3/smbd/smb2_server.c | 69 +++++++++++++++++++++++++++++++------------ > 2 files changed, 53 insertions(+), 19 deletions(-) > >diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h >index 664370b..eefc2c6 100644 >--- a/source3/smbd/globals.h >+++ b/source3/smbd/globals.h >@@ -635,6 +635,9 @@ struct smbd_server_connection { > * The maximum number of credits we will ever > * grant to the client. > * >+ * Typically we will only grant 1/16th of >+ * max_credits. >+ * > * This is the "server max credits" parameter. > */ > uint16_t max_credits; >diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c >index d983527..cef0677 100644 >--- a/source3/smbd/smb2_server.c >+++ b/source3/smbd/smb2_server.c >@@ -503,11 +503,33 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, > uint8_t *outhdr = (uint8_t *)out_vector->iov_base; > uint16_t credits_requested; > uint32_t out_flags; >+ uint16_t cmd; >+ NTSTATUS out_status; > uint16_t credits_granted = 0; > uint64_t credits_possible; >+ uint16_t current_max_credits; >+ >+ /* >+ * first we grant only 1/16th of the max range. >+ * >+ * Windows also starts with the 1/16th and then grants >+ * more later. I was only able to trigger higher >+ * values, when using a verify high credit charge. >+ * >+ * TODO: scale up depending one load, free memory >+ * or other stuff. >+ * Maybe also on the relationship between number >+ * of requests and the used sequence number. >+ * Which means we would grant more credits >+ * for client which use multi credit requests. >+ */ >+ current_max_credits = sconn->smb2.max_credits / 16; >+ current_max_credits = MAX(current_max_credits, 1); > >+ cmd = SVAL(inhdr, SMB2_HDR_OPCODE); > credits_requested = SVAL(inhdr, SMB2_HDR_CREDIT); > out_flags = IVAL(outhdr, SMB2_HDR_FLAGS); >+ out_status = NT_STATUS(IVAL(outhdr, SMB2_HDR_STATUS)); > > SMB_ASSERT(sconn->smb2.max_credits >= sconn->smb2.credits_granted); > >@@ -519,26 +541,34 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, > */ > credits_granted = 0; > } else if (credits_requested > 0) { >- uint16_t modified_credits_requested; >- uint32_t multiplier; >- >- /* >- * Split up max_credits into 1/16ths, and then scale >- * the requested credits by how many 16ths have been >- * currently granted. Less than 1/16th == grant all >- * requested (100%), scale down as more have been >- * granted. Never ask for less than 1 as the client >- * asked for at least 1. JRA. >- */ >+ uint16_t additional_max = 0; >+ uint16_t additional_credits = credits_requested - 1; > >- multiplier = 16 - (((sconn->smb2.credits_granted * 16) / sconn->smb2.max_credits) % 16); >- >- modified_credits_requested = (multiplier * credits_requested) / 16; >- if (modified_credits_requested == 0) { >- modified_credits_requested = 1; >+ switch (cmd) { >+ case SMB2_OP_NEGPROT: >+ break; >+ case SMB2_OP_SESSSETUP: >+ /* >+ * Windows 2012 RC1 starts to grant >+ * additional credits >+ * with a successful session setup >+ */ >+ if (NT_STATUS_IS_OK(out_status)) { >+ additional_max = 32; >+ } >+ break; >+ default: >+ /* >+ * We match windows and only grant additional credits >+ * in chunks of 32. >+ */ >+ additional_max = 32; >+ break; > } > >- credits_granted = modified_credits_requested; >+ additional_credits = MIN(additional_credits, additional_max); >+ >+ credits_granted = 1 + additional_credits; > } else if (sconn->smb2.credits_granted == 0) { > /* > * Make sure the client has always at least one credit >@@ -566,7 +596,7 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, > /* remove UINT64_MAX */ > credits_possible -= 1; > } >- credits_possible = MIN(credits_possible, sconn->smb2.max_credits); >+ credits_possible = MIN(credits_possible, current_max_credits); > credits_possible -= sconn->smb2.seqnum_range; > > credits_granted = MIN(credits_granted, credits_possible); >@@ -576,11 +606,12 @@ static void smb2_set_operation_credit(struct smbd_server_connection *sconn, > sconn->smb2.seqnum_range += credits_granted; > > DEBUG(10,("smb2_set_operation_credit: requested %u, " >- "granted %u, current possible %u, " >+ "granted %u, current possible/max %u/%u, " > "total granted/max/low/range %u/%u/%llu/%u\n", > (unsigned int)credits_requested, > (unsigned int)credits_granted, > (unsigned int)credits_possible, >+ (unsigned int)current_max_credits, > (unsigned int)sconn->smb2.credits_granted, > (unsigned int)sconn->smb2.max_credits, > (unsigned long long)sconn->smb2.seqnum_low, >-- >1.7.4.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=7716">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
jra
:
review+
Actions:
View
Attachments on
bug 9057
: 7716