Re: [PATCH v1 net-next 4/6] socket: Remove kernel socket conversion except for net/rds/.
From: Kuniyuki Iwashima <hidden>
Date: 2025-05-22 17:05:43
Also in:
linux-nfs, linux-nvme, linux-rdma, mptcp
From: Chuck Lever <redacted> Date: Thu, 22 May 2025 12:38:03 -0400
On 5/22/25 4:55 AM, Paolo Abeni wrote:quoted
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)Thank you, Paolo, for forwarding this series. For all hunks modifying net/sunrpc/svcsock.c and net/handshake/handshake-test.c: Acked-by: Chuck Lever [off-list ref] Regarding patch 4/6: This paragraph in the patch description needs to explain /why/ sunrpc is an exception:quoted
For sunrpc, we call sock_create_net() for IPPROTO_TCP only and still call __sock_create_kern() for others.
Sorry I noticed this sentence was not updated from the previous series.
I'll change it as follows
For sunrpc, we call sk_net_refcnt_upgrade() for IPPROTO_TCP only
so we use sock_create_kern() for TCP and keep __sock_create_kern()
for others.
quoted hunk ↗ jump to hunk
The below hunk doesn't seem related to the marquee purpose of this series. Should it be a separate patch with its own rationale?@@ -1541,8 +1544,8 @@ static struct svc_xprt *svc_create_socket(structsvc_serv *serv, newlen = error; if (protocol == IPPROTO_TCP) { - sk_net_refcnt_upgrade(sock->sk);
The part above is related, and the below is not, using the old style warned by checkpatch, so I cleaned it up while at it but didn't think it's worth a patch. I'm fine to drop it.
- if ((error = kernel_listen(sock, 64)) < 0) + error = kernel_listen(sock, 64); + if (error < 0) goto bummer; }