[PATCH][CIFS] make locking consistent around the server session status

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



There were three places where we were not taking the spinlock
around updates to server->tcpStatus when it was being modified.
To be consistent (also removes Coverity warning) and to remove
possibility of race best to lock all places where it is updated.

Addresses-Coverity: 1399512 ("Data race condition")
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
 fs/cifs/cifsglob.h  | 3 ++-
 fs/cifs/connect.c   | 4 ++++
 fs/cifs/transport.c | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 3100f8b66e60..921680fb7931 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -577,6 +577,7 @@ struct TCP_Server_Info {
  char server_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL];
  struct smb_version_operations *ops;
  struct smb_version_values *vals;
+ /* updates to tcpStatus protected by GlobalMid_Lock */
  enum statusEnum tcpStatus; /* what we think the status is */
  char *hostname; /* hostname portion of UNC string */
  struct socket *ssocket;
@@ -1785,7 +1786,7 @@ require use of the stronger protocol */
  * list operations on pending_mid_q and oplockQ
  *      updates to XID counters, multiplex id  and SMB sequence numbers
  *      list operations on global DnotifyReqList
- *      updates to ses->status
+ *      updates to ses->status and TCP_Server_Info->tcpStatus
  *      updates to server->CurrentMid
  *  tcp_ses_lock protects:
  * list operations on tcp and SMB session lists
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 5d269f583dac..944fb92f50c7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1358,7 +1358,9 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx)
  * to the struct since the kernel thread not created yet
  * no need to spinlock this init of tcpStatus or srv_count
  */
+ spin_lock(&GlobalMid_Lock);
  tcp_ses->tcpStatus = CifsNew;
+ spin_unlock(&GlobalMid_Lock);
  ++tcp_ses->srv_count;

  if (ctx->echo_interval >= SMB_ECHO_INTERVAL_MIN &&
@@ -1403,7 +1405,9 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx)
  goto out_err_crypto_release;
  }
  tcp_ses->min_offload = ctx->min_offload;
+ spin_lock(&GlobalMid_Lock);
  tcp_ses->tcpStatus = CifsNeedNegotiate;
+ spin_unlock(&GlobalMid_Lock);

  if ((ctx->max_credits < 20) || (ctx->max_credits > 60000))
  tcp_ses->max_credits = SMB2_MAX_CREDITS_AVAILABLE;
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index f65f9a692ca2..75a95de320cf 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -431,7 +431,9 @@ __smb_send_rqst(struct TCP_Server_Info *server,
int num_rqst,
  * be taken as the remainder of this one. We need to kill the
  * socket so the server throws away the partial SMB
  */
+ spin_lock(&GlobalMid_Lock);
  server->tcpStatus = CifsNeedReconnect;
+ spin_unlock(&GlobalMid_Lock);
  trace_smb3_partial_send_reconnect(server->CurrentMid,
    server->conn_id, server->hostname);
  }

-- 
Thanks,

Steve
From dc43ee1df9b95c4d9e17285926d64c4600340f1e Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@xxxxxxxxxxxxx>
Date: Thu, 1 Jul 2021 12:22:47 -0500
Subject: [PATCH] cifs: make locking consistent around the server session
 status

There were three places where we were not taking the spinlock
around updates to server->tcpStatus when it was being modified.
To be consistent (also removes Coverity warning) and to remove
possibility of race best to lock all places where it is updated.

Addresses-Coverity: 1399512 ("Data race condition")
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
---
 fs/cifs/cifsglob.h  | 3 ++-
 fs/cifs/connect.c   | 4 ++++
 fs/cifs/transport.c | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 3100f8b66e60..921680fb7931 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -577,6 +577,7 @@ struct TCP_Server_Info {
 	char server_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL];
 	struct smb_version_operations	*ops;
 	struct smb_version_values	*vals;
+	/* updates to tcpStatus protected by GlobalMid_Lock */
 	enum statusEnum tcpStatus; /* what we think the status is */
 	char *hostname; /* hostname portion of UNC string */
 	struct socket *ssocket;
@@ -1785,7 +1786,7 @@ require use of the stronger protocol */
  *	list operations on pending_mid_q and oplockQ
  *      updates to XID counters, multiplex id  and SMB sequence numbers
  *      list operations on global DnotifyReqList
- *      updates to ses->status
+ *      updates to ses->status and TCP_Server_Info->tcpStatus
  *      updates to server->CurrentMid
  *  tcp_ses_lock protects:
  *	list operations on tcp and SMB session lists
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 5d269f583dac..944fb92f50c7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -1358,7 +1358,9 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx)
 	 * to the struct since the kernel thread not created yet
 	 * no need to spinlock this init of tcpStatus or srv_count
 	 */
+	spin_lock(&GlobalMid_Lock);
 	tcp_ses->tcpStatus = CifsNew;
+	spin_unlock(&GlobalMid_Lock);
 	++tcp_ses->srv_count;
 
 	if (ctx->echo_interval >= SMB_ECHO_INTERVAL_MIN &&
@@ -1403,7 +1405,9 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx)
 		goto out_err_crypto_release;
 	}
 	tcp_ses->min_offload = ctx->min_offload;
+	spin_lock(&GlobalMid_Lock);
 	tcp_ses->tcpStatus = CifsNeedNegotiate;
+	spin_unlock(&GlobalMid_Lock);
 
 	if ((ctx->max_credits < 20) || (ctx->max_credits > 60000))
 		tcp_ses->max_credits = SMB2_MAX_CREDITS_AVAILABLE;
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index f65f9a692ca2..75a95de320cf 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -431,7 +431,9 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
 		 * be taken as the remainder of this one. We need to kill the
 		 * socket so the server throws away the partial SMB
 		 */
+		spin_lock(&GlobalMid_Lock);
 		server->tcpStatus = CifsNeedReconnect;
+		spin_unlock(&GlobalMid_Lock);
 		trace_smb3_partial_send_reconnect(server->CurrentMid,
 						  server->conn_id, server->hostname);
 	}
-- 
2.30.2


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux