Revert "smb: client: Fix netns refcount imbalance causing leaks and use-after-free"
authorKuniyuki Iwashima <kuniyu@amazon.com>
Wed, 2 Apr 2025 20:26:47 +0000 (13:26 -0700)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 25 Apr 2025 08:45:49 +0000 (10:45 +0200)
commit c707193a17128fae2802d10cbad7239cc57f0c95 upstream.

This reverts commit 4e7f1644f2ac6d01dc584f6301c3b1d5aac4eaef.

The commit e9f2517a3e18 ("smb: client: fix TCP timers deadlock after
rmmod") is not only a bogus fix for LOCKDEP null-ptr-deref but also
introduces a real issue, TCP sockets leak, which will be explained in
detail in the next revert.

Also, CNA assigned CVE-2024-54680 to it but is rejecting it. [0]

Thus, we are reverting the commit and its follow-up commit 4e7f1644f2ac
("smb: client: Fix netns refcount imbalance causing leaks and
use-after-free").

Link: https://lore.kernel.org/all/2025040248-tummy-smilingly-4240@gregkh/
Fixes: 4e7f1644f2ac ("smb: client: Fix netns refcount imbalance causing leaks and use-after-free")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/smb/client/connect.c

index 108e4881d6a265419c8a25bdbb3945c24c05fa04..bc8ecfb59ddd321c55fbfb47d9bc9a38c235a801 100644 (file)
@@ -316,7 +316,6 @@ cifs_abort_connection(struct TCP_Server_Info *server)
                         server->ssocket->flags);
                sock_release(server->ssocket);
                server->ssocket = NULL;
-               put_net(cifs_net_ns(server));
        }
        server->sequence_number = 0;
        server->session_estab = false;
@@ -3150,12 +3149,8 @@ generic_ip_connect(struct TCP_Server_Info *server)
                /*
                 * Grab netns reference for the socket.
                 *
-                * This reference will be released in several situations:
-                * - In the failure path before the cifsd thread is started.
-                * - In the all place where server->socket is released, it is
-                *   also set to NULL.
-                * - Ultimately in clean_demultiplex_info(), during the final
-                *   teardown.
+                * It'll be released here, on error, or in clean_demultiplex_info() upon server
+                * teardown.
                 */
                get_net(net);
 
@@ -3171,8 +3166,10 @@ generic_ip_connect(struct TCP_Server_Info *server)
        }
 
        rc = bind_socket(server);
-       if (rc < 0)
+       if (rc < 0) {
+               put_net(cifs_net_ns(server));
                return rc;
+       }
 
        /*
         * Eventually check for other socket options to change from
@@ -3218,6 +3215,9 @@ generic_ip_connect(struct TCP_Server_Info *server)
        if (sport == htons(RFC1001_PORT))
                rc = ip_rfc1001_connect(server);
 
+       if (rc < 0)
+               put_net(cifs_net_ns(server));
+
        return rc;
 }