Apple has a bug in credit handling with their smbfs, it seems :-)

[Jeremy Allison]

On Mon, Jan 21, 2019 at 07:22:48PM -0800, Richard Sharpe via samba-technical wrote:
> Hi folks,
> 
> Apple has a bug in their handling of credits when Samba returns
> STATUS_PENDING for a SESSION SETUP request.
> 
> Such responses only seem to occur when the Samba server is under heavy load.
> 
> Samba issues one credit in such cases, and, as per the spec issues
> zero credits in the final successful response to the SESSION SETUP.
> 
> The Mac then issues a TREE CREATE, consuming one credit and getting one credit.
> 
> The Mac then issues a compound CREATE and CLOSE. This is a protocol
> violation and Samba drops the connection.
> 
> I have communicated this issue to Apple, but I also created the
> attached patch to mitigate the issue while Apple thinks about fixing
> their code.
> 
> Is it worth doing this? Is it going to work?

So this is sending credits on the SESSION SETUP
interim reply, and then also on the final SESSION SETUP
reply, yeah ?

Have you tested this against the Apple client ?

It's really ugly and a protocol violation :-).

How often does this problem occur ? If it did
go in it'd have to be configured off by default
but I'm not convinced yet :-).

Jeremy.

> Regards,
> Richard Sharpe
> (何以解憂？唯有杜康。--曹操)(传说杜康是酒的发明者)

> From 887bff34ef409208d5e9511fddd75e808c6f3aa9 Mon Sep 17 00:00:00 2001
> From: Richard Sharpe <richard.sharpe at primarydata.com>
> Date: Mon, 21 Jan 2019 17:22:27 -0800
> Subject: [PATCH] Don't issue 1 credit for the pending response of a SESSION
>  SETUP request
> 
> Signed-off-by: Richard Sharpe <realrichardsharpe at gmail.com>
> ---
>  source3/smbd/smb2_server.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
> index e36db1e..4843606 100644
> --- a/source3/smbd/smb2_server.c
> +++ b/source3/smbd/smb2_server.c
> @@ -852,7 +852,7 @@ static void smb2_set_operation_credit(struct smbXsrv_connection *xconn,
>  		return;
>  	}
>  
> -	if (out_flags & SMB2_HDR_FLAG_ASYNC) {
> +	if ((out_flags & SMB2_HDR_FLAG_ASYNC) && (cmd != SMB2_OP_SESSSETUP)) {
>  		/*
>  		 * In case we already send an async interim
>  		 * response, we should not grant
> @@ -877,6 +877,12 @@ static void smb2_set_operation_credit(struct smbXsrv_connection *xconn,
>  			if (NT_STATUS_IS_OK(out_status)) {
>  				additional_max = 32;
>  			}
> +			if (cout_flags & SMB2_HDR_FLAG_ASYNC) {
> +				/*
> +				 * Adjust for the one we already issued.
> +				 */
> +				additional_max -= 1;
> +			}
>  			break;
>  		default:
>  			/*
> -- 
> 2.3.6
>