* PATCH] smb3: include current dialect (SMB3.1.1) when version 3 or greater requested on mount
@ 2021-02-02 6:15 Steve French
2021-02-02 17:58 ` Pavel Shilovsky
0 siblings, 1 reply; 4+ messages in thread
From: Steve French @ 2021-02-02 6:15 UTC (permalink / raw)
To: CIFS, samba-technical; +Cc: Pavel Shilovsky
[-- Attachment #1: Type: text/plain, Size: 3937 bytes --]
SMB3.1.1 is the newest, and preferred dialect, and is included in
the requested dialect list by default (ie if no vers= is specified
on mount) but it should also be requested if SMB3 or later is requested
(vers=3 instead of a specific dialect: vers=2.1, vers=3.02 or vers=3.0).
Currently specifying "vers=3" only requests smb3.0 and smb3.02 but this
patch fixes it to also request smb3.1.1 dialect, as it is the newest
and most secure dialect and is a "version 3 or later" dialect (the intent
of "vers=3").
Signed-off-by: Steve French <stfrench@microsoft.com>
Suggested-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/fs_context.c | 2 +-
fs/cifs/smb2pdu.c | 19 +++++++++++++------
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 5111aadfdb6b..479c24695281 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -391,7 +391,7 @@ cifs_parse_smb_version(char *value, struct
smb3_fs_context *ctx, bool is_smb3)
ctx->vals = &smb3any_values;
break;
case Smb_default:
- ctx->ops = &smb30_operations; /* currently identical with 3.0 */
+ ctx->ops = &smb30_operations;
ctx->vals = &smbdefault_values;
break;
default:
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 794fc3b68b4f..52625549c3b5 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -814,8 +814,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
SMB3ANY_VERSION_STRING) == 0) {
req->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
- req->DialectCount = cpu_to_le16(2);
- total_len += 4;
+ req->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
+ req->DialectCount = cpu_to_le16(3);
+ total_len += 6;
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
@@ -848,6 +849,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
memcpy(req->ClientGUID, server->client_guid,
SMB2_CLIENT_GUID_SIZE);
if ((server->vals->protocol_id == SMB311_PROT_ID) ||
+ (strcmp(server->vals->version_string,
+ SMB3ANY_VERSION_STRING) == 0) ||
(strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0))
assemble_neg_contexts(req, server, &total_len);
@@ -883,6 +886,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
cifs_server_dbg(VFS,
"SMB2.1 dialect returned but not requested\n");
return -EIO;
+ } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID)) {
+ server->ops = &smb311_operations;
+ server->vals = &smb311_values;
}
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
@@ -1042,10 +1048,11 @@ int smb3_validate_negotiate(const unsigned int
xid, struct cifs_tcon *tcon)
SMB3ANY_VERSION_STRING) == 0) {
pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
- pneg_inbuf->DialectCount = cpu_to_le16(2);
- /* structure is big enough for 3 dialects, sending only 2 */
+ pneg_inbuf->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
+ pneg_inbuf->DialectCount = cpu_to_le16(3);
+ /* SMB 2.1 not included so subtract one dialect from len */
inbuflen = sizeof(*pneg_inbuf) -
- (2 * sizeof(pneg_inbuf->Dialects[0]));
+ (sizeof(pneg_inbuf->Dialects[0]));
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
@@ -1053,7 +1060,7 @@ int smb3_validate_negotiate(const unsigned int
xid, struct cifs_tcon *tcon)
pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
pneg_inbuf->DialectCount = cpu_to_le16(4);
- /* structure is big enough for 3 dialects */
+ /* structure is big enough for 4 dialects */
inbuflen = sizeof(*pneg_inbuf);
} else {
/* otherwise specific dialect was requested */
--
Thanks,
Steve
[-- Attachment #2: 0001-smb3-negotiate-current-dialect-SMB3.1.1-when-version.patch --]
[-- Type: text/x-patch, Size: 4251 bytes --]
From e7055cdca03ea5522992345c9248cdbfed5c9d6a Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Tue, 2 Feb 2021 00:03:58 -0600
Subject: [PATCH] smb3: negotiate current dialect (SMB3.1.1) when version 3 or
greater requested
SMB3.1.1 is the newest, and preferred dialect, and is included in
the requested dialect list by default (ie if no vers= is specified
on mount) but it should also be requested if SMB3 or later is requested
(vers=3 instead of a specific dialect: vers=2.1, vers=3.02 or vers=3.0).
Currently specifying "vers=3" only requests smb3.0 and smb3.02 but this
patch fixes it to also request smb3.1.1 dialect, as it is the newest
and most secure dialect and is a "version 3 or later" dialect (the intent
of "vers=3" dialect).
Signed-off-by: Steve French <stfrench@microsoft.com>
Suggested-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/fs_context.c | 2 +-
fs/cifs/smb2pdu.c | 19 +++++++++++++------
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 5111aadfdb6b..479c24695281 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -391,7 +391,7 @@ cifs_parse_smb_version(char *value, struct smb3_fs_context *ctx, bool is_smb3)
ctx->vals = &smb3any_values;
break;
case Smb_default:
- ctx->ops = &smb30_operations; /* currently identical with 3.0 */
+ ctx->ops = &smb30_operations;
ctx->vals = &smbdefault_values;
break;
default:
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 794fc3b68b4f..52625549c3b5 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -814,8 +814,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
SMB3ANY_VERSION_STRING) == 0) {
req->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
- req->DialectCount = cpu_to_le16(2);
- total_len += 4;
+ req->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
+ req->DialectCount = cpu_to_le16(3);
+ total_len += 6;
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
@@ -848,6 +849,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
memcpy(req->ClientGUID, server->client_guid,
SMB2_CLIENT_GUID_SIZE);
if ((server->vals->protocol_id == SMB311_PROT_ID) ||
+ (strcmp(server->vals->version_string,
+ SMB3ANY_VERSION_STRING) == 0) ||
(strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0))
assemble_neg_contexts(req, server, &total_len);
@@ -883,6 +886,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
cifs_server_dbg(VFS,
"SMB2.1 dialect returned but not requested\n");
return -EIO;
+ } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID)) {
+ server->ops = &smb311_operations;
+ server->vals = &smb311_values;
}
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
@@ -1042,10 +1048,11 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
SMB3ANY_VERSION_STRING) == 0) {
pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
- pneg_inbuf->DialectCount = cpu_to_le16(2);
- /* structure is big enough for 3 dialects, sending only 2 */
+ pneg_inbuf->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
+ pneg_inbuf->DialectCount = cpu_to_le16(3);
+ /* SMB 2.1 not included so subtract one dialect from len */
inbuflen = sizeof(*pneg_inbuf) -
- (2 * sizeof(pneg_inbuf->Dialects[0]));
+ (sizeof(pneg_inbuf->Dialects[0]));
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
@@ -1053,7 +1060,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
pneg_inbuf->DialectCount = cpu_to_le16(4);
- /* structure is big enough for 3 dialects */
+ /* structure is big enough for 4 dialects */
inbuflen = sizeof(*pneg_inbuf);
} else {
/* otherwise specific dialect was requested */
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: PATCH] smb3: include current dialect (SMB3.1.1) when version 3 or greater requested on mount
2021-02-02 6:15 PATCH] smb3: include current dialect (SMB3.1.1) when version 3 or greater requested on mount Steve French
@ 2021-02-02 17:58 ` Pavel Shilovsky
2021-02-03 4:05 ` Steve French
0 siblings, 1 reply; 4+ messages in thread
From: Pavel Shilovsky @ 2021-02-02 17:58 UTC (permalink / raw)
To: Steve French; +Cc: CIFS, samba-technical
пн, 1 февр. 2021 г. в 22:15, Steve French <smfrench@gmail.com>:
>
> SMB3.1.1 is the newest, and preferred dialect, and is included in
> the requested dialect list by default (ie if no vers= is specified
> on mount) but it should also be requested if SMB3 or later is requested
> (vers=3 instead of a specific dialect: vers=2.1, vers=3.02 or vers=3.0).
>
> Currently specifying "vers=3" only requests smb3.0 and smb3.02 but this
> patch fixes it to also request smb3.1.1 dialect, as it is the newest
> and most secure dialect and is a "version 3 or later" dialect (the intent
> of "vers=3").
>
> Signed-off-by: Steve French <stfrench@microsoft.com>
> Suggested-by: Pavel Shilovsky <pshilov@microsoft.com>
> ---
> fs/cifs/fs_context.c | 2 +-
> fs/cifs/smb2pdu.c | 19 +++++++++++++------
> 2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> index 5111aadfdb6b..479c24695281 100644
> --- a/fs/cifs/fs_context.c
> +++ b/fs/cifs/fs_context.c
> @@ -391,7 +391,7 @@ cifs_parse_smb_version(char *value, struct
> smb3_fs_context *ctx, bool is_smb3)
> ctx->vals = &smb3any_values;
> break;
> case Smb_default:
> - ctx->ops = &smb30_operations; /* currently identical with 3.0 */
> + ctx->ops = &smb30_operations;
> ctx->vals = &smbdefault_values;
> break;
> default:
> diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> index 794fc3b68b4f..52625549c3b5 100644
> --- a/fs/cifs/smb2pdu.c
> +++ b/fs/cifs/smb2pdu.c
> @@ -814,8 +814,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> SMB3ANY_VERSION_STRING) == 0) {
> req->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
> req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
> - req->DialectCount = cpu_to_le16(2);
> - total_len += 4;
> + req->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
> + req->DialectCount = cpu_to_le16(3);
> + total_len += 6;
> } else if (strcmp(server->vals->version_string,
> SMBDEFAULT_VERSION_STRING) == 0) {
> req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
> @@ -848,6 +849,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> memcpy(req->ClientGUID, server->client_guid,
> SMB2_CLIENT_GUID_SIZE);
> if ((server->vals->protocol_id == SMB311_PROT_ID) ||
> + (strcmp(server->vals->version_string,
> + SMB3ANY_VERSION_STRING) == 0) ||
> (strcmp(server->vals->version_string,
> SMBDEFAULT_VERSION_STRING) == 0))
> assemble_neg_contexts(req, server, &total_len);
> @@ -883,6 +886,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> cifs_server_dbg(VFS,
> "SMB2.1 dialect returned but not requested\n");
> return -EIO;
> + } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID)) {
I think we should include comment "/* ops set to 3.0 by default for
default so update */" as in smbdefault case to improve readability.
> + server->ops = &smb311_operations;
> + server->vals = &smb311_values;
> }
> } else if (strcmp(server->vals->version_string,
> SMBDEFAULT_VERSION_STRING) == 0) {
> @@ -1042,10 +1048,11 @@ int smb3_validate_negotiate(const unsigned int
> xid, struct cifs_tcon *tcon)
> SMB3ANY_VERSION_STRING) == 0) {
> pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
> pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
> - pneg_inbuf->DialectCount = cpu_to_le16(2);
> - /* structure is big enough for 3 dialects, sending only 2 */
> + pneg_inbuf->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
> + pneg_inbuf->DialectCount = cpu_to_le16(3);
> + /* SMB 2.1 not included so subtract one dialect from len */
> inbuflen = sizeof(*pneg_inbuf) -
> - (2 * sizeof(pneg_inbuf->Dialects[0]));
> + (sizeof(pneg_inbuf->Dialects[0]));
> } else if (strcmp(server->vals->version_string,
> SMBDEFAULT_VERSION_STRING) == 0) {
> pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
> @@ -1053,7 +1060,7 @@ int smb3_validate_negotiate(const unsigned int
> xid, struct cifs_tcon *tcon)
> pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
> pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
> pneg_inbuf->DialectCount = cpu_to_le16(4);
> - /* structure is big enough for 3 dialects */
> + /* structure is big enough for 4 dialects */
> inbuflen = sizeof(*pneg_inbuf);
> } else {
> /* otherwise specific dialect was requested */
>
> --
> Thanks,
>
> Steve
Looks good overall.
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
--
Best regards,
Pavel Shilovsky
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH] smb3: include current dialect (SMB3.1.1) when version 3 or greater requested on mount
2021-02-02 17:58 ` Pavel Shilovsky
@ 2021-02-03 4:05 ` Steve French
2021-02-03 11:22 ` Shyam Prasad N
0 siblings, 1 reply; 4+ messages in thread
From: Steve French @ 2021-02-03 4:05 UTC (permalink / raw)
To: Pavel Shilovsky; +Cc: CIFS, samba-technical
[-- Attachment #1: Type: text/plain, Size: 4958 bytes --]
updated as suggested (see attached) and merged into cifs-2.6.git for-next
On Tue, Feb 2, 2021 at 11:58 AM Pavel Shilovsky <piastryyy@gmail.com> wrote:
>
> пн, 1 февр. 2021 г. в 22:15, Steve French <smfrench@gmail.com>:
> >
> > SMB3.1.1 is the newest, and preferred dialect, and is included in
> > the requested dialect list by default (ie if no vers= is specified
> > on mount) but it should also be requested if SMB3 or later is requested
> > (vers=3 instead of a specific dialect: vers=2.1, vers=3.02 or vers=3.0).
> >
> > Currently specifying "vers=3" only requests smb3.0 and smb3.02 but this
> > patch fixes it to also request smb3.1.1 dialect, as it is the newest
> > and most secure dialect and is a "version 3 or later" dialect (the intent
> > of "vers=3").
> >
> > Signed-off-by: Steve French <stfrench@microsoft.com>
> > Suggested-by: Pavel Shilovsky <pshilov@microsoft.com>
> > ---
> > fs/cifs/fs_context.c | 2 +-
> > fs/cifs/smb2pdu.c | 19 +++++++++++++------
> > 2 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> > index 5111aadfdb6b..479c24695281 100644
> > --- a/fs/cifs/fs_context.c
> > +++ b/fs/cifs/fs_context.c
> > @@ -391,7 +391,7 @@ cifs_parse_smb_version(char *value, struct
> > smb3_fs_context *ctx, bool is_smb3)
> > ctx->vals = &smb3any_values;
> > break;
> > case Smb_default:
> > - ctx->ops = &smb30_operations; /* currently identical with 3.0 */
> > + ctx->ops = &smb30_operations;
> > ctx->vals = &smbdefault_values;
> > break;
> > default:
> > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> > index 794fc3b68b4f..52625549c3b5 100644
> > --- a/fs/cifs/smb2pdu.c
> > +++ b/fs/cifs/smb2pdu.c
> > @@ -814,8 +814,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> > SMB3ANY_VERSION_STRING) == 0) {
> > req->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
> > req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
> > - req->DialectCount = cpu_to_le16(2);
> > - total_len += 4;
> > + req->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
> > + req->DialectCount = cpu_to_le16(3);
> > + total_len += 6;
> > } else if (strcmp(server->vals->version_string,
> > SMBDEFAULT_VERSION_STRING) == 0) {
> > req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
> > @@ -848,6 +849,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> > memcpy(req->ClientGUID, server->client_guid,
> > SMB2_CLIENT_GUID_SIZE);
> > if ((server->vals->protocol_id == SMB311_PROT_ID) ||
> > + (strcmp(server->vals->version_string,
> > + SMB3ANY_VERSION_STRING) == 0) ||
> > (strcmp(server->vals->version_string,
> > SMBDEFAULT_VERSION_STRING) == 0))
> > assemble_neg_contexts(req, server, &total_len);
> > @@ -883,6 +886,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> > cifs_server_dbg(VFS,
> > "SMB2.1 dialect returned but not requested\n");
> > return -EIO;
> > + } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID)) {
>
> I think we should include comment "/* ops set to 3.0 by default for
> default so update */" as in smbdefault case to improve readability.
>
> > + server->ops = &smb311_operations;
> > + server->vals = &smb311_values;
> > }
> > } else if (strcmp(server->vals->version_string,
> > SMBDEFAULT_VERSION_STRING) == 0) {
> > @@ -1042,10 +1048,11 @@ int smb3_validate_negotiate(const unsigned int
> > xid, struct cifs_tcon *tcon)
> > SMB3ANY_VERSION_STRING) == 0) {
> > pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
> > pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
> > - pneg_inbuf->DialectCount = cpu_to_le16(2);
> > - /* structure is big enough for 3 dialects, sending only 2 */
> > + pneg_inbuf->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
> > + pneg_inbuf->DialectCount = cpu_to_le16(3);
> > + /* SMB 2.1 not included so subtract one dialect from len */
> > inbuflen = sizeof(*pneg_inbuf) -
> > - (2 * sizeof(pneg_inbuf->Dialects[0]));
> > + (sizeof(pneg_inbuf->Dialects[0]));
> > } else if (strcmp(server->vals->version_string,
> > SMBDEFAULT_VERSION_STRING) == 0) {
> > pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
> > @@ -1053,7 +1060,7 @@ int smb3_validate_negotiate(const unsigned int
> > xid, struct cifs_tcon *tcon)
> > pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
> > pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
> > pneg_inbuf->DialectCount = cpu_to_le16(4);
> > - /* structure is big enough for 3 dialects */
> > + /* structure is big enough for 4 dialects */
> > inbuflen = sizeof(*pneg_inbuf);
> > } else {
> > /* otherwise specific dialect was requested */
> >
> > --
> > Thanks,
> >
> > Steve
>
> Looks good overall.
>
> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
> --
> Best regards,
> Pavel Shilovsky
--
Thanks,
Steve
[-- Attachment #2: 0001-smb3-negotiate-current-dialect-SMB3.1.1-when-version.patch --]
[-- Type: text/x-patch, Size: 4356 bytes --]
From bfd2262035f43f8fe44f98a9bf3cf326d7a5fd5b Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Tue, 2 Feb 2021 00:03:58 -0600
Subject: [PATCH] smb3: negotiate current dialect (SMB3.1.1) when version 3 or
greater requested
SMB3.1.1 is the newest, and preferred dialect, and is included in
the requested dialect list by default (ie if no vers= is specified
on mount) but it should also be requested if SMB3 or later is requested
(vers=3 instead of a specific dialect: vers=2.1, vers=3.02 or vers=3.0).
Currently specifying "vers=3" only requests smb3.0 and smb3.02 but this
patch fixes it to also request smb3.1.1 dialect, as it is the newest
and most secure dialect and is a "version 3 or later" dialect (the intent
of "vers=3").
Signed-off-by: Steve French <stfrench@microsoft.com>
Suggested-by: Pavel Shilovsky <pshilov@microsoft.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
---
fs/cifs/fs_context.c | 2 +-
fs/cifs/smb2pdu.c | 20 ++++++++++++++------
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 5111aadfdb6b..479c24695281 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -391,7 +391,7 @@ cifs_parse_smb_version(char *value, struct smb3_fs_context *ctx, bool is_smb3)
ctx->vals = &smb3any_values;
break;
case Smb_default:
- ctx->ops = &smb30_operations; /* currently identical with 3.0 */
+ ctx->ops = &smb30_operations;
ctx->vals = &smbdefault_values;
break;
default:
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 794fc3b68b4f..e1391bd92768 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -814,8 +814,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
SMB3ANY_VERSION_STRING) == 0) {
req->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
- req->DialectCount = cpu_to_le16(2);
- total_len += 4;
+ req->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
+ req->DialectCount = cpu_to_le16(3);
+ total_len += 6;
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
@@ -848,6 +849,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
memcpy(req->ClientGUID, server->client_guid,
SMB2_CLIENT_GUID_SIZE);
if ((server->vals->protocol_id == SMB311_PROT_ID) ||
+ (strcmp(server->vals->version_string,
+ SMB3ANY_VERSION_STRING) == 0) ||
(strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0))
assemble_neg_contexts(req, server, &total_len);
@@ -883,6 +886,10 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
cifs_server_dbg(VFS,
"SMB2.1 dialect returned but not requested\n");
return -EIO;
+ } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID)) {
+ /* ops set to 3.0 by default for default so update */
+ server->ops = &smb311_operations;
+ server->vals = &smb311_values;
}
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
@@ -1042,10 +1049,11 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
SMB3ANY_VERSION_STRING) == 0) {
pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
- pneg_inbuf->DialectCount = cpu_to_le16(2);
- /* structure is big enough for 3 dialects, sending only 2 */
+ pneg_inbuf->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
+ pneg_inbuf->DialectCount = cpu_to_le16(3);
+ /* SMB 2.1 not included so subtract one dialect from len */
inbuflen = sizeof(*pneg_inbuf) -
- (2 * sizeof(pneg_inbuf->Dialects[0]));
+ (sizeof(pneg_inbuf->Dialects[0]));
} else if (strcmp(server->vals->version_string,
SMBDEFAULT_VERSION_STRING) == 0) {
pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
@@ -1053,7 +1061,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
pneg_inbuf->DialectCount = cpu_to_le16(4);
- /* structure is big enough for 3 dialects */
+ /* structure is big enough for 4 dialects */
inbuflen = sizeof(*pneg_inbuf);
} else {
/* otherwise specific dialect was requested */
--
2.27.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: PATCH] smb3: include current dialect (SMB3.1.1) when version 3 or greater requested on mount
2021-02-03 4:05 ` Steve French
@ 2021-02-03 11:22 ` Shyam Prasad N
0 siblings, 0 replies; 4+ messages in thread
From: Shyam Prasad N @ 2021-02-03 11:22 UTC (permalink / raw)
To: Steve French; +Cc: Pavel Shilovsky, CIFS, samba-technical
Reviewed the patch in for-next.
Looks good to me.
Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
Regards,
Shyam
On Tue, Feb 2, 2021 at 8:23 PM Steve French <smfrench@gmail.com> wrote:
>
> updated as suggested (see attached) and merged into cifs-2.6.git for-next
>
> On Tue, Feb 2, 2021 at 11:58 AM Pavel Shilovsky <piastryyy@gmail.com> wrote:
> >
> > пн, 1 февр. 2021 г. в 22:15, Steve French <smfrench@gmail.com>:
> > >
> > > SMB3.1.1 is the newest, and preferred dialect, and is included in
> > > the requested dialect list by default (ie if no vers= is specified
> > > on mount) but it should also be requested if SMB3 or later is requested
> > > (vers=3 instead of a specific dialect: vers=2.1, vers=3.02 or vers=3.0).
> > >
> > > Currently specifying "vers=3" only requests smb3.0 and smb3.02 but this
> > > patch fixes it to also request smb3.1.1 dialect, as it is the newest
> > > and most secure dialect and is a "version 3 or later" dialect (the intent
> > > of "vers=3").
> > >
> > > Signed-off-by: Steve French <stfrench@microsoft.com>
> > > Suggested-by: Pavel Shilovsky <pshilov@microsoft.com>
> > > ---
> > > fs/cifs/fs_context.c | 2 +-
> > > fs/cifs/smb2pdu.c | 19 +++++++++++++------
> > > 2 files changed, 14 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
> > > index 5111aadfdb6b..479c24695281 100644
> > > --- a/fs/cifs/fs_context.c
> > > +++ b/fs/cifs/fs_context.c
> > > @@ -391,7 +391,7 @@ cifs_parse_smb_version(char *value, struct
> > > smb3_fs_context *ctx, bool is_smb3)
> > > ctx->vals = &smb3any_values;
> > > break;
> > > case Smb_default:
> > > - ctx->ops = &smb30_operations; /* currently identical with 3.0 */
> > > + ctx->ops = &smb30_operations;
> > > ctx->vals = &smbdefault_values;
> > > break;
> > > default:
> > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
> > > index 794fc3b68b4f..52625549c3b5 100644
> > > --- a/fs/cifs/smb2pdu.c
> > > +++ b/fs/cifs/smb2pdu.c
> > > @@ -814,8 +814,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> > > SMB3ANY_VERSION_STRING) == 0) {
> > > req->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
> > > req->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
> > > - req->DialectCount = cpu_to_le16(2);
> > > - total_len += 4;
> > > + req->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
> > > + req->DialectCount = cpu_to_le16(3);
> > > + total_len += 6;
> > > } else if (strcmp(server->vals->version_string,
> > > SMBDEFAULT_VERSION_STRING) == 0) {
> > > req->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
> > > @@ -848,6 +849,8 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> > > memcpy(req->ClientGUID, server->client_guid,
> > > SMB2_CLIENT_GUID_SIZE);
> > > if ((server->vals->protocol_id == SMB311_PROT_ID) ||
> > > + (strcmp(server->vals->version_string,
> > > + SMB3ANY_VERSION_STRING) == 0) ||
> > > (strcmp(server->vals->version_string,
> > > SMBDEFAULT_VERSION_STRING) == 0))
> > > assemble_neg_contexts(req, server, &total_len);
> > > @@ -883,6 +886,9 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
> > > cifs_server_dbg(VFS,
> > > "SMB2.1 dialect returned but not requested\n");
> > > return -EIO;
> > > + } else if (rsp->DialectRevision == cpu_to_le16(SMB311_PROT_ID)) {
> >
> > I think we should include comment "/* ops set to 3.0 by default for
> > default so update */" as in smbdefault case to improve readability.
> >
> > > + server->ops = &smb311_operations;
> > > + server->vals = &smb311_values;
> > > }
> > > } else if (strcmp(server->vals->version_string,
> > > SMBDEFAULT_VERSION_STRING) == 0) {
> > > @@ -1042,10 +1048,11 @@ int smb3_validate_negotiate(const unsigned int
> > > xid, struct cifs_tcon *tcon)
> > > SMB3ANY_VERSION_STRING) == 0) {
> > > pneg_inbuf->Dialects[0] = cpu_to_le16(SMB30_PROT_ID);
> > > pneg_inbuf->Dialects[1] = cpu_to_le16(SMB302_PROT_ID);
> > > - pneg_inbuf->DialectCount = cpu_to_le16(2);
> > > - /* structure is big enough for 3 dialects, sending only 2 */
> > > + pneg_inbuf->Dialects[2] = cpu_to_le16(SMB311_PROT_ID);
> > > + pneg_inbuf->DialectCount = cpu_to_le16(3);
> > > + /* SMB 2.1 not included so subtract one dialect from len */
> > > inbuflen = sizeof(*pneg_inbuf) -
> > > - (2 * sizeof(pneg_inbuf->Dialects[0]));
> > > + (sizeof(pneg_inbuf->Dialects[0]));
> > > } else if (strcmp(server->vals->version_string,
> > > SMBDEFAULT_VERSION_STRING) == 0) {
> > > pneg_inbuf->Dialects[0] = cpu_to_le16(SMB21_PROT_ID);
> > > @@ -1053,7 +1060,7 @@ int smb3_validate_negotiate(const unsigned int
> > > xid, struct cifs_tcon *tcon)
> > > pneg_inbuf->Dialects[2] = cpu_to_le16(SMB302_PROT_ID);
> > > pneg_inbuf->Dialects[3] = cpu_to_le16(SMB311_PROT_ID);
> > > pneg_inbuf->DialectCount = cpu_to_le16(4);
> > > - /* structure is big enough for 3 dialects */
> > > + /* structure is big enough for 4 dialects */
> > > inbuflen = sizeof(*pneg_inbuf);
> > > } else {
> > > /* otherwise specific dialect was requested */
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
> >
> > Looks good overall.
> >
> > Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
> > --
> > Best regards,
> > Pavel Shilovsky
>
>
>
> --
> Thanks,
>
> Steve
--
Regards,
Shyam
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-03 11:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-02 6:15 PATCH] smb3: include current dialect (SMB3.1.1) when version 3 or greater requested on mount Steve French
2021-02-02 17:58 ` Pavel Shilovsky
2021-02-03 4:05 ` Steve French
2021-02-03 11:22 ` Shyam Prasad N
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.