]> git.ipfire.org Git - thirdparty/kernel/stable.git/commit
smb: client: fix TCP timers deadlock after rmmod
authorEnzo Matsumiya <ematsumiya@suse.de>
Tue, 10 Dec 2024 21:15:12 +0000 (18:15 -0300)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 27 Dec 2024 13:02:15 +0000 (14:02 +0100)
commit127e907e11ccd54b59bb78fc22c43ccb76c71079
treef32748a7a5a694874e5452cb7aa0c9fa5ff45ae8
parent22b5c2acd65dbe949032f619d4758a35a82fffc3
smb: client: fix TCP timers deadlock after rmmod

commit e9f2517a3e18a54a3943c098d2226b245d488801 upstream.

Commit ef7134c7fc48 ("smb: client: Fix use-after-free of network namespace.")
fixed a netns UAF by manually enabled socket refcounting
(sk->sk_net_refcnt=1 and sock_inuse_add(net, 1)).

The reason the patch worked for that bug was because we now hold
references to the netns (get_net_track() gets a ref internally)
and they're properly released (internally, on __sk_destruct()),
but only because sk->sk_net_refcnt was set.

Problem:
(this happens regardless of CONFIG_NET_NS_REFCNT_TRACKER and regardless
if init_net or other)

Setting sk->sk_net_refcnt=1 *manually* and *after* socket creation is not
only out of cifs scope, but also technically wrong -- it's set conditionally
based on user (=1) vs kernel (=0) sockets.  And net/ implementations
seem to base their user vs kernel space operations on it.

e.g. upon TCP socket close, the TCP timers are not cleared because
sk->sk_net_refcnt=1:
(cf. commit 151c9c724d05 ("tcp: properly terminate timers for kernel sockets"))

net/ipv4/tcp.c:
    void tcp_close(struct sock *sk, long timeout)
    {
     lock_sock(sk);
     __tcp_close(sk, timeout);
     release_sock(sk);
     if (!sk->sk_net_refcnt)
     inet_csk_clear_xmit_timers_sync(sk);
     sock_put(sk);
    }

Which will throw a lockdep warning and then, as expected, deadlock on
tcp_write_timer().

A way to reproduce this is by running the reproducer from ef7134c7fc48
and then 'rmmod cifs'.  A few seconds later, the deadlock/lockdep
warning shows up.

Fix:
We shouldn't mess with socket internals ourselves, so do not set
sk_net_refcnt manually.

Also change __sock_create() to sock_create_kern() for explicitness.

As for non-init_net network namespaces, we deal with it the best way
we can -- hold an extra netns reference for server->ssocket and drop it
when it's released.  This ensures that the netns still exists whenever
we need to create/destroy server->ssocket, but is not directly tied to
it.

Fixes: ef7134c7fc48 ("smb: client: Fix use-after-free of network namespace.")
Cc: stable@vger.kernel.org
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
fs/smb/client/connect.c