From 479ad21a53dee0921d2517319a167db20b485568 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Wed, 28 Jan 2015 15:22:30 +0100 Subject: [PATCH 1/2] s3:smb2_server: always try to grant the credits the client just consumed It turns out that the effective credits_requested is always at least 1, even if the client sends credits_requested == 0. This means the client is not able to reduce the amount of credits itself. Without this fix a client (e.g. Windows7) would reach the case where it has been granted all credits it asked for. When copying a large file with a lot of parallel requests, all these requests have credits_requested == 0. This means the amount of granted credits where reduced by each request and only when the granted credits reached 0, the server granted one credit to allow the client to go on. The client might require more than one credit ([MS-SMB2] says Windows clients require at least 4 credits) and freezes with just 1 credit. Bug: https://bugzilla.samba.org/show_bug.cgi?id=9702 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme Reviewed-by: Jeremy Allison --- source3/smbd/smb2_server.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index 46bf6f9..a740b40 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -803,6 +803,7 @@ static void smb2_set_operation_credit(struct smbXsrv_connection *xconn, cmd = SVAL(inhdr, SMB2_HDR_OPCODE); credits_requested = SVAL(inhdr, SMB2_HDR_CREDIT); + credits_requested = MAX(credits_requested, 1); out_flags = IVAL(outhdr, SMB2_HDR_FLAGS); out_status = NT_STATUS(IVAL(outhdr, SMB2_HDR_STATUS)); @@ -821,7 +822,7 @@ static void smb2_set_operation_credit(struct smbXsrv_connection *xconn, * credits on the final response. */ credits_granted = 0; - } else if (credits_requested > 0) { + } else { uint16_t additional_max = 0; uint16_t additional_credits = credits_requested - 1; @@ -850,11 +851,6 @@ static void smb2_set_operation_credit(struct smbXsrv_connection *xconn, additional_credits = MIN(additional_credits, additional_max); credits_granted = credit_charge + additional_credits; - } else if (xconn->smb2.credits.granted == 0) { - /* - * Make sure the client has always at least one credit - */ - credits_granted = 1; } /* -- 1.9.1 From 10ba023f03310b22494d092e16e74b599d64277e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 29 Jan 2015 10:12:30 +0100 Subject: [PATCH 2/2] s3:smb2_server: protect against integer wrap with "smb2 max credits = 65535" Bug: https://bugzilla.samba.org/show_bug.cgi?id=9702 Signed-off-by: Stefan Metzmacher Reviewed-by: Ralph Boehme --- source3/smbd/smb2_server.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c index a740b40..25d11b1 100644 --- a/source3/smbd/smb2_server.c +++ b/source3/smbd/smb2_server.c @@ -823,6 +823,8 @@ static void smb2_set_operation_credit(struct smbXsrv_connection *xconn, */ credits_granted = 0; } else { + uint16_t additional_possible = + xconn->smb2.credits.max - credit_charge; uint16_t additional_max = 0; uint16_t additional_credits = credits_requested - 1; @@ -848,6 +850,7 @@ static void smb2_set_operation_credit(struct smbXsrv_connection *xconn, break; } + additional_max = MIN(additional_max, additional_possible); additional_credits = MIN(additional_credits, additional_max); credits_granted = credit_charge + additional_credits; -- 1.9.1