| Message ID | 20240314141816.2640229-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6ebfad33161afacb3e1e59ed1c2feefef70f9f97 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] packet: annotate data-races around ignore_outgoing | expand |
Eric Dumazet wrote: > ignore_outgoing is read locklessly from dev_queue_xmit_nit() > and packet_getsockopt() > > Add appropriate READ_ONCE()/WRITE_ONCE() annotations. > > syzbot reported: > > BUG: KCSAN: data-race in dev_queue_xmit_nit / packet_setsockopt > > write to 0xffff888107804542 of 1 bytes by task 22618 on cpu 0: > packet_setsockopt+0xd83/0xfd0 net/packet/af_packet.c:4003 > do_sock_setsockopt net/socket.c:2311 [inline] > __sys_setsockopt+0x1d8/0x250 net/socket.c:2334 > __do_sys_setsockopt net/socket.c:2343 [inline] > __se_sys_setsockopt net/socket.c:2340 [inline] > __x64_sys_setsockopt+0x66/0x80 net/socket.c:2340 > do_syscall_64+0xd3/0x1d0 > entry_SYSCALL_64_after_hwframe+0x6d/0x75 > > read to 0xffff888107804542 of 1 bytes by task 27 on cpu 1: > dev_queue_xmit_nit+0x82/0x620 net/core/dev.c:2248 > xmit_one net/core/dev.c:3527 [inline] > dev_hard_start_xmit+0xcc/0x3f0 net/core/dev.c:3547 > __dev_queue_xmit+0xf24/0x1dd0 net/core/dev.c:4335 > dev_queue_xmit include/linux/netdevice.h:3091 [inline] > batadv_send_skb_packet+0x264/0x300 net/batman-adv/send.c:108 > batadv_send_broadcast_skb+0x24/0x30 net/batman-adv/send.c:127 > batadv_iv_ogm_send_to_if net/batman-adv/bat_iv_ogm.c:392 [inline] > batadv_iv_ogm_emit net/batman-adv/bat_iv_ogm.c:420 [inline] > batadv_iv_send_outstanding_bat_ogm_packet+0x3f0/0x4b0 net/batman-adv/bat_iv_ogm.c:1700 > process_one_work kernel/workqueue.c:3254 [inline] > process_scheduled_works+0x465/0x990 kernel/workqueue.c:3335 > worker_thread+0x526/0x730 kernel/workqueue.c:3416 > kthread+0x1d1/0x210 kernel/kthread.c:388 > ret_from_fork+0x4b/0x60 arch/x86/kernel/process.c:147 > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243 > > value changed: 0x00 -> 0x01 > > Reported by Kernel Concurrency Sanitizer on: > CPU: 1 PID: 27 Comm: kworker/u8:1 Tainted: G W 6.8.0-syzkaller-08073-g480e035fc4c7 #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/29/2024 > Workqueue: bat_events batadv_iv_send_outstanding_bat_ogm_packet > > Fixes: fa788d986a3a ("packet: add sockopt to ignore outgoing packets") > Reported-by: syzbot+c669c1136495a2e7c31f@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/netdev/CANn89i+Z7MfbkBLOv=p7KZ7=K1rKHO4P1OL5LYDCtBiyqsa9oQ@mail.gmail.com/T/#t > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> > --- > net/core/dev.c | 2 +- > net/packet/af_packet.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index 0766a245816bdf70f6609dc7b6d694ae81e7a9e5..722787c3275527f1652ec98623f61500ee753b45 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -2245,7 +2245,7 @@ void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev) > rcu_read_lock(); > again: > list_for_each_entry_rcu(ptype, ptype_list, list) { > - if (ptype->ignore_outgoing) > + if (READ_ONCE(ptype->ignore_outgoing)) > continue; > > /* Never send packets back to the socket > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c > index 61270826b9ac73e66f9011c3230d4668f0bf7c77..7cfc7d301508fcead214fbdb4e962b0553a17916 100644 > --- a/net/packet/af_packet.c > +++ b/net/packet/af_packet.c > @@ -4000,7 +4000,7 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, > if (val < 0 || val > 1) > return -EINVAL; > > - po->prot_hook.ignore_outgoing = !!val; > + WRITE_ONCE(po->prot_hook.ignore_outgoing, !!val); Should we also include a WRITE_ONCE on the fanout prot_hook: match->prot_hook.ignore_outgoing = type_flags & PACKET_FANOUT_FLAG_IGNORE_OUTGOING; > return 0; > } > case PACKET_TX_HAS_OFF: > @@ -4134,7 +4134,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname, > 0); > break; > case PACKET_IGNORE_OUTGOING: > - val = po->prot_hook.ignore_outgoing; > + val = READ_ONCE(po->prot_hook.ignore_outgoing); > break; > case PACKET_ROLLOVER_STATS: > if (!po->rollover) > -- > 2.44.0.278.ge034bb2e1d-goog >
On Thu, Mar 14, 2024 at 4:33 PM Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote: > > Should we also include a WRITE_ONCE on the fanout prot_hook: > > match->prot_hook.ignore_outgoing = type_flags & PACKET_FANOUT_FLAG_IGNORE_OUTGOING; This is not needed, the variable is not yet visible by other cpus.
Eric Dumazet wrote: > On Thu, Mar 14, 2024 at 4:33 PM Willem de Bruijn > <willemdebruijn.kernel@gmail.com> wrote: > > > > Should we also include a WRITE_ONCE on the fanout prot_hook: > > > > match->prot_hook.ignore_outgoing = type_flags & PACKET_FANOUT_FLAG_IGNORE_OUTGOING; > > This is not needed, the variable is not yet visible by other cpus. Oh of course. Thanks. Reviewed-by: Willem de Bruijn <willemb@google.com>
On Thu, Mar 14, 2024 at 10:18 PM Eric Dumazet <edumazet@google.com> wrote: > > ignore_outgoing is read locklessly from dev_queue_xmit_nit() > and packet_getsockopt() > > Add appropriate READ_ONCE()/WRITE_ONCE() annotations. > > syzbot reported: > > BUG: KCSAN: data-race in dev_queue_xmit_nit / packet_setsockopt > > write to 0xffff888107804542 of 1 bytes by task 22618 on cpu 0: > packet_setsockopt+0xd83/0xfd0 net/packet/af_packet.c:4003 > do_sock_setsockopt net/socket.c:2311 [inline] > __sys_setsockopt+0x1d8/0x250 net/socket.c:2334 > __do_sys_setsockopt net/socket.c:2343 [inline] > __se_sys_setsockopt net/socket.c:2340 [inline] > __x64_sys_setsockopt+0x66/0x80 net/socket.c:2340 > do_syscall_64+0xd3/0x1d0 > entry_SYSCALL_64_after_hwframe+0x6d/0x75 > > read to 0xffff888107804542 of 1 bytes by task 27 on cpu 1: > dev_queue_xmit_nit+0x82/0x620 net/core/dev.c:2248 > xmit_one net/core/dev.c:3527 [inline] > dev_hard_start_xmit+0xcc/0x3f0 net/core/dev.c:3547 > __dev_queue_xmit+0xf24/0x1dd0 net/core/dev.c:4335 > dev_queue_xmit include/linux/netdevice.h:3091 [inline] > batadv_send_skb_packet+0x264/0x300 net/batman-adv/send.c:108 > batadv_send_broadcast_skb+0x24/0x30 net/batman-adv/send.c:127 > batadv_iv_ogm_send_to_if net/batman-adv/bat_iv_ogm.c:392 [inline] > batadv_iv_ogm_emit net/batman-adv/bat_iv_ogm.c:420 [inline] > batadv_iv_send_outstanding_bat_ogm_packet+0x3f0/0x4b0 net/batman-adv/bat_iv_ogm.c:1700 > process_one_work kernel/workqueue.c:3254 [inline] > process_scheduled_works+0x465/0x990 kernel/workqueue.c:3335 > worker_thread+0x526/0x730 kernel/workqueue.c:3416 > kthread+0x1d1/0x210 kernel/kthread.c:388 > ret_from_fork+0x4b/0x60 arch/x86/kernel/process.c:147 > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243 > > value changed: 0x00 -> 0x01 > > Reported by Kernel Concurrency Sanitizer on: > CPU: 1 PID: 27 Comm: kworker/u8:1 Tainted: G W 6.8.0-syzkaller-08073-g480e035fc4c7 #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/29/2024 > Workqueue: bat_events batadv_iv_send_outstanding_bat_ogm_packet > > Fixes: fa788d986a3a ("packet: add sockopt to ignore outgoing packets") > Reported-by: syzbot+c669c1136495a2e7c31f@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/netdev/CANn89i+Z7MfbkBLOv=p7KZ7=K1rKHO4P1OL5LYDCtBiyqsa9oQ@mail.gmail.com/T/#t > Signed-off-by: Eric Dumazet <edumazet@google.com> > Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Reviewed-by: Jason Xing <kerneljasonxing@gmail.com> Thanks!
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Thu, 14 Mar 2024 14:18:16 +0000 you wrote: > ignore_outgoing is read locklessly from dev_queue_xmit_nit() > and packet_getsockopt() > > Add appropriate READ_ONCE()/WRITE_ONCE() annotations. > > syzbot reported: > > [...] Here is the summary with links: - [net] packet: annotate data-races around ignore_outgoing https://git.kernel.org/netdev/net/c/6ebfad33161a You are awesome, thank you!
diff --git a/net/core/dev.c b/net/core/dev.c index 0766a245816bdf70f6609dc7b6d694ae81e7a9e5..722787c3275527f1652ec98623f61500ee753b45 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2245,7 +2245,7 @@ void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev) rcu_read_lock(); again: list_for_each_entry_rcu(ptype, ptype_list, list) { - if (ptype->ignore_outgoing) + if (READ_ONCE(ptype->ignore_outgoing)) continue; /* Never send packets back to the socket diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 61270826b9ac73e66f9011c3230d4668f0bf7c77..7cfc7d301508fcead214fbdb4e962b0553a17916 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -4000,7 +4000,7 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, if (val < 0 || val > 1) return -EINVAL; - po->prot_hook.ignore_outgoing = !!val; + WRITE_ONCE(po->prot_hook.ignore_outgoing, !!val); return 0; } case PACKET_TX_HAS_OFF: @@ -4134,7 +4134,7 @@ static int packet_getsockopt(struct socket *sock, int level, int optname, 0); break; case PACKET_IGNORE_OUTGOING: - val = po->prot_hook.ignore_outgoing; + val = READ_ONCE(po->prot_hook.ignore_outgoing); break; case PACKET_ROLLOVER_STATS: if (!po->rollover)
ignore_outgoing is read locklessly from dev_queue_xmit_nit() and packet_getsockopt() Add appropriate READ_ONCE()/WRITE_ONCE() annotations. syzbot reported: BUG: KCSAN: data-race in dev_queue_xmit_nit / packet_setsockopt write to 0xffff888107804542 of 1 bytes by task 22618 on cpu 0: packet_setsockopt+0xd83/0xfd0 net/packet/af_packet.c:4003 do_sock_setsockopt net/socket.c:2311 [inline] __sys_setsockopt+0x1d8/0x250 net/socket.c:2334 __do_sys_setsockopt net/socket.c:2343 [inline] __se_sys_setsockopt net/socket.c:2340 [inline] __x64_sys_setsockopt+0x66/0x80 net/socket.c:2340 do_syscall_64+0xd3/0x1d0 entry_SYSCALL_64_after_hwframe+0x6d/0x75 read to 0xffff888107804542 of 1 bytes by task 27 on cpu 1: dev_queue_xmit_nit+0x82/0x620 net/core/dev.c:2248 xmit_one net/core/dev.c:3527 [inline] dev_hard_start_xmit+0xcc/0x3f0 net/core/dev.c:3547 __dev_queue_xmit+0xf24/0x1dd0 net/core/dev.c:4335 dev_queue_xmit include/linux/netdevice.h:3091 [inline] batadv_send_skb_packet+0x264/0x300 net/batman-adv/send.c:108 batadv_send_broadcast_skb+0x24/0x30 net/batman-adv/send.c:127 batadv_iv_ogm_send_to_if net/batman-adv/bat_iv_ogm.c:392 [inline] batadv_iv_ogm_emit net/batman-adv/bat_iv_ogm.c:420 [inline] batadv_iv_send_outstanding_bat_ogm_packet+0x3f0/0x4b0 net/batman-adv/bat_iv_ogm.c:1700 process_one_work kernel/workqueue.c:3254 [inline] process_scheduled_works+0x465/0x990 kernel/workqueue.c:3335 worker_thread+0x526/0x730 kernel/workqueue.c:3416 kthread+0x1d1/0x210 kernel/kthread.c:388 ret_from_fork+0x4b/0x60 arch/x86/kernel/process.c:147 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243 value changed: 0x00 -> 0x01 Reported by Kernel Concurrency Sanitizer on: CPU: 1 PID: 27 Comm: kworker/u8:1 Tainted: G W 6.8.0-syzkaller-08073-g480e035fc4c7 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/29/2024 Workqueue: bat_events batadv_iv_send_outstanding_bat_ogm_packet Fixes: fa788d986a3a ("packet: add sockopt to ignore outgoing packets") Reported-by: syzbot+c669c1136495a2e7c31f@syzkaller.appspotmail.com Closes: https://lore.kernel.org/netdev/CANn89i+Z7MfbkBLOv=p7KZ7=K1rKHO4P1OL5LYDCtBiyqsa9oQ@mail.gmail.com/T/#t Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Willem de Bruijn <willemdebruijn.kernel@gmail.com> --- net/core/dev.c | 2 +- net/packet/af_packet.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)