From 9e5c82177910b8ef9ff6a7754b83545b703d4d0a Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher 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 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 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 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 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 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 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 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 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 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