CIFS: Do not reconnect TCP session in add_credits()

When executing add_credits() we currently call cifs_reconnect()
if the number of credits is zero and there are no requests in
flight. In this case we may call cifs_reconnect() recursively
twice and cause memory corruption given the following sequence
of functions:

mid1.callback() -> add_credits() -> cifs_reconnect() ->
-> mid2.callback() -> add_credits() -> cifs_reconnect().

Fix this by avoiding to call cifs_reconnect() in add_credits()
and checking for zero credits in the demultiplex thread.

Cc: <stable@vger.kernel.org>
Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 683310f261718d3d07088342fde4b89a27bac4bc..8463c940e0e59779390b210fa3ebba45df86f80b 100644 (file)
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -720,6 +720,21 @@ server_unresponsive(struct TCP_Server_Info *server)
        return false;
 }
 
+static inline bool
+zero_credits(struct TCP_Server_Info *server)
+{
+       int val;
+
+       spin_lock(&server->req_lock);
+       val = server->credits + server->echo_credits + server->oplock_credits;
+       if (server->in_flight == 0 && val == 0) {
+               spin_unlock(&server->req_lock);
+               return true;
+       }
+       spin_unlock(&server->req_lock);
+       return false;
+}
+
 static int
 cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg)
 {
@@ -732,6 +747,12 @@ cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg)
        for (total_read = 0; msg_data_left(smb_msg); total_read += length) {
                try_to_freeze();
 
+               /* reconnect if no credits and no requests in flight */
+               if (zero_credits(server)) {
+                       cifs_reconnect(server);
+                       return -ECONNABORTED;
+               }
+
                if (server_unresponsive(server))
                        return -ECONNABORTED;
                if (cifs_rdma_enabled(server) && server->smbd_conn)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 1d3ce127b02e7b19debb765af92773af9ab94156..fa8d2e1076c8a54a31cfd667bbab6f3695c9de90 100644 (file)
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -34,6 +34,7 @@
 #include "cifs_ioctl.h"
 #include "smbdirect.h"
 
+/* Change credits for different ops and return the total number of credits */
 static int
 change_conf(struct TCP_Server_Info *server)
 {
@@ -41,17 +42,15 @@ change_conf(struct TCP_Server_Info *server)
        server->oplock_credits = server->echo_credits = 0;
        switch (server->credits) {
        case 0:
-               return -1;
+               return 0;
        case 1:
                server->echoes = false;
                server->oplocks = false;
-               cifs_dbg(VFS, "disabling echoes and oplocks\n");
                break;
        case 2:
                server->echoes = true;
                server->oplocks = false;
                server->echo_credits = 1;
-               cifs_dbg(FYI, "disabling oplocks\n");
                break;
        default:
                server->echoes = true;
@@ -64,14 +63,15 @@ change_conf(struct TCP_Server_Info *server)
                server->echo_credits = 1;
        }
        server->credits -= server->echo_credits + server->oplock_credits;
-       return 0;
+       return server->credits + server->echo_credits + server->oplock_credits;
 }
 
 static void
 smb2_add_credits(struct TCP_Server_Info *server, const unsigned int add,
                 const int optype)
 {
-       int *val, rc = 0;
+       int *val, rc = -1;
+
        spin_lock(&server->req_lock);
        val = server->ops->get_credits_field(server, optype);
 
@@ -101,8 +101,26 @@ smb2_add_credits(struct TCP_Server_Info *server, const unsigned int add,
        }
        spin_unlock(&server->req_lock);
        wake_up(&server->request_q);
-       if (rc)
-               cifs_reconnect(server);
+
+       if (server->tcpStatus == CifsNeedReconnect)
+               return;
+
+       switch (rc) {
+       case -1:
+               /* change_conf hasn't been executed */
+               break;
+       case 0:
+               cifs_dbg(VFS, "Possible client or server bug - zero credits\n");
+               break;
+       case 1:
+               cifs_dbg(VFS, "disabling echoes and oplocks\n");
+               break;
+       case 2:
+               cifs_dbg(FYI, "disabling oplocks\n");
+               break;
+       default:
+               cifs_dbg(FYI, "add %u credits total=%d\n", add, rc);
+       }
 }
 
 static void