Hi Henrique,
On 03/10, Henrique Carvalho wrote:
Fix a bug in match_session() that can result in duplicate sessions being
created even when the session data is identical.
match_session() compares ctx->sectype against ses->sectype only. This is
flawed because ses->sectype could be Unspecified while ctx->sectype
could be the same selected security type for the compared session. This
causes the function to mismatch the potential same session, resulting in
two of the same sessions.
Reproduction steps:
mount.cifs //server/share /mnt/a -o credentials=creds
mount.cifs //server/share /mnt/b -o credentials=creds,sec=ntlmssp
cat /proc/fs/cifs/DebugData | grep SessionId | wc -l # output is 1
mount.cifs //server/share /mnt/b -o credentials=creds,sec=ntlmssp
mount.cifs //server/share /mnt/a -o credentials=creds
cat /proc/fs/cifs/DebugData | grep SessionId | wc -l # output is 2
Minor nit: I think your reproducer results are inverted -- on my tests,
the session is reused when sec=ntlmssp is passed on first mount, not the
other way around.
Fixes: 3f618223dc0bd ("move sectype to the cifs_ses instead of
TCP_Server_Info")
Signed-off-by: Henrique Carvalho <henrique.carvalho@xxxxxxxx>
---
fs/smb/client/connect.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index f917de020dd5..0c8c523d52be 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -1825,8 +1825,11 @@ static int match_session(struct cifs_ses *ses,
struct smb3_fs_context *ctx,
bool match_super)
{
+ struct TCP_Server_Info *server = ses->server;
+ enum securityEnum selected_sectype = server->ops->select_sectype(ses->server, ctx->sectype);
Calling select_sectype() with ctx->sectype as argument works fine for
Unspecified and ntlmssp* cases (because Unspecified will default to
ntlmssp if server supports it), but if you do the first mount with
sec=krb5 and the second with sec=ntlmssp/no sec=, the krb5 session
will be reused (which is wrong).
I would suggest something like (quickly tested only):
{
- if (ctx->sectype != Unspecified &&
- ctx->sectype != ses->sectype)
+ struct TCP_Server_Info *server = ses->server;
+ enum securityEnum requested_sectype = server->ops->select_sectype(server, ctx->sectype),
+ selected_sectype = server->ops->select_sectype(server, ses->sectype);
+
+ if (requested_sectype != selected_sectype)
return 0;
i.e. comparing what the new session would negotiate as sectype vs what
was negotiated for the previous session.
+
if (ctx->sectype != Unspecified &&
- ctx->sectype != ses->sectype)
+ ctx->sectype != selected_sectype)
return 0;
if (!match_super && ctx->dfs_root_ses != ses->dfs_root_ses)
--
Other than that,
Reviewed-by: Enzo Matsumiya <ematsumiya@xxxxxxx>
Cheers,
Enzo
