cifs: fix lock ordering while disabling multichannel

commit 5eef12c4e3230f2025dc46ad8c4a3bc19978e5d7 upstream.

The code to handle the case of server disabling multichannel
was picking iface_lock with chan_lock held. This goes against
the lock ordering rules, as iface_lock is a higher order lock
(even if it isn't so obvious).

This change fixes the lock ordering by doing the following in
that order for each secondary channel:
1. store iface and server pointers in local variable
2. remove references to iface and server in channels
3. unlock chan_lock
4. lock iface_lock
5. dec ref count for iface
6. unlock iface_lock
7. dec ref count for server
8. lock chan_lock again

Since this function can only be called in smb2_reconnect, and
that cannot be called by two parallel processes, we should not
have races due to dropping chan_lock between steps 3 and 8.

Fixes: ee1d21794e55 ("cifs: handle when server stops supporting multichannel")
Reported-by: Paulo Alcantara <pc@manguebit.com>
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index 2ce1b7571371410b58042b8a93eb11e3d6fa3504..62596299a3964c757444ebd85a934bcc29d6a293 100644 (file)
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -316,28 +316,32 @@ cifs_disable_secondary_channels(struct cifs_ses *ses)
                iface = ses->chans[i].iface;
                server = ses->chans[i].server;
 
+               /*
+                * remove these references first, since we need to unlock
+                * the chan_lock here, since iface_lock is a higher lock
+                */
+               ses->chans[i].iface = NULL;
+               ses->chans[i].server = NULL;
+               spin_unlock(&ses->chan_lock);
+
                if (iface) {
                        spin_lock(&ses->iface_lock);
                        kref_put(&iface->refcount, release_iface);
-                       ses->chans[i].iface = NULL;
                        iface->num_channels--;
                        if (iface->weight_fulfilled)
                                iface->weight_fulfilled--;
                        spin_unlock(&ses->iface_lock);
                }
 
-               spin_unlock(&ses->chan_lock);
-               if (server && !server->terminate) {
-                       server->terminate = true;
-                       cifs_signal_cifsd_for_reconnect(server, false);
-               }
-               spin_lock(&ses->chan_lock);
-
                if (server) {
-                       ses->chans[i].server = NULL;
+                       if (!server->terminate) {
+                               server->terminate = true;
+                               cifs_signal_cifsd_for_reconnect(server, false);
+                       }
                        cifs_put_tcp_session(server, false);
                }
 
+               spin_lock(&ses->chan_lock);
        }
 
 done: