Message-ID: <CAH2r5mv02iZ3OWk9ZhQdFFH89rbEAuLF_yek6+v_yvyMPHugxw@mail.gmail.com>
Date: Sun, 20 Jul 2025 19:19:52 -0500
From: Steve French <smfrench@...il.com>
To: Wang Zhaolong <wangzhaolong@...weicloud.com>
Cc: sfrench@...ba.org, kuniyu@...gle.com, linux-cifs@...r.kernel.org, 
	samba-technical@...ts.samba.org, linux-kernel@...r.kernel.org, 
	chengzhihao1@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH V2] smb: client: fix netns refcount leak after net_passive changes

merged into cifs-2.6.git for-next pending more review and testing

On Thu, Jul 17, 2025 at 8:35 AM Wang Zhaolong
<wangzhaolong@...weicloud.com> wrote:
>
> After commit 5c70eb5c593d ("net: better track kernel sockets lifetime"),
> kernel sockets now use net_passive reference counting. However, commit
> 95d2b9f693ff ("Revert "smb: client: fix TCP timers deadlock after rmmod"")
> restored the manual socket refcount manipulation without adapting to this
> new mechanism, causing a memory leak.
>
> The issue can be reproduced by[1]:
> 1. Creating a network namespace
> 2. Mounting and Unmounting CIFS within the namespace
> 3. Deleting the namespace
>
> Some memory leaks may appear after a period of time following step 3.
>
> unreferenced object 0xffff9951419f6b00 (size 256):
>   comm "ip", pid 447, jiffies 4294692389 (age 14.730s)
>   hex dump (first 32 bytes):
>     1b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 80 77 c2 44 51 99 ff ff  .........w.DQ...
>   backtrace:
>     __kmem_cache_alloc_node+0x30e/0x3d0
>     __kmalloc+0x52/0x120
>     net_alloc_generic+0x1d/0x30
>     copy_net_ns+0x86/0x200
>     create_new_namespaces+0x117/0x300
>     unshare_nsproxy_namespaces+0x60/0xa0
>     ksys_unshare+0x148/0x360
>     __x64_sys_unshare+0x12/0x20
>     do_syscall_64+0x59/0x110
>     entry_SYSCALL_64_after_hwframe+0x78/0xe2
> ...
> unreferenced object 0xffff9951442e7500 (size 32):
>   comm "mount.cifs", pid 475, jiffies 4294693782 (age 13.343s)
>   hex dump (first 32 bytes):
>     40 c5 38 46 51 99 ff ff 18 01 96 42 51 99 ff ff  @.8FQ......BQ...
>     01 00 00 00 6f 00 c5 07 6f 00 d8 07 00 00 00 00  ....o...o.......
>   backtrace:
>     __kmem_cache_alloc_node+0x30e/0x3d0
>     kmalloc_trace+0x2a/0x90
>     ref_tracker_alloc+0x8e/0x1d0
>     sk_alloc+0x18c/0x1c0
>     inet_create+0xf1/0x370
>     __sock_create+0xd7/0x1e0
>     generic_ip_connect+0x1d4/0x5a0 [cifs]
>     cifs_get_tcp_session+0x5d0/0x8a0 [cifs]
>     cifs_mount_get_session+0x47/0x1b0 [cifs]
>     dfs_mount_share+0xfa/0xa10 [cifs]
>     cifs_mount+0x68/0x2b0 [cifs]
>     cifs_smb3_do_mount+0x10b/0x760 [cifs]
>     smb3_get_tree+0x112/0x2e0 [cifs]
>     vfs_get_tree+0x29/0xf0
>     path_mount+0x2d4/0xa00
>     __se_sys_mount+0x165/0x1d0
>
> Root cause:
> When creating kernel sockets, sk_alloc() calls net_passive_inc() for
> sockets with sk_net_refcnt=0. The CIFS code manually converts kernel
> sockets to user sockets by setting sk_net_refcnt=1, but doesn't call
> the corresponding net_passive_dec(). This creates an imbalance in the
> net_passive counter, which prevents the network namespace from being
> destroyed when its last user reference is dropped. As a result, the
> entire namespace and all its associated resources remain allocated.
>
> Timeline of patches leading to this issue:
> - commit ef7134c7fc48 ("smb: client: Fix use-after-free of network
>   namespace.") in v6.12 fixed the original netns UAF by manually
>   managing socket refcounts
> - commit e9f2517a3e18 ("smb: client: fix TCP timers deadlock after
>   rmmod") in v6.13 attempted to use kernel sockets but introduced
>   TCP timer issues
> - commit 5c70eb5c593d ("net: better track kernel sockets lifetime")
>   in v6.14-rc5 introduced the net_passive mechanism with
>   sk_net_refcnt_upgrade() for proper socket conversion
> - commit 95d2b9f693ff ("Revert "smb: client: fix TCP timers deadlock
>   after rmmod"") in v6.15-rc3 reverted to manual refcount management
>   without adapting to the new net_passive changes
>
> Fix this by using sk_net_refcnt_upgrade() which properly handles the
> net_passive counter when converting kernel sockets to user sockets.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=220343 [1]
> Fixes: 95d2b9f693ff ("Revert "smb: client: fix TCP timers deadlock after rmmod"")
> Cc: stable@...r.kernel.org
> Signed-off-by: Wang Zhaolong <wangzhaolong@...weicloud.com>
> ---
>  fs/smb/client/connect.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> V1 -> V2:
> - Add a simplified description of the reproduction steps in the
>   commit message.
>
> diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
> index 205f547ca49e..5eec8957f2a9 100644
> --- a/fs/smb/client/connect.c
> +++ b/fs/smb/client/connect.c
> @@ -3360,22 +3360,19 @@ generic_ip_connect(struct TCP_Server_Info *server)
>                 socket = server->ssocket;
>         } else {
>                 struct net *net = cifs_net_ns(server);
>                 struct sock *sk;
>
> -               rc = __sock_create(net, sfamily, SOCK_STREAM,
> -                                  IPPROTO_TCP, &server->ssocket, 1);
> +               rc = sock_create_kern(net, sfamily, SOCK_STREAM,
> +                                     IPPROTO_TCP, &server->ssocket);
>                 if (rc < 0) {
>                         cifs_server_dbg(VFS, "Error %d creating socket\n", rc);
>                         return rc;
>                 }
>
>                 sk = server->ssocket->sk;
> -               __netns_tracker_free(net, &sk->ns_tracker, false);
> -               sk->sk_net_refcnt = 1;
> -               get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
> -               sock_inuse_add(net, 1);
> +               sk_net_refcnt_upgrade(sk);
>
>                 /* BB other socket options to set KEEPALIVE, NODELAY? */
>                 cifs_dbg(FYI, "Socket created\n");
>                 socket = server->ssocket;
>                 socket->sk->sk_allocation = GFP_NOFS;
> --
> 2.39.2
>
>


-- 
Thanks,

Steve

Powered by blists - more mailing lists