| Message ID | CAH2r5mvRQj1hyDbBY8DTMtDShr2uxmJqYWWJg+H=iO3RUDc3oQ@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [CIFS] Fix match_server check to allow for multidialect negotiate | expand |
сб, 8 июн. 2019 г. в 02:06, Steve French <smfrench@gmail.com>: > > 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@microsoft.com> > --- > 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
Updated version with your suggestion - see attached On Mon, Jun 10, 2019 at 1:41 PM Pavel Shilovsky <piastryyy@gmail.com> wrote: > > сб, 8 июн. 2019 г. в 02:06, Steve French <smfrench@gmail.com>: > > > > 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@microsoft.com> > > --- > > 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
Since this patch - I also fixed the misspelled word 'specifying' On Thu, Jun 13, 2019 at 2:32 PM Steve French <smfrench@gmail.com> wrote: > > Updated version with your suggestion - see attached > > On Mon, Jun 10, 2019 at 1:41 PM Pavel Shilovsky <piastryyy@gmail.com> wrote: > > > > сб, 8 июн. 2019 г. в 02:06, Steve French <smfrench@gmail.com>: > > > > > > 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@microsoft.com> > > > --- > > > 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
чт, 13 июн. 2019 г. в 12:34, Steve French <smfrench@gmail.com>: > > Since this patch - I also fixed the misspelled word 'specifying' > > On Thu, Jun 13, 2019 at 2:32 PM Steve French <smfrench@gmail.com> wrote: > > > > Updated version with your suggestion - see attached > > > > On Mon, Jun 10, 2019 at 1:41 PM Pavel Shilovsky <piastryyy@gmail.com> wrote: > > > > > > сб, 8 июн. 2019 г. в 02:06, Steve French <smfrench@gmail.com>: > > > > > > > > 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@microsoft.com> > > > > --- > > > > 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 > > > > -- > Thanks, > > Steve Looks good. Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> -- Best regards, Pavel Shilovsky
From fd5e01a89742bb85d758a6651294a8a20193bc27 Mon Sep 17 00:00:00 2001 From: Steve French <stfrench@microsoft.com> Date: Sat, 8 Jun 2019 03:56:29 -0500 Subject: [PATCH] [CIFS] Fix match_server check to allow for multidialect negotiate 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@microsoft.com> --- 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 */ + } 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; + /* 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
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@microsoft.com> --- fs/cifs/connect.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) if (!net_eq(cifs_net_ns(server), current->nsproxy->net_ns))