From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 22 May 2025 10:55:47 +0200

On 5/17/25 5:50 AM, Kuniyuki Iwashima wrote:

quoted

Since commit 26abe14379f8 ("net: Modify sk_alloc to not reference
count the netns of kernel sockets."), TCP kernel socket has caused
many UAF.

We have converted such sockets to hold netns refcnt, and we have
the same pattern in cifs, mptcp, nvme, rds, smc, and sunrpc.

  __sock_create_kern(..., &sock);
  sk_net_refcnt_upgrade(sock->sk);

Let's drop the conversion and use sock_create_kern() instead.

The changes for cifs, mptcp, nvme, and smc are straightforward.

For sunrpc, we call sock_create_net() for IPPROTO_TCP only and still
call __sock_create_kern() for others.

For rds, we cannot drop sk_net_refcnt_upgrade() for accept()ed
sockets.

Signed-off-by: Kuniyuki Iwashima <redacted>

This LGTM, but is touching a few other subsystems, it would be great to
collect acks from the relevant maintainers: I'm adding a few CCs.

Direct link to the series:

https://lore.kernel.org/all/20250517035120.55560-1-kuniyu@amazon.com/#t (local)

quoted

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 37a2ba38f10e..c7b4f5a7cca1 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c

@@ -3348,21 +3348,14 @@ 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_kern(net, sfamily, SOCK_STREAM,
-					IPPROTO_TCP, &server->ssocket);
+		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);

AFAICS the above implicitly adds a missing net_passive_dec(net), which
in turns looks like a separate bugfix. What about adding a separate
patch introducing that line? Could be in the same series to simplify the
processing.

Thanks for catching!

Will add this patch before this change.

---8<---
commit c7ff005fe4d930169f319aca0bd9577541cd7459 (HEAD)
Author: Kuniyuki Iwashima [off-list ref]
Date:   Thu May 22 16:03:29 2025 +0000

    smb: client: Add missing net_passive_dec().
    
    While reverting commit e9f2517a3e18 ("smb: client: fix TCP timers deadlock
    after rmmod"), I should have added net_passive_dec(), which was added between
    the original commit and the revert by commit 5c70eb5c593d ("net: better track
    kernel sockets lifetime").
    
    Let's call net_passive_dec() in generic_ip_connect().
    
    Note that this commit is only needed for 6.14+.
    
    Fixes: 95d2b9f693ff ("Revert "smb: client: fix TCP timers deadlock after rmmod"")
    Cc: [off-list ref] # 6.14.x
    Signed-off-by: Kuniyuki Iwashima [off-list ref]

diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 37a2ba38f10e..afac23a5a3ec 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c

@@ -3359,6 +3359,7 @@ generic_ip_connect(struct TCP_Server_Info *server)
 
                sk = server->ssocket->sk;
                __netns_tracker_free(net, &sk->ns_tracker, false);
+               net_passive_dec(net);
                sk->sk_net_refcnt = 1;
                get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
                sock_inuse_add(net, 1);
---8<---