All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
@ 2026-09-14  1:14 Kuniyuki Iwashima
  2026-09-14  8:53 ` Xuanqiang Luo
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-14  1:14 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Neal Cardwell
  Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev,
	Taras Madan

tcp_v6_do_rcv() no longer calls skb_clone_and_charge_r() for
TCP_LISTEN since commit 073d89808c06 ("net: fix data-races around
sk->sk_forward_alloc").

However, there is still a small race window between tcp_v6_rcv()
and tcp_v6_do_rcv(), where concurrent close() changes TCP_LISTEN
to TCP_CLOSE, causing skb_clone_and_charge_r() to be called
locklessly and resulting in the splat below. [0]

Let's avoid calling skb_clone_and_charge_r() for TCP_CLOSE as well.

This is fine for non-listeners because tcp_rcv_state_process()
drops skb for TCP_CLOSE and opt_skb was freed immediately anyway.

[0]:
sk->sk_forward_alloc
WARNING: net/ipv4/af_inet.c:162 at inet_sock_destruct+0x64d/0x810 net/ipv4/af_inet.c:162, CPU#1: ksoftirqd/1/28
Modules linked in:
CPU: 1 UID: 0 PID: 28 Comm: ksoftirqd/1 Not tainted 7.2.0 #17 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
RIP: 0010:inet_sock_destruct+0x64d/0x810 net/ipv4/af_inet.c:162
Code: 3d 49 ff e9 06 fd ff ff e8 d0 5b 83 f8 90 0f 0b 90 e9 35 fe ff ff e8 c2 5b 83 f8 90 0f 0b 90 e9 c5 fe ff ff e8 b4 5b 83 f8 90 <0f> 0b 90 e9 04 ff ff ff e8 a6 5b 83 f8 90 0f 0b 90 e9 65 fe ff ff
RSP: 0018:ffffc90000677bb8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff8880117bde80 RCX: ffffffff8957eb41
RDX: ffff88801dad5d00 RSI: ffffffff8957ec3c RDI: 0000000000000005
RBP: 00000000fffff000 R08: ffffffff8957eb41 R09: 00000000fffff000
R10: 0000000000000005 R11: 0000000000000000 R12: dffffc0000000000
R13: ffff8880117bdf10 R14: ffffffff81c08eb7 R15: 0000000000000003
FS:  0000000000000000(0000) GS:ffff8880d7ae5000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f93a1021138 CR3: 00000000207a9000 CR4: 0000000000350ef0
Call Trace:
 <TASK>
 __sk_destruct+0x82/0xae0 net/core/sock.c:2356
 rcu_do_batch kernel/rcu/tree.c:2645 [inline]
 rcu_core+0x59c/0x1100 kernel/rcu/tree.c:2897
 handle_softirqs+0x1e4/0x9b0 kernel/softirq.c:622
 run_ksoftirqd kernel/softirq.c:1076 [inline]
 run_ksoftirqd+0x38/0x60 kernel/softirq.c:1068
 smpboot_thread_fn+0x458/0xc80 kernel/smpboot.c:160
 kthread+0x396/0x4a0 kernel/kthread.c:436
 ret_from_fork+0x8e0/0xe40 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>

Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
Reported-by: Taras Madan <tarasmadan@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv6/tcp_ipv6.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index df9c29eb5c1f..7fa4ed2fd4f1 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1604,7 +1604,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 	   by tcp. Feel free to propose better solution.
 					       --ANK (980728)
 	 */
-	if (np->rxopt.all && sk->sk_state != TCP_LISTEN)
+	if (np->rxopt.all &&
+	    !((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)))
 		opt_skb = skb_clone_and_charge_r(skb, sk);
 
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
-- 
2.55.0.1007.g17ff1f9808-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
  2026-09-14  1:14 [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv() Kuniyuki Iwashima
@ 2026-09-14  8:53 ` Xuanqiang Luo
  2026-09-14 10:30   ` Eric Dumazet
  2026-09-15  1:15 ` netdev-bot+sashiko
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Xuanqiang Luo @ 2026-09-14  8:53 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: Simon Horman, Kuniyuki Iwashima, netdev, Taras Madan,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Neal Cardwell

在 2026/9/14 09:14, Kuniyuki Iwashima 写道:
> tcp_v6_do_rcv() no longer calls skb_clone_and_charge_r() for
> TCP_LISTEN since commit 073d89808c06 ("net: fix data-races around
> sk->sk_forward_alloc").
> 
> However, there is still a small race window between tcp_v6_rcv()
> and tcp_v6_do_rcv(), where concurrent close() changes TCP_LISTEN
> to TCP_CLOSE, causing skb_clone_and_charge_r() to be called
> locklessly and resulting in the splat below. [0]
> 
> Let's avoid calling skb_clone_and_charge_r() for TCP_CLOSE as well.
> 
> This is fine for non-listeners because tcp_rcv_state_process()
> drops skb for TCP_CLOSE and opt_skb was freed immediately anyway.
> 
> [0]:
> sk->sk_forward_alloc
> WARNING: net/ipv4/af_inet.c:162 at inet_sock_destruct+0x64d/0x810 net/ipv4/af_inet.c:162, CPU#1: ksoftirqd/1/28
> Modules linked in:
> CPU: 1 UID: 0 PID: 28 Comm: ksoftirqd/1 Not tainted 7.2.0 #17 PREEMPT(full)
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
> RIP: 0010:inet_sock_destruct+0x64d/0x810 net/ipv4/af_inet.c:162
> Code: 3d 49 ff e9 06 fd ff ff e8 d0 5b 83 f8 90 0f 0b 90 e9 35 fe ff ff e8 c2 5b 83 f8 90 0f 0b 90 e9 c5 fe ff ff e8 b4 5b 83 f8 90 <0f> 0b 90 e9 04 ff ff ff e8 a6 5b 83 f8 90 0f 0b 90 e9 65 fe ff ff
> RSP: 0018:ffffc90000677bb8 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff8880117bde80 RCX: ffffffff8957eb41
> RDX: ffff88801dad5d00 RSI: ffffffff8957ec3c RDI: 0000000000000005
> RBP: 00000000fffff000 R08: ffffffff8957eb41 R09: 00000000fffff000
> R10: 0000000000000005 R11: 0000000000000000 R12: dffffc0000000000
> R13: ffff8880117bdf10 R14: ffffffff81c08eb7 R15: 0000000000000003
> FS:  0000000000000000(0000) GS:ffff8880d7ae5000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f93a1021138 CR3: 00000000207a9000 CR4: 0000000000350ef0
> Call Trace:
>   <TASK>
>   __sk_destruct+0x82/0xae0 net/core/sock.c:2356
>   rcu_do_batch kernel/rcu/tree.c:2645 [inline]
>   rcu_core+0x59c/0x1100 kernel/rcu/tree.c:2897
>   handle_softirqs+0x1e4/0x9b0 kernel/softirq.c:622
>   run_ksoftirqd kernel/softirq.c:1076 [inline]
>   run_ksoftirqd+0x38/0x60 kernel/softirq.c:1068
>   smpboot_thread_fn+0x458/0xc80 kernel/smpboot.c:160
>   kthread+0x396/0x4a0 kernel/kthread.c:436
>   ret_from_fork+0x8e0/0xe40 arch/x86/kernel/process.c:158
>   ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
>   </TASK>
> 
> Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> Reported-by: Taras Madan <tarasmadan@google.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>

Reviewed-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

Thanks,


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
  2026-09-14  8:53 ` Xuanqiang Luo
@ 2026-09-14 10:30   ` Eric Dumazet
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2026-09-14 10:30 UTC (permalink / raw)
  To: Xuanqiang Luo
  Cc: Kuniyuki Iwashima, Simon Horman, Kuniyuki Iwashima, netdev,
	Taras Madan, David S. Miller, Jakub Kicinski, Paolo Abeni,
	Neal Cardwell

On Mon, Sep 14, 2026 at 1:53 AM Xuanqiang Luo <xuanqiang.luo@linux.dev> wrote:
>
> 在 2026/9/14 09:14, Kuniyuki Iwashima 写道:
> > tcp_v6_do_rcv() no longer calls skb_clone_and_charge_r() for
> > TCP_LISTEN since commit 073d89808c06 ("net: fix data-races around
> > sk->sk_forward_alloc").
> >
> > However, there is still a small race window between tcp_v6_rcv()
> > and tcp_v6_do_rcv(), where concurrent close() changes TCP_LISTEN
> > to TCP_CLOSE, causing skb_clone_and_charge_r() to be called
> > locklessly and resulting in the splat below. [0]
> >
> > Let's avoid calling skb_clone_and_charge_r() for TCP_CLOSE as well.
> >
> > This is fine for non-listeners because tcp_rcv_state_process()
> > drops skb for TCP_CLOSE and opt_skb was freed immediately anyway.
> >
> > [0]:
> > sk->sk_forward_alloc
> > WARNING: net/ipv4/af_inet.c:162 at inet_sock_destruct+0x64d/0x810 net/ipv4/af_inet.c:162, CPU#1: ksoftirqd/1/28
> > Modules linked in:
> > CPU: 1 UID: 0 PID: 28 Comm: ksoftirqd/1 Not tainted 7.2.0 #17 PREEMPT(full)
> > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
> > RIP: 0010:inet_sock_destruct+0x64d/0x810 net/ipv4/af_inet.c:162
> > Code: 3d 49 ff e9 06 fd ff ff e8 d0 5b 83 f8 90 0f 0b 90 e9 35 fe ff ff e8 c2 5b 83 f8 90 0f 0b 90 e9 c5 fe ff ff e8 b4 5b 83 f8 90 <0f> 0b 90 e9 04 ff ff ff e8 a6 5b 83 f8 90 0f 0b 90 e9 65 fe ff ff
> > RSP: 0018:ffffc90000677bb8 EFLAGS: 00010246
> > RAX: 0000000000000000 RBX: ffff8880117bde80 RCX: ffffffff8957eb41
> > RDX: ffff88801dad5d00 RSI: ffffffff8957ec3c RDI: 0000000000000005
> > RBP: 00000000fffff000 R08: ffffffff8957eb41 R09: 00000000fffff000
> > R10: 0000000000000005 R11: 0000000000000000 R12: dffffc0000000000
> > R13: ffff8880117bdf10 R14: ffffffff81c08eb7 R15: 0000000000000003
> > FS:  0000000000000000(0000) GS:ffff8880d7ae5000(0000) knlGS:0000000000000000
> > CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 00007f93a1021138 CR3: 00000000207a9000 CR4: 0000000000350ef0
> > Call Trace:
> >   <TASK>
> >   __sk_destruct+0x82/0xae0 net/core/sock.c:2356
> >   rcu_do_batch kernel/rcu/tree.c:2645 [inline]
> >   rcu_core+0x59c/0x1100 kernel/rcu/tree.c:2897
> >   handle_softirqs+0x1e4/0x9b0 kernel/softirq.c:622
> >   run_ksoftirqd kernel/softirq.c:1076 [inline]
> >   run_ksoftirqd+0x38/0x60 kernel/softirq.c:1068
> >   smpboot_thread_fn+0x458/0xc80 kernel/smpboot.c:160
> >   kthread+0x396/0x4a0 kernel/kthread.c:436
> >   ret_from_fork+0x8e0/0xe40 arch/x86/kernel/process.c:158
> >   ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
> >   </TASK>
> >
> > Fixes: e994b2f0fb92 ("tcp: do not lock listener to process SYN packets")
> > Reported-by: Taras Madan <tarasmadan@google.com>
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
>
> Reviewed-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

Reviewed-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
  2026-09-14  1:14 [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv() Kuniyuki Iwashima
  2026-09-14  8:53 ` Xuanqiang Luo
@ 2026-09-15  1:15 ` netdev-bot+sashiko
  2026-09-15  4:38   ` Kuniyuki Iwashima
  2026-09-15 13:13 ` Paolo Abeni
  2026-09-15 13:20 ` patchwork-bot+netdevbpf
  3 siblings, 1 reply; 7+ messages in thread
From: netdev-bot+sashiko @ 2026-09-15  1:15 UTC (permalink / raw)
  To: kuniyu
  Cc: davem, edumazet, kuba, pabeni, ncardwell, horms, kuni1840, netdev,
	tarasmadan

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 0

- [High] Incomplete fix / residual TOCTOU race in tcp_v6_do_rcv(): the
  new guard is a state *blacklist* (`!((1 << sk->sk_state) &…

--

tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().

The patch extends the guard in tcp_v6_do_rcv() so that
skb_clone_and_charge_r() is skipped for TCP_CLOSE in addition to
TCP_LISTEN, closing the window where a concurrent close() turns a listener
into TCP_CLOSE while the lockless listener path is running.  The commit
message notes this is safe for non-listeners because tcp_rcv_state_process()
drops the skb for TCP_CLOSE.

No Link: tags are present in the commit header.

> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index df9c29eb5c1f4..7fa4ed2fd4f16 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1604,7 +1604,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
>  	   by tcp. Feel free to propose better solution.
>  					       --ANK (980728)
>  	 */
> -	if (np->rxopt.all && sk->sk_state != TCP_LISTEN)
> +	if (np->rxopt.all &&
> +	    !((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)))
>  		opt_skb = skb_clone_and_charge_r(skb, sk);

[Severity: High]
Is TCP_CLOSE the only other state the lockless listener path can observe
here?

tcp_v6_rcv() calls tcp_v6_do_rcv() without taking the socket lock when it
samples TCP_LISTEN:

net/ipv6/tcp_ipv6.c:tcp_v6_rcv() {
	...
	if (sk->sk_state == TCP_LISTEN) {
		ret = tcp_v6_do_rcv(sk, skb);
		goto put_and_return;
	}

	sk_incoming_cpu_update(sk);

	bh_lock_sock_nested(sk);
	...
}

Only a socket reference is held across that call, so nothing pins
sk->sk_state while tcp_v6_do_rcv() re-reads it.  Can user space move the
same socket further than TCP_CLOSE inside that window?  inet_shutdown() on
a listener falls through to sk->sk_prot->disconnect(), i.e. tcp_disconnect()
-> tcp_set_state(sk, TCP_CLOSE), and a following connect() on the still-open
fd sets TCP_SYN_SENT.

If TCP_SYN_SENT (or later TCP_ESTABLISHED) is what gets loaded at the new
check, the blacklist passes and skb_clone_and_charge_r() runs with no socket
lock again:

include/net/sock.h:skb_clone_and_charge_r() {
	skb = skb_clone(skb, sk_gfp_mask(sk, GFP_ATOMIC));
	if (skb) {
		if (sk_rmem_schedule(sk, skb, skb->truesize)) {
			skb_set_owner_r(skb, sk);
	...
}

sk_rmem_schedule() and skb_set_owner_r() -> sk_mem_charge() ->
sk_forward_alloc_add() perform a non-atomic read-modify-write of
sk->sk_forward_alloc, which is the same accounting corruption behind the
inet_sock_destruct() "sk->sk_forward_alloc" warning quoted in the commit
message.

Would a positive condition be more robust than extending the blacklist,
for example bailing out of tcp_v6_do_rcv() when the lockless listener path
observes sk->sk_state != TCP_LISTEN?  That would also match the comment at
the top of the function:

	/* The socket must have it's spinlock held when we get
	 * here, unless it is a TCP_LISTEN socket.
	 ...
	 */

On the same window, does the TCP_ESTABLISHED fast path just below become
reachable without the lock as well?

	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
		struct dst_entry *dst;

		dst = rcu_dereference_protected(sk->sk_rx_dst,
						lockdep_sock_is_held(sk));

The lockdep_sock_is_held() assertion, the RCU_INIT_POINTER()/dst_release()
of sk->sk_rx_dst and tcp_rcv_established() all assume the socket lock is
held.

Should the state reads on this path also use READ_ONCE() (or
inet_sk_state_load()) instead of plain loads, given the socket lock is not
held?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914011420.115556-1-kuniyu%40google.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
  2026-09-15  1:15 ` netdev-bot+sashiko
@ 2026-09-15  4:38   ` Kuniyuki Iwashima
  0 siblings, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2026-09-15  4:38 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: davem, edumazet, kuba, pabeni, ncardwell, horms, kuni1840, netdev,
	tarasmadan

On Mon, Sep 14, 2026 at 6:15 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 · Medium: 0 · Low: 0
>
> - [High] Incomplete fix / residual TOCTOU race in tcp_v6_do_rcv(): the
>   new guard is a state *blacklist* (`!((1 << sk->sk_state) &…
>
> --
>
> tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
>
> The patch extends the guard in tcp_v6_do_rcv() so that
> skb_clone_and_charge_r() is skipped for TCP_CLOSE in addition to
> TCP_LISTEN, closing the window where a concurrent close() turns a listener
> into TCP_CLOSE while the lockless listener path is running.  The commit
> message notes this is safe for non-listeners because tcp_rcv_state_process()
> drops the skb for TCP_CLOSE.
>
> No Link: tags are present in the commit header.
>
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index df9c29eb5c1f4..7fa4ed2fd4f16 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -1604,7 +1604,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
> >          by tcp. Feel free to propose better solution.
> >                                              --ANK (980728)
> >        */
> > -     if (np->rxopt.all && sk->sk_state != TCP_LISTEN)
> > +     if (np->rxopt.all &&
> > +         !((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)))
> >               opt_skb = skb_clone_and_charge_r(skb, sk);
>
> [Severity: High]
> Is TCP_CLOSE the only other state the lockless listener path can observe
> here?
>
> tcp_v6_rcv() calls tcp_v6_do_rcv() without taking the socket lock when it
> samples TCP_LISTEN:
>
> net/ipv6/tcp_ipv6.c:tcp_v6_rcv() {
>         ...
>         if (sk->sk_state == TCP_LISTEN) {
>                 ret = tcp_v6_do_rcv(sk, skb);
>                 goto put_and_return;
>         }
>
>         sk_incoming_cpu_update(sk);
>
>         bh_lock_sock_nested(sk);
>         ...
> }
>
> Only a socket reference is held across that call, so nothing pins
> sk->sk_state while tcp_v6_do_rcv() re-reads it.  Can user space move the
> same socket further than TCP_CLOSE inside that window?  inet_shutdown() on
> a listener falls through to sk->sk_prot->disconnect(), i.e. tcp_disconnect()
> -> tcp_set_state(sk, TCP_CLOSE), and a following connect() on the still-open
> fd sets TCP_SYN_SENT.

This is yet another bug due to the buggy transition we forbade
in 8cc3aef0cb19 but only in net-next.


>
> If TCP_SYN_SENT (or later TCP_ESTABLISHED) is what gets loaded at the new
> check, the blacklist passes and skb_clone_and_charge_r() runs with no socket
> lock again:
>
> include/net/sock.h:skb_clone_and_charge_r() {
>         skb = skb_clone(skb, sk_gfp_mask(sk, GFP_ATOMIC));
>         if (skb) {
>                 if (sk_rmem_schedule(sk, skb, skb->truesize)) {
>                         skb_set_owner_r(skb, sk);
>         ...
> }
>
> sk_rmem_schedule() and skb_set_owner_r() -> sk_mem_charge() ->
> sk_forward_alloc_add() perform a non-atomic read-modify-write of
> sk->sk_forward_alloc, which is the same accounting corruption behind the
> inet_sock_destruct() "sk->sk_forward_alloc" warning quoted in the commit
> message.
>
> Would a positive condition be more robust than extending the blacklist,
> for example bailing out of tcp_v6_do_rcv() when the lockless listener path
> observes sk->sk_state != TCP_LISTEN?  That would also match the comment at
> the top of the function:
>
>         /* The socket must have it's spinlock held when we get
>          * here, unless it is a TCP_LISTEN socket.
>          ...
>          */
>
> On the same window, does the TCP_ESTABLISHED fast path just below become
> reachable without the lock as well?
>
>         if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
>                 struct dst_entry *dst;
>
>                 dst = rcu_dereference_protected(sk->sk_rx_dst,
>                                                 lockdep_sock_is_held(sk));
>
> The lockdep_sock_is_held() assertion, the RCU_INIT_POINTER()/dst_release()
> of sk->sk_rx_dst and tcp_rcv_established() all assume the socket lock is
> held.
>
> Should the state reads on this path also use READ_ONCE() (or
> inet_sk_state_load()) instead of plain loads, given the socket lock is not
> held?
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260914011420.115556-1-kuniyu%40google.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
  2026-09-14  1:14 [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv() Kuniyuki Iwashima
  2026-09-14  8:53 ` Xuanqiang Luo
  2026-09-15  1:15 ` netdev-bot+sashiko
@ 2026-09-15 13:13 ` Paolo Abeni
  2026-09-15 13:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 7+ messages in thread
From: Paolo Abeni @ 2026-09-15 13:13 UTC (permalink / raw)
  To: Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Neal Cardwell
  Cc: Simon Horman, Kuniyuki Iwashima, netdev, Taras Madan

On 9/14/26 3:14 AM, Kuniyuki Iwashima wrote:
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index df9c29eb5c1f..7fa4ed2fd4f1 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1604,7 +1604,8 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
>  	   by tcp. Feel free to propose better solution.
>  					       --ANK (980728)
>  	 */
> -	if (np->rxopt.all && sk->sk_state != TCP_LISTEN)
> +	if (np->rxopt.all &&
> +	    !((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)))
>  		opt_skb = skb_clone_and_charge_r(skb, sk);

FTR clashiko thinks/laments that other states could be observed, but that
is a sloop. Apparently it can't digest commit 8cc3aef0cb19

/P


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
  2026-09-14  1:14 [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv() Kuniyuki Iwashima
                   ` (2 preceding siblings ...)
  2026-09-15 13:13 ` Paolo Abeni
@ 2026-09-15 13:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-15 13:20 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: davem, edumazet, kuba, pabeni, ncardwell, horms, kuni1840, netdev,
	tarasmadan

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 14 Sep 2026 01:14:01 +0000 you wrote:
> tcp_v6_do_rcv() no longer calls skb_clone_and_charge_r() for
> TCP_LISTEN since commit 073d89808c06 ("net: fix data-races around
> sk->sk_forward_alloc").
> 
> However, there is still a small race window between tcp_v6_rcv()
> and tcp_v6_do_rcv(), where concurrent close() changes TCP_LISTEN
> to TCP_CLOSE, causing skb_clone_and_charge_r() to be called
> locklessly and resulting in the splat below. [0]
> 
> [...]

Here is the summary with links:
  - [v1,net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv().
    https://git.kernel.org/netdev/net/c/8e759cd1f644

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-15 13:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14  1:14 [PATCH v1 net] tcp: Don't call skb_clone_and_charge_r() for close()d listener in tcp_v6_do_rcv() Kuniyuki Iwashima
2026-09-14  8:53 ` Xuanqiang Luo
2026-09-14 10:30   ` Eric Dumazet
2026-09-15  1:15 ` netdev-bot+sashiko
2026-09-15  4:38   ` Kuniyuki Iwashima
2026-09-15 13:13 ` Paolo Abeni
2026-09-15 13:20 ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.