Discussion:
STATUS_USER_SESSION_DELETED: auth problems when downgrading from vers=3.0 to vers=2.0/2.1
Sachin Prabhu
2014-05-08 10:51:06 UTC
Permalink
I just finished installing a copy of Windows 2012 and was testing
version 3 when I came across this particular problem.

To reproduce
1) First mount a share using vers=3.0
# mount -t cifs -o username=user1,password=pass1,vers=3.0 //vm140-54/exports /mnt
This is successful.
2) Now unmount this share and attempt to mount with vers=2.0 to 2.1
# mount -t cifs -o username=user1,password=pass1,vers=2.1 //vm140-54/exports /mnt
mount error(5): Input/output error
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs)

/var/log/messages shows
kernel: [ 2730.843939] Status code returned 0xc0000203 STATUS_USER_SESSION_DELETED
kernel: [ 2730.846883] CIFS VFS: Send error in SessSetup = -5
kernel: [ 2730.849247] CIFS VFS: cifs_mount failed w/return code = -5

I can then mount using vers=3.0 and vers=1.0 but not 2.0 or 2.1.

I am attempting to debug this but was wondering if anyone had
encountered this before.

Sachin Prabhu
Sachin Prabhu
2014-05-12 23:48:12 UTC
Permalink
When mounting from a Windows 2012R2 server, we hit the following
problem:
1) Mount with any of the following versions - 2.0, 2.1 or 3.0
2) unmount
3) Attempt a mount again using a different SMB version >= 2.0.

You end up with the following failure:
Status code returned 0xc0000203 STATUS_USER_SESSION_DELETED
CIFS VFS: Send error in SessSetup = -5
CIFS VFS: cifs_mount failed w/return code = -5

I cannot reproduce this issue using a Windows 2008 R2 server.

This appears to be caused because we use the same client guid for the
connection on first mount which we then disconnect and attempt to mount
again using a different protocol version. By generating a new guid each
time a new connection is Negotiated, we avoid hitting this problem.

Signed-off-by: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
---
fs/cifs/cifsfs.c | 8 --------
fs/cifs/cifsglob.h | 1 +
fs/cifs/connect.c | 3 +++
fs/cifs/smb2pdu.c | 5 +++--
fs/cifs/smb2pdu.h | 2 --
5 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 5be1f99..c80aa5a 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -87,10 +87,6 @@ extern mempool_t *cifs_mid_poolp;

struct workqueue_struct *cifsiod_wq;

-#ifdef CONFIG_CIFS_SMB2
-__u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-#endif
-
/*
* Bumps refcount for cifs super block.
* Note that it should be only called if a referece to VFS super block is
@@ -1192,10 +1188,6 @@ init_cifs(void)
spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock);

-#ifdef CONFIG_CIFS_SMB2
- get_random_bytes(cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
-#endif
-
if (cifs_max_pending < 2) {
cifs_max_pending = 2;
cifs_dbg(FYI, "cifs_max_pending set to min of 2\n");
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 30f6e92..f74edd2 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -559,6 +559,7 @@ struct TCP_Server_Info {
int echo_credits; /* echo reserved slots */
int oplock_credits; /* oplock break reserved slots */
bool echoes:1; /* enable echoes */
+ __u8 client_guid[SMB2_CLIENT_GUID_SIZE]; /* Client GUID */
#endif
u16 dialect; /* dialect index that server chose */
bool oplocks:1; /* enable oplocks */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8813ff7..8b8fe9b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2144,6 +2144,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
sizeof(tcp_ses->srcaddr));
memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr,
sizeof(tcp_ses->dstaddr));
+#ifdef CONFIG_CIFS_SMB2
+ get_random_bytes(tcp_ses->client_guid, SMB2_CLIENT_GUID_SIZE);
+#endif
/*
* at this point we are the only ones with the pointer
* to the struct since the kernel thread not created yet
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 3802f8c..dc44610 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -375,7 +375,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)

req->Capabilities = cpu_to_le32(ses->server->vals->req_capabilities);

- memcpy(req->ClientGUID, cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
+ memcpy(req->ClientGUID, server->client_guid, SMB2_CLIENT_GUID_SIZE);

iov[0].iov_base = (char *)req;
/* 4 for rfc1002 length field */
@@ -478,7 +478,8 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)

vneg_inbuf.Capabilities =
cpu_to_le32(tcon->ses->server->vals->req_capabilities);
- memcpy(vneg_inbuf.Guid, cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
+ memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);

if (tcon->ses->sign)
vneg_inbuf.SecurityMode =
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 2022c54..743e11e 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -183,8 +183,6 @@ struct smb2_symlink_err_rsp {

#define SMB2_CLIENT_GUID_SIZE 16

-extern __u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-
struct smb2_negotiate_req {
struct smb2_hdr hdr;
__le16 StructureSize; /* Must be 36 */
--
1.8.4.2
Steve French
2014-05-13 20:49:17 UTC
Permalink
merged into cifs-2.6.git for-next

but added an additional patch to fix the problem where we are sending
a non-zero ClientGUID for SMB2.02 dialect (where it MUST be zero
according to MS-SMB2, it is non-zero starting in SMB2.1)
Post by Sachin Prabhu
When mounting from a Windows 2012R2 server, we hit the following
1) Mount with any of the following versions - 2.0, 2.1 or 3.0
2) unmount
3) Attempt a mount again using a different SMB version >= 2.0.
Status code returned 0xc0000203 STATUS_USER_SESSION_DELETED
CIFS VFS: Send error in SessSetup = -5
CIFS VFS: cifs_mount failed w/return code = -5
I cannot reproduce this issue using a Windows 2008 R2 server.
This appears to be caused because we use the same client guid for the
connection on first mount which we then disconnect and attempt to mount
again using a different protocol version. By generating a new guid each
time a new connection is Negotiated, we avoid hitting this problem.
---
fs/cifs/cifsfs.c | 8 --------
fs/cifs/cifsglob.h | 1 +
fs/cifs/connect.c | 3 +++
fs/cifs/smb2pdu.c | 5 +++--
fs/cifs/smb2pdu.h | 2 --
5 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 5be1f99..c80aa5a 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -87,10 +87,6 @@ extern mempool_t *cifs_mid_poolp;
struct workqueue_struct *cifsiod_wq;
-#ifdef CONFIG_CIFS_SMB2
-__u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-#endif
-
/*
* Bumps refcount for cifs super block.
* Note that it should be only called if a referece to VFS super block is
@@ -1192,10 +1188,6 @@ init_cifs(void)
spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock);
-#ifdef CONFIG_CIFS_SMB2
- get_random_bytes(cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
-#endif
-
if (cifs_max_pending < 2) {
cifs_max_pending = 2;
cifs_dbg(FYI, "cifs_max_pending set to min of 2\n");
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 30f6e92..f74edd2 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -559,6 +559,7 @@ struct TCP_Server_Info {
int echo_credits; /* echo reserved slots */
int oplock_credits; /* oplock break reserved slots */
bool echoes:1; /* enable echoes */
+ __u8 client_guid[SMB2_CLIENT_GUID_SIZE]; /* Client GUID */
#endif
u16 dialect; /* dialect index that server chose */
bool oplocks:1; /* enable oplocks */
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 8813ff7..8b8fe9b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2144,6 +2144,9 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
sizeof(tcp_ses->srcaddr));
memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr,
sizeof(tcp_ses->dstaddr));
+#ifdef CONFIG_CIFS_SMB2
+ get_random_bytes(tcp_ses->client_guid, SMB2_CLIENT_GUID_SIZE);
+#endif
/*
* at this point we are the only ones with the pointer
* to the struct since the kernel thread not created yet
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c
index 3802f8c..dc44610 100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -375,7 +375,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
req->Capabilities = cpu_to_le32(ses->server->vals->req_capabilities);
- memcpy(req->ClientGUID, cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
+ memcpy(req->ClientGUID, server->client_guid, SMB2_CLIENT_GUID_SIZE);
iov[0].iov_base = (char *)req;
/* 4 for rfc1002 length field */
@@ -478,7 +478,8 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon)
vneg_inbuf.Capabilities =
cpu_to_le32(tcon->ses->server->vals->req_capabilities);
- memcpy(vneg_inbuf.Guid, cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
+ memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
if (tcon->ses->sign)
vneg_inbuf.SecurityMode =
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index 2022c54..743e11e 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -183,8 +183,6 @@ struct smb2_symlink_err_rsp {
#define SMB2_CLIENT_GUID_SIZE 16
-extern __u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-
struct smb2_negotiate_req {
struct smb2_hdr hdr;
__le16 StructureSize; /* Must be 36 */
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Thanks,

Steve
Tom Talpey
2014-05-15 06:36:14 UTC
Permalink
Won't this need to be undone for multichannel support? The client GUID needs to be the same when connecting multiple channels, a different one will break cross-channel replay detection.

It's definitely true that if you intend to negotiate a different dialect, then the client GUID should not be reused.
-----Original Message-----
Sent: Tuesday, May 13, 2014 10:49 PM
To: Sachin Prabhu
Cc: linux-cifs
Subject: Re: [PATCH] cifs: Set client guid on per connection basis
merged into cifs-2.6.git for-next
but added an additional patch to fix the problem where we are sending a
non-zero ClientGUID for SMB2.02 dialect (where it MUST be zero according
to MS-SMB2, it is non-zero starting in SMB2.1)
Post by Sachin Prabhu
When mounting from a Windows 2012R2 server, we hit the following
1) Mount with any of the following versions - 2.0, 2.1 or 3.0
2) unmount
3) Attempt a mount again using a different SMB version >= 2.0.
Status code returned 0xc0000203 STATUS_USER_SESSION_DELETED CIFS
Send error in SessSetup = -5 CIFS VFS: cifs_mount failed w/return code
= -5
I cannot reproduce this issue using a Windows 2008 R2 server.
This appears to be caused because we use the same client guid for the
connection on first mount which we then disconnect and attempt to
mount again using a different protocol version. By generating a new
guid each time a new connection is Negotiated, we avoid hitting this
problem.
Post by Sachin Prabhu
---
fs/cifs/cifsfs.c | 8 --------
fs/cifs/cifsglob.h | 1 +
fs/cifs/connect.c | 3 +++
fs/cifs/smb2pdu.c | 5 +++--
fs/cifs/smb2pdu.h | 2 --
5 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index
5be1f99..c80aa5a 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -87,10 +87,6 @@ extern mempool_t *cifs_mid_poolp;
struct workqueue_struct *cifsiod_wq;
-#ifdef CONFIG_CIFS_SMB2
-__u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-#endif
-
/*
* Bumps refcount for cifs super block.
* Note that it should be only called if a referece to VFS super
spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock);
-#ifdef CONFIG_CIFS_SMB2
- get_random_bytes(cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
-#endif
-
if (cifs_max_pending < 2) {
cifs_max_pending = 2;
cifs_dbg(FYI, "cifs_max_pending set to min of 2\n");
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index
30f6e92..f74edd2 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -559,6 +559,7 @@ struct TCP_Server_Info {
int echo_credits; /* echo reserved slots */
int oplock_credits; /* oplock break reserved slots */
bool echoes:1; /* enable echoes */
+ __u8 client_guid[SMB2_CLIENT_GUID_SIZE]; /* Client GUID */
#endif
u16 dialect; /* dialect index that server chose */
bool oplocks:1; /* enable oplocks */ diff --git
a/fs/cifs/connect.c b/fs/cifs/connect.c index 8813ff7..8b8fe9b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2144,6 +2144,9 @@ cifs_get_tcp_session(struct smb_vol
*volume_info)
Post by Sachin Prabhu
sizeof(tcp_ses->srcaddr));
memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr,
sizeof(tcp_ses->dstaddr));
+#ifdef CONFIG_CIFS_SMB2
+ get_random_bytes(tcp_ses->client_guid, SMB2_CLIENT_GUID_SIZE);
+#endif
/*
* at this point we are the only ones with the pointer
* to the struct since the kernel thread not created yet diff
--git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 3802f8c..dc44610
100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -375,7 +375,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
req->Capabilities =
cpu_to_le32(ses->server->vals->req_capabilities);
- memcpy(req->ClientGUID, cifs_client_guid,
SMB2_CLIENT_GUID_SIZE);
Post by Sachin Prabhu
+ memcpy(req->ClientGUID, server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
iov[0].iov_base = (char *)req;
smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon
*tcon)
vneg_inbuf.Capabilities =
cpu_to_le32(tcon->ses->server->vals->req_capabilities);
- memcpy(vneg_inbuf.Guid, cifs_client_guid,
SMB2_CLIENT_GUID_SIZE);
Post by Sachin Prabhu
+ memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
if (tcon->ses->sign)
vneg_inbuf.SecurityMode = diff --git
a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h index 2022c54..743e11e 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -183,8 +183,6 @@ struct smb2_symlink_err_rsp {
#define SMB2_CLIENT_GUID_SIZE 16
-extern __u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-
struct smb2_negotiate_req {
struct smb2_hdr hdr;
__le16 StructureSize; /* Must be 36 */
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs"
majordomo
Post by Sachin Prabhu
info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the
http://vger.kernel.org/maj
Steve French
2014-05-15 08:12:12 UTC
Permalink
Would multichannel require a new negotiate protocol? We are only
generating it in get_tcp_session - and would probably not go through
that path when connecting another socket to the original parent
struct. Currently the model where tcons hang off smb sessions which
hang off tcp sessions looks strange when you consider multiple tcp
connections but have to distinguish the code which connects the first
tcp session for a machine and sets up the parent structure that all
the others (tcons and smb sessions) hang off, from the code which
connects subsequent tcp sessions for multichannel
Post by Tom Talpey
Won't this need to be undone for multichannel support? The client GUID needs to be the same when connecting multiple channels, a different one will break cross-channel replay detection.
It's definitely true that if you intend to negotiate a different dialect, then the client GUID should not be reused.
-----Original Message-----
Sent: Tuesday, May 13, 2014 10:49 PM
To: Sachin Prabhu
Cc: linux-cifs
Subject: Re: [PATCH] cifs: Set client guid on per connection basis
merged into cifs-2.6.git for-next
but added an additional patch to fix the problem where we are sending a
non-zero ClientGUID for SMB2.02 dialect (where it MUST be zero according
to MS-SMB2, it is non-zero starting in SMB2.1)
Post by Sachin Prabhu
When mounting from a Windows 2012R2 server, we hit the following
1) Mount with any of the following versions - 2.0, 2.1 or 3.0
2) unmount
3) Attempt a mount again using a different SMB version >= 2.0.
Status code returned 0xc0000203 STATUS_USER_SESSION_DELETED CIFS
Send error in SessSetup = -5 CIFS VFS: cifs_mount failed w/return code
= -5
I cannot reproduce this issue using a Windows 2008 R2 server.
This appears to be caused because we use the same client guid for the
connection on first mount which we then disconnect and attempt to
mount again using a different protocol version. By generating a new
guid each time a new connection is Negotiated, we avoid hitting this
problem.
Post by Sachin Prabhu
---
fs/cifs/cifsfs.c | 8 --------
fs/cifs/cifsglob.h | 1 +
fs/cifs/connect.c | 3 +++
fs/cifs/smb2pdu.c | 5 +++--
fs/cifs/smb2pdu.h | 2 --
5 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index
5be1f99..c80aa5a 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -87,10 +87,6 @@ extern mempool_t *cifs_mid_poolp;
struct workqueue_struct *cifsiod_wq;
-#ifdef CONFIG_CIFS_SMB2
-__u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-#endif
-
/*
* Bumps refcount for cifs super block.
* Note that it should be only called if a referece to VFS super
spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock);
-#ifdef CONFIG_CIFS_SMB2
- get_random_bytes(cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
-#endif
-
if (cifs_max_pending < 2) {
cifs_max_pending = 2;
cifs_dbg(FYI, "cifs_max_pending set to min of 2\n");
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index
30f6e92..f74edd2 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -559,6 +559,7 @@ struct TCP_Server_Info {
int echo_credits; /* echo reserved slots */
int oplock_credits; /* oplock break reserved slots */
bool echoes:1; /* enable echoes */
+ __u8 client_guid[SMB2_CLIENT_GUID_SIZE]; /* Client GUID */
#endif
u16 dialect; /* dialect index that server chose */
bool oplocks:1; /* enable oplocks */ diff --git
a/fs/cifs/connect.c b/fs/cifs/connect.c index 8813ff7..8b8fe9b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2144,6 +2144,9 @@ cifs_get_tcp_session(struct smb_vol
*volume_info)
Post by Sachin Prabhu
sizeof(tcp_ses->srcaddr));
memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr,
sizeof(tcp_ses->dstaddr));
+#ifdef CONFIG_CIFS_SMB2
+ get_random_bytes(tcp_ses->client_guid, SMB2_CLIENT_GUID_SIZE);
+#endif
/*
* at this point we are the only ones with the pointer
* to the struct since the kernel thread not created yet diff
--git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 3802f8c..dc44610
100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -375,7 +375,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
req->Capabilities =
cpu_to_le32(ses->server->vals->req_capabilities);
- memcpy(req->ClientGUID, cifs_client_guid,
SMB2_CLIENT_GUID_SIZE);
Post by Sachin Prabhu
+ memcpy(req->ClientGUID, server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
iov[0].iov_base = (char *)req;
smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon
*tcon)
vneg_inbuf.Capabilities =
cpu_to_le32(tcon->ses->server->vals->req_capabilities);
- memcpy(vneg_inbuf.Guid, cifs_client_guid,
SMB2_CLIENT_GUID_SIZE);
Post by Sachin Prabhu
+ memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
if (tcon->ses->sign)
vneg_inbuf.SecurityMode = diff --git
a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h index 2022c54..743e11e 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -183,8 +183,6 @@ struct smb2_symlink_err_rsp {
#define SMB2_CLIENT_GUID_SIZE 16
-extern __u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-
struct smb2_negotiate_req {
struct smb2_hdr hdr;
__le16 StructureSize; /* Must be 36 */
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs"
majordomo
Post by Sachin Prabhu
info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the
http://vger.kernel.org/majordomo-info.html
--
Thanks,

Steve
Tom Talpey
2014-05-15 08:35:03 UTC
Permalink
Post by Steve French
Would multichannel require a new negotiate protocol?
A new connection always requires a negotiate, multichannel is no exception. What's different is that to make a multichannel association, the client should provide the original ClientGUID, and establish the same dialect and capabilities as the original connection. The session bind needs these in order join appropriately on the new multichannel.

The ClientGUID is mostly for supporting leases, and if you swizzle it per-connection then clients will start breaking their own leases when using multichannel. There is also some enforcement at session setup/bind time, as documented in MS-SMB2 3.3.5.3.3:

"The server MUST look up all existing connections from the client in the global ConnectionList where Connection.ClientGuid matches Session.Connection.ClientGuid. For any matching Connection, if Connection.Dialect is not the same as Session.Connection.Dialect, the server SHOULD<228> close the newly created Session, as specified in section 3.3.4.12, by providing Session.SessionGlobalId as the input parameter, and fail the session setup request with STATUS_USER_SESSION_DELETED."


I guess if you only call get_tcp_session once, then you might be ok (that's your call). I was just pointing out that a new ClientGUID per-*connection* will get in the way for you later, because multichannel reuses sessions on many connections. Normally the protocol specifies a single ClientGUID which is used for all connections from a given client.
Post by Steve French
-----Original Message-----
Sent: Thursday, May 15, 2014 10:12 AM
To: Tom Talpey
Cc: Sachin Prabhu; linux-cifs
Subject: Re: [PATCH] cifs: Set client guid on per connection basis
Would multichannel require a new negotiate protocol? We are only
generating it in get_tcp_session - and would probably not go through that
path when connecting another socket to the original parent
struct. Currently the model where tcons hang off smb sessions which
hang off tcp sessions looks strange when you consider multiple tcp
connections but have to distinguish the code which connects the first tcp
session for a machine and sets up the parent structure that all the others
(tcons and smb sessions) hang off, from the code which connects
subsequent tcp sessions for multichannel
Post by Tom Talpey
Won't this need to be undone for multichannel support? The client GUID
needs to be the same when connecting multiple channels, a different one
will break cross-channel replay detection.
Post by Tom Talpey
It's definitely true that if you intend to negotiate a different dialect, then
the client GUID should not be reused.
Post by Tom Talpey
-----Original Message-----
Sent: Tuesday, May 13, 2014 10:49 PM
To: Sachin Prabhu
Cc: linux-cifs
Subject: Re: [PATCH] cifs: Set client guid on per connection basis
merged into cifs-2.6.git for-next
but added an additional patch to fix the problem where we are sending
a non-zero ClientGUID for SMB2.02 dialect (where it MUST be zero
according to MS-SMB2, it is non-zero starting in SMB2.1)
Post by Sachin Prabhu
When mounting from a Windows 2012R2 server, we hit the following
1) Mount with any of the following versions - 2.0, 2.1 or 3.0
2) unmount
3) Attempt a mount again using a different SMB version >= 2.0.
Status code returned 0xc0000203 STATUS_USER_SESSION_DELETED CIFS
Send error in SessSetup = -5 CIFS VFS: cifs_mount failed w/return
code = -5
I cannot reproduce this issue using a Windows 2008 R2 server.
This appears to be caused because we use the same client guid for
the connection on first mount which we then disconnect and attempt
to mount again using a different protocol version. By generating a
new guid each time a new connection is Negotiated, we avoid hitting
this
problem.
Post by Sachin Prabhu
---
fs/cifs/cifsfs.c | 8 --------
fs/cifs/cifsglob.h | 1 +
fs/cifs/connect.c | 3 +++
fs/cifs/smb2pdu.c | 5 +++--
fs/cifs/smb2pdu.h | 2 --
5 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index
5be1f99..c80aa5a 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -87,10 +87,6 @@ extern mempool_t *cifs_mid_poolp;
struct workqueue_struct *cifsiod_wq;
-#ifdef CONFIG_CIFS_SMB2
-__u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-#endif
-
/*
* Bumps refcount for cifs super block.
* Note that it should be only called if a referece to VFS super
spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock);
-#ifdef CONFIG_CIFS_SMB2
- get_random_bytes(cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
-#endif
-
if (cifs_max_pending < 2) {
cifs_max_pending = 2;
cifs_dbg(FYI, "cifs_max_pending set to min of
2\n"); diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index
30f6e92..f74edd2 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -559,6 +559,7 @@ struct TCP_Server_Info {
int echo_credits; /* echo reserved slots */
int oplock_credits; /* oplock break reserved slots */
bool echoes:1; /* enable echoes */
+ __u8 client_guid[SMB2_CLIENT_GUID_SIZE]; /* Client GUID */
#endif
u16 dialect; /* dialect index that server chose */
bool oplocks:1; /* enable oplocks */ diff --git
a/fs/cifs/connect.c b/fs/cifs/connect.c index 8813ff7..8b8fe9b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2144,6 +2144,9 @@ cifs_get_tcp_session(struct smb_vol
*volume_info)
Post by Sachin Prabhu
sizeof(tcp_ses->srcaddr));
memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr,
sizeof(tcp_ses->dstaddr));
+#ifdef CONFIG_CIFS_SMB2
+ get_random_bytes(tcp_ses->client_guid,
+SMB2_CLIENT_GUID_SIZE); #endif
/*
* at this point we are the only ones with the pointer
* to the struct since the kernel thread not created yet
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index
3802f8c..dc44610
100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -375,7 +375,7 @@ SMB2_negotiate(const unsigned int xid, struct cifs_ses *ses)
req->Capabilities =
cpu_to_le32(ses->server->vals->req_capabilities);
- memcpy(req->ClientGUID, cifs_client_guid,
SMB2_CLIENT_GUID_SIZE);
Post by Sachin Prabhu
+ memcpy(req->ClientGUID, server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
iov[0].iov_base = (char *)req;
smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon
*tcon)
vneg_inbuf.Capabilities =
cpu_to_le32(tcon->ses->server->vals->req_capabilities);
- memcpy(vneg_inbuf.Guid, cifs_client_guid,
SMB2_CLIENT_GUID_SIZE);
Post by Sachin Prabhu
+ memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
if (tcon->ses->sign)
vneg_inbuf.SecurityMode = diff --git
a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h index 2022c54..743e11e 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -183,8 +183,6 @@ struct smb2_symlink_err_rsp {
#define SMB2_CLIENT_GUID_SIZE 16
-extern __u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-
struct smb2_negotiate_req {
struct smb2_hdr hdr;
__le16 StructureSize; /* Must be 36 */
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs"
majordomo
Post by Sachin Prabhu
info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs"
majordomo
Post by Tom Talpey
info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Tom Talpey
2014-05-15 11:46:33 UTC
Permalink
Thinking about this a bit more, it seems to me that if you do want to support an explicit dialect selection at mount time, then you may want to check whether you have an existing mount over a different dialect from the client before allowing it. The alternative may be to select a new ClientGUID for the new mount, to avoid the session create failure - but that will mean leasing won't work very well. Might be a tough choice, if you do want this dialect option.
-----Original Message-----
Sent: Thursday, May 15, 2014 10:35 AM
To: Steve French
Cc: Sachin Prabhu; linux-cifs
Subject: RE: [PATCH] cifs: Set client guid on per connection basis
Post by Steve French
Would multichannel require a new negotiate protocol?
A new connection always requires a negotiate, multichannel is no exception.
What's different is that to make a multichannel association, the client should
provide the original ClientGUID, and establish the same dialect and
capabilities as the original connection. The session bind needs these in order
join appropriately on the new multichannel.
The ClientGUID is mostly for supporting leases, and if you swizzle it per-
connection then clients will start breaking their own leases when using
multichannel. There is also some enforcement at session setup/bind time, as
"The server MUST look up all existing connections from the client in the
global ConnectionList where Connection.ClientGuid matches
Session.Connection.ClientGuid. For any matching Connection, if
Connection.Dialect is not the same as Session.Connection.Dialect, the server
SHOULD<228> close the newly created Session, as specified in section
3.3.4.12, by providing Session.SessionGlobalId as the input parameter, and
fail the session setup request with STATUS_USER_SESSION_DELETED."
I guess if you only call get_tcp_session once, then you might be ok (that's
your call). I was just pointing out that a new ClientGUID per-*connection*
will get in the way for you later, because multichannel reuses sessions on
many connections. Normally the protocol specifies a single ClientGUID which
is used for all connections from a given client.
Post by Steve French
-----Original Message-----
Sent: Thursday, May 15, 2014 10:12 AM
To: Tom Talpey
Cc: Sachin Prabhu; linux-cifs
Subject: Re: [PATCH] cifs: Set client guid on per connection basis
Would multichannel require a new negotiate protocol? We are only
generating it in get_tcp_session - and would probably not go through
that path when connecting another socket to the original parent
struct. Currently the model where tcons hang off smb sessions which
hang off tcp sessions looks strange when you consider multiple tcp
connections but have to distinguish the code which connects the first
tcp session for a machine and sets up the parent structure that all
the others (tcons and smb sessions) hang off, from the code which
connects subsequent tcp sessions for multichannel
Post by Tom Talpey
Won't this need to be undone for multichannel support? The client GUID
needs to be the same when connecting multiple channels, a different
one will break cross-channel replay detection.
Post by Tom Talpey
It's definitely true that if you intend to negotiate a different dialect, then
the client GUID should not be reused.
Post by Tom Talpey
-----Original Message-----
Sent: Tuesday, May 13, 2014 10:49 PM
To: Sachin Prabhu
Cc: linux-cifs
Subject: Re: [PATCH] cifs: Set client guid on per connection basis
merged into cifs-2.6.git for-next
but added an additional patch to fix the problem where we are
sending a non-zero ClientGUID for SMB2.02 dialect (where it MUST be
zero according to MS-SMB2, it is non-zero starting in SMB2.1)
On Tue, May 13, 2014 at 1:48 AM, Sachin Prabhu
Post by Sachin Prabhu
When mounting from a Windows 2012R2 server, we hit the following
1) Mount with any of the following versions - 2.0, 2.1 or 3.0
2) unmount
3) Attempt a mount again using a different SMB version >= 2.0.
Status code returned 0xc0000203 STATUS_USER_SESSION_DELETED
CIFS
Post by Steve French
Post by Tom Talpey
Post by Sachin Prabhu
Send error in SessSetup = -5 CIFS VFS: cifs_mount failed w/return
code = -5
I cannot reproduce this issue using a Windows 2008 R2 server.
This appears to be caused because we use the same client guid for
the connection on first mount which we then disconnect and
attempt to mount again using a different protocol version. By
generating a new guid each time a new connection is Negotiated,
we avoid hitting this
problem.
Post by Sachin Prabhu
---
fs/cifs/cifsfs.c | 8 --------
fs/cifs/cifsglob.h | 1 +
fs/cifs/connect.c | 3 +++
fs/cifs/smb2pdu.c | 5 +++--
fs/cifs/smb2pdu.h | 2 --
5 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index
5be1f99..c80aa5a 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -87,10 +87,6 @@ extern mempool_t *cifs_mid_poolp;
struct workqueue_struct *cifsiod_wq;
-#ifdef CONFIG_CIFS_SMB2
-__u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-#endif
-
/*
* Bumps refcount for cifs super block.
* Note that it should be only called if a referece to VFS super
spin_lock_init(&cifs_file_list_lock);
spin_lock_init(&GlobalMid_Lock);
-#ifdef CONFIG_CIFS_SMB2
- get_random_bytes(cifs_client_guid, SMB2_CLIENT_GUID_SIZE);
-#endif
-
if (cifs_max_pending < 2) {
cifs_max_pending = 2;
cifs_dbg(FYI, "cifs_max_pending set to min of
2\n"); diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index
30f6e92..f74edd2 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -559,6 +559,7 @@ struct TCP_Server_Info {
int echo_credits; /* echo reserved slots */
int oplock_credits; /* oplock break reserved slots */
bool echoes:1; /* enable echoes */
+ __u8 client_guid[SMB2_CLIENT_GUID_SIZE]; /* Client GUID
+ */
#endif
u16 dialect; /* dialect index that server chose */
bool oplocks:1; /* enable oplocks */ diff --git
a/fs/cifs/connect.c b/fs/cifs/connect.c index 8813ff7..8b8fe9b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2144,6 +2144,9 @@ cifs_get_tcp_session(struct smb_vol
*volume_info)
Post by Sachin Prabhu
sizeof(tcp_ses->srcaddr));
memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr,
sizeof(tcp_ses->dstaddr));
+#ifdef CONFIG_CIFS_SMB2
+ get_random_bytes(tcp_ses->client_guid,
+SMB2_CLIENT_GUID_SIZE); #endif
/*
* at this point we are the only ones with the pointer
* to the struct since the kernel thread not created yet
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index
3802f8c..dc44610
100644
--- a/fs/cifs/smb2pdu.c
+++ b/fs/cifs/smb2pdu.c
@@ -375,7 +375,7 @@ SMB2_negotiate(const unsigned int xid, struct
cifs_ses *ses)
req->Capabilities =
cpu_to_le32(ses->server->vals->req_capabilities);
- memcpy(req->ClientGUID, cifs_client_guid,
SMB2_CLIENT_GUID_SIZE);
Post by Sachin Prabhu
+ memcpy(req->ClientGUID, server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
iov[0].iov_base = (char *)req;
smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon
*tcon)
vneg_inbuf.Capabilities =
cpu_to_le32(tcon->ses->server->vals->req_capabilities);
- memcpy(vneg_inbuf.Guid, cifs_client_guid,
SMB2_CLIENT_GUID_SIZE);
Post by Sachin Prabhu
+ memcpy(vneg_inbuf.Guid, tcon->ses->server->client_guid,
+ SMB2_CLIENT_GUID_SIZE);
if (tcon->ses->sign)
vneg_inbuf.SecurityMode = diff --git
a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h index 2022c54..743e11e 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -183,8 +183,6 @@ struct smb2_symlink_err_rsp {
#define SMB2_CLIENT_GUID_SIZE 16
-extern __u8 cifs_client_guid[SMB2_CLIENT_GUID_SIZE];
-
struct smb2_negotiate_req {
struct smb2_hdr hdr;
__le16 StructureSize; /* Must be 36 */
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs"
majordomo
Post by Sachin Prabhu
info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs"
majordomo
Post by Tom Talpey
info at http://vger.kernel.org/majordomo-info.html
--
Thanks,
Steve
N r y b X ǧv ^ )޺{.n + { r' {ay ʇڙ ,j f h z  w j:+v
Loading...