diff mbox

[SRU,Yakkety,1/1] cifs: Do not send echoes before Negotiate is complete

Message ID 3cfcabfda498cf29702730e7c719dd8335012608.1496169951.git.joseph.salisbury@canonical.com
State New
Headers show

Commit Message

Joseph Salisbury May 30, 2017, 6:55 p.m. UTC
From: Sachin Prabhu <sprabhu@redhat.com>

BugLink: http://bugs.launchpad.net/bugs/1686099

commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect
long after socket reconnect") added support for Negotiate requests to
be initiated by echo calls.

To avoid delays in calling echo after a reconnect, I added the patch
introduced by the commit b8c600120fc8 ("Call echo service immediately
after socket reconnect").

This has however caused a regression with cifs shares which do not have
support for echo calls to trigger Negotiate requests. On connections
which need to call Negotiation, the echo calls trigger an error which
triggers a reconnect which in turn triggers another echo call. This
results in a loop which is only broken when an operation is performed on
the cifs share. For an idle share, it can DOS a server.

The patch uses the smb_operation can_echo() for cifs so that it is
called only if connection has been already been setup.

kernel bz: 194531

Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
Tested-by: Jonathan Liu <net147@gmail.com>
Acked-by: Pavel Shilovsky <pshilov@microsoft.com>
CC: Stable <stable@vger.kernel.org>
Signed-off-by: Steve French <smfrench@gmail.com>
(cherry picked from commit 62a6cfddcc0a5313e7da3e8311ba16226fe0ac10)
Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
---
 fs/cifs/smb1ops.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Stefan Bader May 31, 2017, 7:26 a.m. UTC | #1

Colin Ian King May 31, 2017, 7:57 a.m. UTC | #2
On 30/05/17 19:55, Joseph Salisbury wrote:
> From: Sachin Prabhu <sprabhu@redhat.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1686099
> 
> commit 4fcd1813e640 ("Fix reconnect to not defer smb3 session reconnect
> long after socket reconnect") added support for Negotiate requests to
> be initiated by echo calls.
> 
> To avoid delays in calling echo after a reconnect, I added the patch
> introduced by the commit b8c600120fc8 ("Call echo service immediately
> after socket reconnect").
> 
> This has however caused a regression with cifs shares which do not have
> support for echo calls to trigger Negotiate requests. On connections
> which need to call Negotiation, the echo calls trigger an error which
> triggers a reconnect which in turn triggers another echo call. This
> results in a loop which is only broken when an operation is performed on
> the cifs share. For an idle share, it can DOS a server.
> 
> The patch uses the smb_operation can_echo() for cifs so that it is
> called only if connection has been already been setup.
> 
> kernel bz: 194531
> 
> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
> Tested-by: Jonathan Liu <net147@gmail.com>
> Acked-by: Pavel Shilovsky <pshilov@microsoft.com>
> CC: Stable <stable@vger.kernel.org>
> Signed-off-by: Steve French <smfrench@gmail.com>
> (cherry picked from commit 62a6cfddcc0a5313e7da3e8311ba16226fe0ac10)
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
> ---
>  fs/cifs/smb1ops.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
> index fc537c2..87b87e0 100644
> --- a/fs/cifs/smb1ops.c
> +++ b/fs/cifs/smb1ops.c
> @@ -1015,6 +1015,15 @@ cifs_dir_needs_close(struct cifsFileInfo *cfile)
>  	return !cfile->srch_inf.endOfSearch && !cfile->invalidHandle;
>  }
>  
> +static bool
> +cifs_can_echo(struct TCP_Server_Info *server)
> +{
> +	if (server->tcpStatus == CifsGood)
> +		return true;
> +
> +	return false;
> +}
> +
>  struct smb_version_operations smb1_operations = {
>  	.send_cancel = send_nt_cancel,
>  	.compare_fids = cifs_compare_fids,
> @@ -1049,6 +1058,7 @@ struct smb_version_operations smb1_operations = {
>  	.get_dfs_refer = CIFSGetDFSRefer,
>  	.qfs_tcon = cifs_qfs_tcon,
>  	.is_path_accessible = cifs_is_path_accessible,
> +	.can_echo = cifs_can_echo,
>  	.query_path_info = cifs_query_path_info,
>  	.query_file_info = cifs_query_file_info,
>  	.get_srv_inum = cifs_get_srv_inum,
> 

Looks good. Thanks Joe

Acked-by: Colin Ian King <colin.king@canonical.com>
diff mbox

Patch

diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index fc537c2..87b87e0 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -1015,6 +1015,15 @@  cifs_dir_needs_close(struct cifsFileInfo *cfile)
 	return !cfile->srch_inf.endOfSearch && !cfile->invalidHandle;
 }
 
+static bool
+cifs_can_echo(struct TCP_Server_Info *server)
+{
+	if (server->tcpStatus == CifsGood)
+		return true;
+
+	return false;
+}
+
 struct smb_version_operations smb1_operations = {
 	.send_cancel = send_nt_cancel,
 	.compare_fids = cifs_compare_fids,
@@ -1049,6 +1058,7 @@  struct smb_version_operations smb1_operations = {
 	.get_dfs_refer = CIFSGetDFSRefer,
 	.qfs_tcon = cifs_qfs_tcon,
 	.is_path_accessible = cifs_is_path_accessible,
+	.can_echo = cifs_can_echo,
 	.query_path_info = cifs_query_path_info,
 	.query_file_info = cifs_query_file_info,
 	.get_srv_inum = cifs_get_srv_inum,