Updated version with your suggestion - see attached
On Mon, Jun 10, 2019 at 1:41 PM Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote:
>
> сб, 8 июн. 2019 г. в 02:06, Steve French <smfrench@xxxxxxxxx>:
> >
> > When using multidialect negotiate (default or allowing any smb3
> > dialect via vers=3)
> > allow matching on existing server session. Before this fix if you mount
> > a second time to a different share on the same server, we will only reuse
> > the existing smb session if a single dialect is requested (e.g. specifying
> > vers=2.1 or vers=3.0 or vers=3.1.1 on the mount command). If a default mount
> > (e.g. not specifying vers=) is done then we will create a new socket
> > connection and SMB3 (or SMB3.1.1) session each time we connect to a
> > different share
> > on the same server rather than reusing the existing one.
> >
> > Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
> > ---
> > fs/cifs/connect.c | 21 +++++++++++++++++++--
> > 1 file changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index 8c4121da624e..6200207565db 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -2542,8 +2542,25 @@ static int match_server(struct TCP_Server_Info
> > *server, struct smb_vol *vol)
> > if (vol->nosharesock)
> > return 0;
> >
> > - /* BB update this for smb3any and default case */
> > - if ((server->vals != vol->vals) || (server->ops != vol->ops))
> > + /* If multidialect negotiation see if existing sessions match one */
> > + if (strcmp(vol->vals->version_string, SMB3ANY_VERSION_STRING) == 0) {
> > + if (server->vals->protocol_id == SMB20_PROT_ID)
> > + return 0;
> > + else if (server->vals->protocol_id == SMB21_PROT_ID)
> > + return 0;
> > + else if (strcmp(server->vals->version_string,
> > + SMB1_VERSION_STRING) == 0)
> > + return 0;
> > + /* else SMB3 or later, which is fine */
>
> May be better to check
>
> if (server->vals->protocol_id < SMB30_PROT_ID)
> return 0;
>
> ? SMB1 case should work too because protocol_id is 0.
>
>
> > + } else if (strcmp(vol->vals->version_string,
> > + SMBDEFAULT_VERSION_STRING) == 0) {
> > + if (server->vals->protocol_id == SMB20_PROT_ID)
> > + return 0;
> > + else if (strcmp(server->vals->version_string,
> > + SMB1_VERSION_STRING) == 0)
> > + return 0;
>
> and here the same way:
>
> if (server->vals->protocol_id < SMB21_PROT_ID)
> return 0;
>
> > + /* else SMB2.1 or later, which is fine */
> > + } else if ((server->vals != vol->vals) || (server->ops != vol->ops))
> > return 0;
> >
> > if (!net_eq(cifs_net_ns(server), current->nsproxy->net_ns))
> > --
> > 2.20.1
>
> In this case we can avoid nested IFs - should be cleaner I guess.
>
>
> --
> Best regards,
> Pavel Shilovsky
--
Thanks,
Steve
From 33fe924c49c4e27e4ef23e6f6c844ddae9078044 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@xxxxxxxxxxxxx>
Date: Thu, 13 Jun 2019 14:26:49 -0500
Subject: [PATCH] [CIFS] Fix match_server check to allow for auto dialect
negotiate
When using multidialect negotiate (default or specifing vers=3.0 which
allows any smb3 dialect), fix how we check for an existing server session.
Before this fix if you mounted a second time to the same server (e.g. a
different share on the same server) we would only reuse the existing smb
session if a single dialect were requested (e.g. specifying vers=2.1 or vers=3.0
or vers=3.1.1 on the mount command). If a default mount (e.g. not
specifying vers=) is done then would always create a new socket connection
and SMB3 (or SMB3.1.1) session each time we connect to a different share
on the same server rather than reusing the existing one.
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
fs/cifs/connect.c | 11 +++++++++--
fs/cifs/smb1ops.c | 1 +
fs/cifs/smb2pdu.h | 1 +
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8c4121da624e..854dd077f2fc 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2542,8 +2542,15 @@ static int match_server(struct TCP_Server_Info *server, struct smb_vol *vol)
if (vol->nosharesock)
return 0;
- /* BB update this for smb3any and default case */
- if ((server->vals != vol->vals) || (server->ops != vol->ops))
+ /* If multidialect negotiation see if existing sessions match one */
+ if (strcmp(vol->vals->version_string, SMB3ANY_VERSION_STRING) == 0) {
+ if (server->vals->protocol_id < SMB30_PROT_ID)
+ return 0;
+ } else if (strcmp(vol->vals->version_string,
+ SMBDEFAULT_VERSION_STRING) == 0) {
+ if (server->vals->protocol_id < SMB21_PROT_ID)
+ return 0;
+ } else if ((server->vals != vol->vals) || (server->ops != vol->ops))
return 0;
if (!net_eq(cifs_net_ns(server), current->nsproxy->net_ns))
diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index c4e75afa3258..73a94bb41704 100644
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -1245,6 +1245,7 @@ struct smb_version_operations smb1_operations = {
struct smb_version_values smb1_values = {
.version_string = SMB1_VERSION_STRING,
+ .protocol_id = SMB10_PROT_ID,
.large_lock_type = LOCKING_ANDX_LARGE_FILES,
.exclusive_lock_type = 0,
.shared_lock_type = LOCKING_ANDX_SHARED_LOCK,
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index d3a64cf812d9..23524fe947de 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -227,6 +227,7 @@ struct smb2_negotiate_req {
} __packed;
/* Dialects */
+#define SMB10_PROT_ID 0x0000 /* local only, not sent on wire w/CIFS negprot */
#define SMB20_PROT_ID 0x0202
#define SMB21_PROT_ID 0x0210
#define SMB30_PROT_ID 0x0300
--
2.20.1
