[PATCH] Fix bug 11079 - libsmbclient not checking the cached connection alive status before re-using it from connection cache

[Jeremy Allison]

On Thu, Mar 19, 2015 at 11:33:23AM -0700, Jeremy Allison wrote:
> If a server disconnects a libsmbclient using app due to
> idle timeout (gnome VFS for example), we fail the first
> call after reselecting the SMBCSRV object from the cache.
> 
> We then correctly reconnect, but this doesn't fix the
> previous NT_STATUS_CONNECTION_RESET we've already passed
> back up the stack).
> 
> This patches fixes the problem by ensuring that if we're
> re-using a SMBCSRV object we haven't used for cli->timeout
> miliseconds (which is normally 20 seconds, much shorter
> than a typical server idle timeout length) we test the
> connection with an SMB1 or SMB2 echo request before allowing
> it to be re-used. If the echo fails, we then correctly
> reconnect before retrying the caller-initiated operation.
> 
> Tested (along with the previous patch for bug 11173,
> on which this depends) by the GNOME VFS maintainer.
> 
> Please review and push !

Oh yeah, before anyone comments - I just realized
the:

> +	now = time(NULL);
> +

should be:

> +	now = time_mono(NULL);
> +

instead to correctly get monotonically increasing time :-).

So review as if that were there (it will be in anything
I push :-).

Cheers,

	Jeremy.

> From bfca13f667824943b36dc45ea909b38f49cdc09e Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Wed, 18 Mar 2015 14:15:16 -0700
> Subject: [PATCH] s3: lib: libsmbclient: If reusing a server struct, check
>  every cli->timout miliseconds if it's still valid before use.
> 
> Uses an cli_echo() call to do so.
> 
> Based on code from <shargagan at novell.com>
> 
> Bug 11079 - libsmbclient not checking the cached connection alive status before re-using it from connection cache
> 
> https://bugzilla.samba.org/show_bug.cgi?id=11079
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  source3/include/libsmb_internal.h |  1 +
>  source3/libsmb/libsmb_server.c    | 16 ++++++++++++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/source3/include/libsmb_internal.h b/source3/include/libsmb_internal.h
> index ce73181..65fad99 100644
> --- a/source3/include/libsmb_internal.h
> +++ b/source3/include/libsmb_internal.h
> @@ -81,6 +81,7 @@ struct _SMBCSRV {
>  	bool no_pathinfo3;
>          bool no_nt_session;
>          struct policy_handle pol;
> +	time_t last_echo_time;
>  
>  	SMBCSRV *next, *prev;
>  };
> diff --git a/source3/libsmb/libsmb_server.c b/source3/libsmb/libsmb_server.c
> index 8f68a40..d05570d 100644
> --- a/source3/libsmb/libsmb_server.c
> +++ b/source3/libsmb/libsmb_server.c
> @@ -45,10 +45,26 @@ int
>  SMBC_check_server(SMBCCTX * context,
>                    SMBCSRV * server)
>  {
> +	time_t now;
> +
>  	if (!cli_state_is_connected(server->cli)) {
>  		return 1;
>  	}
>  
> +	now = time(NULL);
> +
> +	if (server->last_echo_time == (time_t)0 ||
> +			now > server->last_echo_time +
> +				(server->cli->timeout/1000)) {
> +		unsigned char data[16] = {0};
> +		NTSTATUS status = cli_echo(server->cli,
> +					1,
> +					data_blob_const(data, sizeof(data)));
> +		if (!NT_STATUS_IS_OK(status)) {
> +			return 1;
> +		}
> +		server->last_echo_time = now;
> +	}
>  	return 0;
>  }
>  
> -- 
> 2.2.0.rc0.207.ga3a616c
>