Attachment 9214 Details for Bug 10113 – patch: stop cifs from using virtual circuits (revised)

[patch] patch: stop cifs from using virtual circuits (revised)

0001-cifs-stop-trying-to-use-virtual-circuits.patch (text/plain), 5.16 KB, created by Jeff Layton on 2013-09-14 14:05:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Layton

Created: 2013-09-14 14:05:44 UTC

Size: 5.16 KB

patch

obsolete

>From fdbe5094fb3536753d4b3b32813ccae696fcf75c Mon Sep 17 00:00:00 2001
>From: Jeff Layton <jlayton@redhat.com>
>Date: Sat, 14 Sep 2013 10:01:09 -0400
>Subject: [PATCH] cifs: stop trying to use virtual circuits
>
>Currently, we try to ensure that we use vcnum of 0 on the first
>established session on a connection and then try to use a different
>vcnum on each session after that.
>
>This is problematic in a couple of very common situations:
>
>1) it totally ignores that other programs on the box (e.g. smbclient)
>might have connections established to the server. Using a vcnum of 0
>causes them to get kicked off.
>
>2) it causes problems with NAT -- if several clients are running
>connected to the same server via the same NAT'ed address, whenever
>one connects to the server it kicks off all the others, which then
>reconnect and kick off the others...ad nauseum.
>
>I don't see any reason to ignore the advice in "Implementing CIFS". In
>there, it states "...and contrary to the specs the client should always
>use a VcNumber of one, never zero."
>
>Have the client just use a hardcoded vcnum of 1, and stop abusing the
>special behavior of vcnum 0.
>
>Reported-by: Sauron99@gmx.de <sauron99@gmx.de>
>Signed-off-by: Jeff Layton <jlayton@redhat.com>
>---
> fs/cifs/cifsglob.h |  1 -
> fs/cifs/sess.c     | 84 +-----------------------------------------------------
> 2 files changed, 1 insertion(+), 84 deletions(-)
>
>diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>index cfa14c8..60002c3 100644
>--- a/fs/cifs/cifsglob.h
>+++ b/fs/cifs/cifsglob.h
>@@ -715,7 +715,6 @@ struct cifs_ses {
> 	enum statusEnum status;
> 	unsigned overrideSecFlg;  /* if non-zero override global sec flags */
> 	__u16 ipc_tid;		/* special tid for connection to IPC share */
>-	__u16 vcnum;
> 	char *serverOS;		/* name of operating system underlying server */
> 	char *serverNOS;	/* name of network operating system of server */
> 	char *serverDomain;	/* security realm of server */
>diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
>index 5f99b7f..352358d 100644
>--- a/fs/cifs/sess.c
>+++ b/fs/cifs/sess.c
>@@ -32,88 +32,6 @@
> #include <linux/slab.h>
> #include "cifs_spnego.h"
> 
>-/*
>- * Checks if this is the first smb session to be reconnected after
>- * the socket has been reestablished (so we know whether to use vc 0).
>- * Called while holding the cifs_tcp_ses_lock, so do not block
>- */
>-static bool is_first_ses_reconnect(struct cifs_ses *ses)
>-{
>-	struct list_head *tmp;
>-	struct cifs_ses *tmp_ses;
>-
>-	list_for_each(tmp, &ses->server->smb_ses_list) {
>-		tmp_ses = list_entry(tmp, struct cifs_ses,
>-				     smb_ses_list);
>-		if (tmp_ses->need_reconnect == false)
>-			return false;
>-	}
>-	/* could not find a session that was already connected,
>-	   this must be the first one we are reconnecting */
>-	return true;
>-}
>-
>-/*
>- *	vc number 0 is treated specially by some servers, and should be the
>- *      first one we request.  After that we can use vcnumbers up to maxvcs,
>- *	one for each smb session (some Windows versions set maxvcs incorrectly
>- *	so maxvc=1 can be ignored).  If we have too many vcs, we can reuse
>- *	any vc but zero (some servers reset the connection on vcnum zero)
>- *
>- */
>-static __le16 get_next_vcnum(struct cifs_ses *ses)
>-{
>-	__u16 vcnum = 0;
>-	struct list_head *tmp;
>-	struct cifs_ses *tmp_ses;
>-	__u16 max_vcs = ses->server->max_vcs;
>-	__u16 i;
>-	int free_vc_found = 0;
>-
>-	/* Quoting the MS-SMB specification: "Windows-based SMB servers set this
>-	field to one but do not enforce this limit, which allows an SMB client
>-	to establish more virtual circuits than allowed by this value ... but
>-	other server implementations can enforce this limit." */
>-	if (max_vcs < 2)
>-		max_vcs = 0xFFFF;
>-
>-	spin_lock(&cifs_tcp_ses_lock);
>-	if ((ses->need_reconnect) && is_first_ses_reconnect(ses))
>-			goto get_vc_num_exit;  /* vcnum will be zero */
>-	for (i = ses->server->srv_count - 1; i < max_vcs; i++) {
>-		if (i == 0) /* this is the only connection, use vc 0 */
>-			break;
>-
>-		free_vc_found = 1;
>-
>-		list_for_each(tmp, &ses->server->smb_ses_list) {
>-			tmp_ses = list_entry(tmp, struct cifs_ses,
>-					     smb_ses_list);
>-			if (tmp_ses->vcnum == i) {
>-				free_vc_found = 0;
>-				break; /* found duplicate, try next vcnum */
>-			}
>-		}
>-		if (free_vc_found)
>-			break; /* we found a vcnumber that will work - use it */
>-	}
>-
>-	if (i == 0)
>-		vcnum = 0; /* for most common case, ie if one smb session, use
>-			      vc zero.  Also for case when no free vcnum, zero
>-			      is safest to send (some clients only send zero) */
>-	else if (free_vc_found == 0)
>-		vcnum = 1;  /* we can not reuse vc=0 safely, since some servers
>-				reset all uids on that, but 1 is ok. */
>-	else
>-		vcnum = i;
>-	ses->vcnum = vcnum;
>-get_vc_num_exit:
>-	spin_unlock(&cifs_tcp_ses_lock);
>-
>-	return cpu_to_le16(vcnum);
>-}
>-
> static __u32 cifs_ssetup_hdr(struct cifs_ses *ses, SESSION_SETUP_ANDX *pSMB)
> {
> 	__u32 capabilities = 0;
>@@ -128,7 +46,7 @@ static __u32 cifs_ssetup_hdr(struct cifs_ses *ses, SESSION_SETUP_ANDX *pSMB)
> 					CIFSMaxBufSize + MAX_CIFS_HDR_SIZE - 4,
> 					USHRT_MAX));
> 	pSMB->req.MaxMpxCount = cpu_to_le16(ses->server->maxReq);
>-	pSMB->req.VcNumber = get_next_vcnum(ses);
>+	pSMB->req.VcNumber = __constant_cpu_to_le16(1);
> 
> 	/* Now no need to set SMBFLG_CASELESS or obsolete CANONICAL PATH */
> 
>-- 
>1.8.3.1
>

Actions: View

Attachments on bug 10113: 9213 | 9214