All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 net] udp: Use READ_ONCE()/WRITE_ONCE() for hslot->count.
@ 2026-04-23  1:09 Kuniyuki Iwashima
  2026-04-23  7:29 ` Eric Dumazet
  0 siblings, 1 reply; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-04-23  1:09 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Willem de Bruijn
  Cc: Simon Horman, David Held, Kuniyuki Iwashima, Kuniyuki Iwashima,
	netdev, syzbot+04905b8b3523856476c0

__udp4_lib_mcast_demux_lookup() and __udp4_lib_mcast_deliver()
reads the number of sockets in the 1-tuple hash table chain
locklessly. [0]

Let's use READ_ONCE() and WRITE_ONCE() for hslot->count.

[0]:
BUG: KCSAN: data-race in __udp4_lib_mcast_deliver / udp_lib_unhash

read-write to 0xffff88810194a9c8 of 4 bytes by task 15107 on cpu 0:
 udp_lib_unhash+0x1f4/0x3e0 net/ipv4/udp.c:2261
 sk_common_release+0x7a/0x230 net/core/sock.c:3990
 udp_lib_close+0x15/0x20 include/net/udp.h:324
 inet_release+0xcd/0xf0 net/ipv4/af_inet.c:437
 __sock_release net/socket.c:662 [inline]
 sock_close+0x6b/0x150 net/socket.c:1455
 __fput+0x29b/0x650 fs/file_table.c:469
 ____fput+0x1c/0x30 fs/file_table.c:497
 task_work_run+0x130/0x1a0 kernel/task_work.c:233
 resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
 __exit_to_user_mode_loop kernel/entry/common.c:67 [inline]
 exit_to_user_mode_loop+0x1f4/0x6f0 kernel/entry/common.c:98
 __exit_to_user_mode_prepare include/linux/irq-entry-common.h:226 [inline]
 syscall_exit_to_user_mode_prepare include/linux/irq-entry-common.h:256 [inline]
 syscall_exit_to_user_mode include/linux/entry-common.h:325 [inline]
 do_syscall_64+0x249/0x370 arch/x86/entry/syscall_64.c:100
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

read to 0xffff88810194a9c8 of 4 bytes by task 15111 on cpu 1:
 __udp4_lib_mcast_deliver+0x9a/0x790 net/ipv4/udp.c:2564
 __udp4_lib_rcv+0x972/0x1250 net/ipv4/udp.c:2749
 udp_rcv+0x4f/0x60 net/ipv4/udp.c:2936
 ip_protocol_deliver_rcu+0x3f7/0x790 net/ipv4/ip_input.c:207
 ip_local_deliver_finish+0x1fc/0x2f0 net/ipv4/ip_input.c:241
 NF_HOOK include/linux/netfilter.h:318 [inline]
 ip_local_deliver+0xe8/0x1e0 net/ipv4/ip_input.c:262
 dst_input include/net/dst.h:480 [inline]
 ip_sublist_rcv_finish net/ipv4/ip_input.c:584 [inline]
 ip_list_rcv_finish net/ipv4/ip_input.c:628 [inline]
 ip_sublist_rcv+0x415/0x6a0 net/ipv4/ip_input.c:644
 ip_list_rcv+0x261/0x290 net/ipv4/ip_input.c:678
 __netif_receive_skb_list_ptype net/core/dev.c:6207 [inline]
 __netif_receive_skb_list_core+0x4dc/0x500 net/core/dev.c:6254
 __netif_receive_skb_list net/core/dev.c:6306 [inline]
 netif_receive_skb_list_internal+0x47d/0x5f0 net/core/dev.c:6397
 netif_receive_skb_list+0x35/0x220 net/core/dev.c:6449
 xdp_recv_frames net/bpf/test_run.c:269 [inline]
 xdp_test_run_batch net/bpf/test_run.c:350 [inline]
 bpf_test_run_xdp_live+0x104c/0x1360 net/bpf/test_run.c:379
 bpf_prog_test_run_xdp+0x57b/0xa10 net/bpf/test_run.c:1397
 bpf_prog_test_run+0x204/0x340 kernel/bpf/syscall.c:4722
 __sys_bpf+0x52e/0x7e0 kernel/bpf/syscall.c:6246
 __do_sys_bpf kernel/bpf/syscall.c:6341 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:6339 [inline]
 __x64_sys_bpf+0x41/0x50 kernel/bpf/syscall.c:6339
 x64_sys_call+0x10cb/0x3020 arch/x86/include/generated/asm/syscalls_64.h:322
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x12c/0x370 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

value changed: 0x00000001 -> 0x00000000

Reported by Kernel Concurrency Sanitizer on:
CPU: 1 UID: 0 PID: 15111 Comm: syz.6.4060 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2026

Fixes: 2dc41cff7545 ("udp: Use hash2 for long hash1 chains in __udp*_lib_mcast_deliver.")
Fixes: 63c6f81cdde5 ("udp: ipv4: do not waste time in __udp4_lib_mcast_demux_lookup")
Reported-by: syzbot+04905b8b3523856476c0@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/69e97093.050a0220.24bfd3.0048.GAE@google.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/udp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0ac2bf4f8759..f7e6045f5985 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -323,7 +323,7 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
 		sock_set_flag(sk, SOCK_RCU_FREE);
 
 		sk_add_node_rcu(sk, &hslot->head);
-		hslot->count++;
+		WRITE_ONCE(hslot->count, hslot->count + 1);
 		sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
 
 		hslot2 = udp_hashslot2(udptable, udp_sk(sk)->udp_portaddr_hash);
@@ -2200,7 +2200,7 @@ void udp_lib_unhash(struct sock *sk)
 		if (rcu_access_pointer(sk->sk_reuseport_cb))
 			reuseport_detach_sock(sk);
 		if (sk_del_node_init_rcu(sk)) {
-			hslot->count--;
+			WRITE_ONCE(hslot->count, hslot->count - 1);
 			inet_sk(sk)->inet_num = 0;
 			sock_prot_inuse_add(net, sk->sk_prot, -1);
 
@@ -2471,7 +2471,7 @@ static int __udp4_lib_mcast_deliver(struct net *net, struct sk_buff *skb,
 	hash2_any = 0;
 	hash2 = 0;
 	hslot = udp_hashslot(udptable, net, hnum);
-	use_hash2 = hslot->count > 10;
+	use_hash2 = READ_ONCE(hslot->count) > 10;
 	offset = offsetof(typeof(*sk), sk_node);
 
 	if (use_hash2) {
@@ -2703,7 +2703,7 @@ static struct sock *__udp4_lib_mcast_demux_lookup(struct net *net,
 	hslot = &udptable->hash[slot];
 
 	/* Do not bother scanning a too big list */
-	if (hslot->count > 10)
+	if (READ_ONCE(hslot->count) > 10)
 		return NULL;
 
 	result = NULL;
-- 
2.54.0.rc2.533.g4f5dca5207-goog


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

* Re: [PATCH v1 net] udp: Use READ_ONCE()/WRITE_ONCE() for hslot->count.
  2026-04-23  1:09 [PATCH v1 net] udp: Use READ_ONCE()/WRITE_ONCE() for hslot->count Kuniyuki Iwashima
@ 2026-04-23  7:29 ` Eric Dumazet
  2026-04-23 18:51   ` Kuniyuki Iwashima
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2026-04-23  7:29 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, David Held, Kuniyuki Iwashima, netdev,
	syzbot+04905b8b3523856476c0

On Wed, Apr 22, 2026 at 6:09 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> __udp4_lib_mcast_demux_lookup() and __udp4_lib_mcast_deliver()
> reads the number of sockets in the 1-tuple hash table chain
> locklessly. [0]
>
> Let's use READ_ONCE() and WRITE_ONCE() for hslot->count.
>
> [0]:
> BUG: KCSAN: data-race in __udp4_lib_mcast_deliver / udp_lib_unhash

...

>
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 1 UID: 0 PID: 15111 Comm: syz.6.4060 Not tainted syzkaller #0 PREEMPT(full)
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2026
>
> Fixes: 2dc41cff7545 ("udp: Use hash2 for long hash1 chains in __udp*_lib_mcast_deliver.")
> Fixes: 63c6f81cdde5 ("udp: ipv4: do not waste time in __udp4_lib_mcast_demux_lookup")
> Reported-by: syzbot+04905b8b3523856476c0@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/69e97093.050a0220.24bfd3.0048.GAE@google.com/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---

You forgot to change net/ipv6/udp.c

diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 15e032194eccc3c29b3e5f3a09214cad60623329..1f4dabe4c350118441f61c1cf0398e698581542c
100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -953,7 +953,7 @@ static int __udp6_lib_mcast_deliver(struct net
*net, struct sk_buff *skb,
        hash2_any = 0;
        hash2 = 0;
        hslot = udp_hashslot(udptable, net, hnum);
-       use_hash2 = hslot->count > 10;
+       use_hash2 = READ_ONCE(hslot->count) > 10;
        offset = offsetof(typeof(*sk), sk_node);

        if (use_hash2) {


Also one part is missing in udp_lib_get_port()

@@ -289,7 +289,7 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
                        hash2_nulladdr &= udptable->mask;

                        hslot2 = udp_hashslot2(udptable, slot2);
-                       if (hslot->count < hslot2->count)
+                       if (hslot->count < READ_ONCE(hslot2->count))
                                goto scan_primary_hash;

                        exist = udp_lib_lport_inuse2(net, snum, hslot2, sk);

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

* Re: [PATCH v1 net] udp: Use READ_ONCE()/WRITE_ONCE() for hslot->count.
  2026-04-23  7:29 ` Eric Dumazet
@ 2026-04-23 18:51   ` Kuniyuki Iwashima
  2026-04-23 18:59     ` Eric Dumazet
  0 siblings, 1 reply; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-04-23 18:51 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, David Held, Kuniyuki Iwashima, netdev,
	syzbot+04905b8b3523856476c0

On Thu, Apr 23, 2026 at 12:29 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Wed, Apr 22, 2026 at 6:09 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > __udp4_lib_mcast_demux_lookup() and __udp4_lib_mcast_deliver()
> > reads the number of sockets in the 1-tuple hash table chain
> > locklessly. [0]
> >
> > Let's use READ_ONCE() and WRITE_ONCE() for hslot->count.
> >
> > [0]:
> > BUG: KCSAN: data-race in __udp4_lib_mcast_deliver / udp_lib_unhash
>
> ...
>
> >
> > Reported by Kernel Concurrency Sanitizer on:
> > CPU: 1 UID: 0 PID: 15111 Comm: syz.6.4060 Not tainted syzkaller #0 PREEMPT(full)
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2026
> >
> > Fixes: 2dc41cff7545 ("udp: Use hash2 for long hash1 chains in __udp*_lib_mcast_deliver.")
> > Fixes: 63c6f81cdde5 ("udp: ipv4: do not waste time in __udp4_lib_mcast_demux_lookup")
> > Reported-by: syzbot+04905b8b3523856476c0@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/netdev/69e97093.050a0220.24bfd3.0048.GAE@google.com/
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
>
> You forgot to change net/ipv6/udp.c

Ah exactly.  Will add this in v2.

>
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 15e032194eccc3c29b3e5f3a09214cad60623329..1f4dabe4c350118441f61c1cf0398e698581542c
> 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -953,7 +953,7 @@ static int __udp6_lib_mcast_deliver(struct net
> *net, struct sk_buff *skb,
>         hash2_any = 0;
>         hash2 = 0;
>         hslot = udp_hashslot(udptable, net, hnum);
> -       use_hash2 = hslot->count > 10;
> +       use_hash2 = READ_ONCE(hslot->count) > 10;
>         offset = offsetof(typeof(*sk), sk_node);
>
>         if (use_hash2) {
>
>
> Also one part is missing in udp_lib_get_port()

I excluded this part since it's under spin_lock_bh(&hslot->lock).

>
> @@ -289,7 +289,7 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
>                         hash2_nulladdr &= udptable->mask;
>
>                         hslot2 = udp_hashslot2(udptable, slot2);
> -                       if (hslot->count < hslot2->count)
> +                       if (hslot->count < READ_ONCE(hslot2->count))
>                                 goto scan_primary_hash;
>
>                         exist = udp_lib_lport_inuse2(net, snum, hslot2, sk);

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

* Re: [PATCH v1 net] udp: Use READ_ONCE()/WRITE_ONCE() for hslot->count.
  2026-04-23 18:51   ` Kuniyuki Iwashima
@ 2026-04-23 18:59     ` Eric Dumazet
  2026-04-23 20:15       ` Kuniyuki Iwashima
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2026-04-23 18:59 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, David Held, Kuniyuki Iwashima, netdev,
	syzbot+04905b8b3523856476c0

On Thu, Apr 23, 2026 at 11:51 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> On Thu, Apr 23, 2026 at 12:29 AM Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Wed, Apr 22, 2026 at 6:09 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> > >
> > > __udp4_lib_mcast_demux_lookup() and __udp4_lib_mcast_deliver()
> > > reads the number of sockets in the 1-tuple hash table chain
> > > locklessly. [0]
> > >
> > > Let's use READ_ONCE() and WRITE_ONCE() for hslot->count.
> > >
> > > [0]:
> > > BUG: KCSAN: data-race in __udp4_lib_mcast_deliver / udp_lib_unhash
> >
> > ...
> >
> > >
> > > Reported by Kernel Concurrency Sanitizer on:
> > > CPU: 1 UID: 0 PID: 15111 Comm: syz.6.4060 Not tainted syzkaller #0 PREEMPT(full)
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2026
> > >
> > > Fixes: 2dc41cff7545 ("udp: Use hash2 for long hash1 chains in __udp*_lib_mcast_deliver.")
> > > Fixes: 63c6f81cdde5 ("udp: ipv4: do not waste time in __udp4_lib_mcast_demux_lookup")
> > > Reported-by: syzbot+04905b8b3523856476c0@syzkaller.appspotmail.com
> > > Closes: https://lore.kernel.org/netdev/69e97093.050a0220.24bfd3.0048.GAE@google.com/
> > > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > > ---
> >
> > You forgot to change net/ipv6/udp.c
>
> Ah exactly.  Will add this in v2.
>
> >
> > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> > index 15e032194eccc3c29b3e5f3a09214cad60623329..1f4dabe4c350118441f61c1cf0398e698581542c
> > 100644
> > --- a/net/ipv6/udp.c
> > +++ b/net/ipv6/udp.c
> > @@ -953,7 +953,7 @@ static int __udp6_lib_mcast_deliver(struct net
> > *net, struct sk_buff *skb,
> >         hash2_any = 0;
> >         hash2 = 0;
> >         hslot = udp_hashslot(udptable, net, hnum);
> > -       use_hash2 = hslot->count > 10;
> > +       use_hash2 = READ_ONCE(hslot->count) > 10;
> >         offset = offsetof(typeof(*sk), sk_node);
> >
> >         if (use_hash2) {
> >
> >
> > Also one part is missing in udp_lib_get_port()
>
> I excluded this part since it's under spin_lock_bh(&hslot->lock).
>

But you would need a spin_lock(hslot2->lock), wich we do not hold.

> >
> > @@ -289,7 +289,7 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
> >                         hash2_nulladdr &= udptable->mask;
> >
> >                         hslot2 = udp_hashslot2(udptable, slot2);
> > -                       if (hslot->count < hslot2->count)
> > +                       if (hslot->count < READ_ONCE(hslot2->count))
> >                                 goto scan_primary_hash;
> >
> >                         exist = udp_lib_lport_inuse2(net, snum, hslot2, sk);

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

* Re: [PATCH v1 net] udp: Use READ_ONCE()/WRITE_ONCE() for hslot->count.
  2026-04-23 18:59     ` Eric Dumazet
@ 2026-04-23 20:15       ` Kuniyuki Iwashima
  0 siblings, 0 replies; 5+ messages in thread
From: Kuniyuki Iwashima @ 2026-04-23 20:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Willem de Bruijn,
	Simon Horman, David Held, Kuniyuki Iwashima, netdev,
	syzbot+04905b8b3523856476c0

On Thu, Apr 23, 2026 at 11:59 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, Apr 23, 2026 at 11:51 AM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > On Thu, Apr 23, 2026 at 12:29 AM Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Wed, Apr 22, 2026 at 6:09 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> > > >
> > > > __udp4_lib_mcast_demux_lookup() and __udp4_lib_mcast_deliver()
> > > > reads the number of sockets in the 1-tuple hash table chain
> > > > locklessly. [0]
> > > >
> > > > Let's use READ_ONCE() and WRITE_ONCE() for hslot->count.
> > > >
> > > > [0]:
> > > > BUG: KCSAN: data-race in __udp4_lib_mcast_deliver / udp_lib_unhash
> > >
> > > ...
> > >
> > > >
> > > > Reported by Kernel Concurrency Sanitizer on:
> > > > CPU: 1 UID: 0 PID: 15111 Comm: syz.6.4060 Not tainted syzkaller #0 PREEMPT(full)
> > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2026
> > > >
> > > > Fixes: 2dc41cff7545 ("udp: Use hash2 for long hash1 chains in __udp*_lib_mcast_deliver.")
> > > > Fixes: 63c6f81cdde5 ("udp: ipv4: do not waste time in __udp4_lib_mcast_demux_lookup")
> > > > Reported-by: syzbot+04905b8b3523856476c0@syzkaller.appspotmail.com
> > > > Closes: https://lore.kernel.org/netdev/69e97093.050a0220.24bfd3.0048.GAE@google.com/
> > > > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > > > ---
> > >
> > > You forgot to change net/ipv6/udp.c
> >
> > Ah exactly.  Will add this in v2.
> >
> > >
> > > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> > > index 15e032194eccc3c29b3e5f3a09214cad60623329..1f4dabe4c350118441f61c1cf0398e698581542c
> > > 100644
> > > --- a/net/ipv6/udp.c
> > > +++ b/net/ipv6/udp.c
> > > @@ -953,7 +953,7 @@ static int __udp6_lib_mcast_deliver(struct net
> > > *net, struct sk_buff *skb,
> > >         hash2_any = 0;
> > >         hash2 = 0;
> > >         hslot = udp_hashslot(udptable, net, hnum);
> > > -       use_hash2 = hslot->count > 10;
> > > +       use_hash2 = READ_ONCE(hslot->count) > 10;
> > >         offset = offsetof(typeof(*sk), sk_node);
> > >
> > >         if (use_hash2) {
> > >
> > >
> > > Also one part is missing in udp_lib_get_port()
> >
> > I excluded this part since it's under spin_lock_bh(&hslot->lock).
> >
>
> But you would need a spin_lock(hslot2->lock), wich we do not hold.

Ah, it's hslot2->count !  I'll cover that too, thank you !

>
> > >
> > > @@ -289,7 +289,7 @@ int udp_lib_get_port(struct sock *sk, unsigned short snum,
> > >                         hash2_nulladdr &= udptable->mask;
> > >
> > >                         hslot2 = udp_hashslot2(udptable, slot2);
> > > -                       if (hslot->count < hslot2->count)
> > > +                       if (hslot->count < READ_ONCE(hslot2->count))
> > >                                 goto scan_primary_hash;
> > >
> > >                         exist = udp_lib_lport_inuse2(net, snum, hslot2, sk);

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

end of thread, other threads:[~2026-04-23 20:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-23  1:09 [PATCH v1 net] udp: Use READ_ONCE()/WRITE_ONCE() for hslot->count Kuniyuki Iwashima
2026-04-23  7:29 ` Eric Dumazet
2026-04-23 18:51   ` Kuniyuki Iwashima
2026-04-23 18:59     ` Eric Dumazet
2026-04-23 20:15       ` Kuniyuki Iwashima

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.