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

2 files changed, 92 insertions, 35 deletions

diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h
index 4cbafe47ca..9512d05735 100644
--- a/source3/smbd/globals.h
+++ b/source3/smbd/globals.h
@@ -641,14 +641,49 @@ 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;
+		bool supports_multicredit;
 		uint32_t max_trans;
 		uint32_t max_read;
 		uint32_t max_write;
-		bool supports_multicredit;
-		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 872e5b9dce..320923a4cf 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -109,6 +109,7 @@ static NTSTATUS smbd_initialize_smb2(struct smbd_server_connection *sconn)
 	}
 
 	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,
@@ -301,21 +302,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;
 	}
 
@@ -323,12 +328,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;
 	}
@@ -352,6 +359,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,23 +384,25 @@ static bool smb2_validate_message_id(struct smbd_server_connection *sconn,
 		credit_charge = MAX(credit_charge, 1);
 	}
 
-	DEBUG(11, ("smb2_validate_message_id: mid %llu, credits_granted %llu, "
-		   "charge %llu, max_credits %llu, seqnum_low: %llu\n",
+	DEBUG(11, ("smb2_validate_message_id: mid %llu (charge %llu), "
+		   "credits_granted %llu, "
+		   "seqnum low/range: %llu/%llu\n",
 		   (unsigned long long) message_id,
-		   (unsigned long long) sconn->smb2.credits_granted,
 		   (unsigned long long) credit_charge,
-		   (unsigned long long) sconn->smb2.max_credits,
-		   (unsigned long long) sconn->smb2.seqnum_low));
+		   (unsigned long long) sconn->smb2.credits_granted,
+		   (unsigned long long) sconn->smb2.seqnum_low,
+		   (unsigned long long) sconn->smb2.seqnum_range));
 
 	if (sconn->smb2.credits_granted < credit_charge) {
 		DEBUG(0, ("smb2_validate_message_id: client used more "
-			  "credits than granted, mid %llu, credits_granted %llu, "
-			  "charge %llu, max_credits %llu, seqnum_low: %llu\n",
+			  "credits than granted, mid %llu, charge %llu, "
+			  "credits_granted %llu, "
+			  "seqnum low/range: %llu/%llu\n",
 			  (unsigned long long) message_id,
-			  (unsigned long long) sconn->smb2.credits_granted,
 			  (unsigned long long) credit_charge,
-			  (unsigned long long) sconn->smb2.max_credits,
-			  (unsigned long long) sconn->smb2.seqnum_low));
+			  (unsigned long long) sconn->smb2.credits_granted,
+			  (unsigned long long) sconn->smb2.seqnum_low,
+			  (unsigned long long) sconn->smb2.seqnum_range));
 		return false;
 	}
 
@@ -517,6 +527,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);
@@ -529,10 +540,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;
 
@@ -552,25 +561,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,