netdev - Re: [PATCH net] veth: fix races around rq->rx_notify

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 9 Feb 2022 21:36:37 +0900
From:   Toshiaki Makita <toshiaki.makita1@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     netdev <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Toshiaki Makita <makita.toshiaki@....ntt.co.jp>,
        syzbot <syzkaller@...glegroups.com>
Subject: Re: [PATCH net] veth: fix races around rq->rx_notify_masked

On 2022/02/09 8:28, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@...gle.com>

Thank you for handling this case.

> veth being NETIF_F_LLTX enabled, we need to be more careful
> whenever we read/write rq->rx_notify_masked.
> 
> BUG: KCSAN: data-race in veth_xmit / veth_xmit
> 
> write to 0xffff888133d9a9f8 of 1 bytes by task 23552 on cpu 0:
>   __veth_xdp_flush drivers/net/veth.c:269 [inline]
>   veth_xmit+0x307/0x470 drivers/net/veth.c:350
>   __netdev_start_xmit include/linux/netdevice.h:4683 [inline]
>   netdev_start_xmit include/linux/netdevice.h:4697 [inline]
>   xmit_one+0x105/0x2f0 net/core/dev.c:3473
>   dev_hard_start_xmit net/core/dev.c:3489 [inline]
>   __dev_queue_xmit+0x86d/0xf90 net/core/dev.c:4116
>   dev_queue_xmit+0x13/0x20 net/core/dev.c:4149
>   br_dev_queue_push_xmit+0x3ce/0x430 net/bridge/br_forward.c:53
>   NF_HOOK include/linux/netfilter.h:307 [inline]
>   br_forward_finish net/bridge/br_forward.c:66 [inline]
>   NF_HOOK include/linux/netfilter.h:307 [inline]
>   __br_forward+0x2e4/0x400 net/bridge/br_forward.c:115
>   br_flood+0x521/0x5c0 net/bridge/br_forward.c:242
>   br_dev_xmit+0x8b6/0x960
>   __netdev_start_xmit include/linux/netdevice.h:4683 [inline]
>   netdev_start_xmit include/linux/netdevice.h:4697 [inline]
>   xmit_one+0x105/0x2f0 net/core/dev.c:3473
>   dev_hard_start_xmit net/core/dev.c:3489 [inline]
>   __dev_queue_xmit+0x86d/0xf90 net/core/dev.c:4116
>   dev_queue_xmit+0x13/0x20 net/core/dev.c:4149
>   neigh_hh_output include/net/neighbour.h:525 [inline]
>   neigh_output include/net/neighbour.h:539 [inline]
>   ip_finish_output2+0x6f8/0xb70 net/ipv4/ip_output.c:228
>   ip_finish_output+0xfb/0x240 net/ipv4/ip_output.c:316
>   NF_HOOK_COND include/linux/netfilter.h:296 [inline]
>   ip_output+0xf3/0x1a0 net/ipv4/ip_output.c:430
>   dst_output include/net/dst.h:451 [inline]
>   ip_local_out net/ipv4/ip_output.c:126 [inline]
>   ip_send_skb+0x6e/0xe0 net/ipv4/ip_output.c:1570
>   udp_send_skb+0x641/0x880 net/ipv4/udp.c:967
>   udp_sendmsg+0x12ea/0x14c0 net/ipv4/udp.c:1254
>   inet_sendmsg+0x5f/0x80 net/ipv4/af_inet.c:819
>   sock_sendmsg_nosec net/socket.c:705 [inline]
>   sock_sendmsg net/socket.c:725 [inline]
>   ____sys_sendmsg+0x39a/0x510 net/socket.c:2413
>   ___sys_sendmsg net/socket.c:2467 [inline]
>   __sys_sendmmsg+0x267/0x4c0 net/socket.c:2553
>   __do_sys_sendmmsg net/socket.c:2582 [inline]
>   __se_sys_sendmmsg net/socket.c:2579 [inline]
>   __x64_sys_sendmmsg+0x53/0x60 net/socket.c:2579
>   do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>   do_syscall_64+0x44/0xd0 arch/x86/entry/common.c:80
>   entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> read to 0xffff888133d9a9f8 of 1 bytes by task 23563 on cpu 1:
>   __veth_xdp_flush drivers/net/veth.c:268 [inline]
>   veth_xmit+0x2d6/0x470 drivers/net/veth.c:350
>   __netdev_start_xmit include/linux/netdevice.h:4683 [inline]
>   netdev_start_xmit include/linux/netdevice.h:4697 [inline]
>   xmit_one+0x105/0x2f0 net/core/dev.c:3473
>   dev_hard_start_xmit net/core/dev.c:3489 [inline]
>   __dev_queue_xmit+0x86d/0xf90 net/core/dev.c:4116
>   dev_queue_xmit+0x13/0x20 net/core/dev.c:4149
>   br_dev_queue_push_xmit+0x3ce/0x430 net/bridge/br_forward.c:53
>   NF_HOOK include/linux/netfilter.h:307 [inline]
>   br_forward_finish net/bridge/br_forward.c:66 [inline]
>   NF_HOOK include/linux/netfilter.h:307 [inline]
>   __br_forward+0x2e4/0x400 net/bridge/br_forward.c:115
>   br_flood+0x521/0x5c0 net/bridge/br_forward.c:242
>   br_dev_xmit+0x8b6/0x960
>   __netdev_start_xmit include/linux/netdevice.h:4683 [inline]
>   netdev_start_xmit include/linux/netdevice.h:4697 [inline]
>   xmit_one+0x105/0x2f0 net/core/dev.c:3473
>   dev_hard_start_xmit net/core/dev.c:3489 [inline]
>   __dev_queue_xmit+0x86d/0xf90 net/core/dev.c:4116
>   dev_queue_xmit+0x13/0x20 net/core/dev.c:4149
>   neigh_hh_output include/net/neighbour.h:525 [inline]
>   neigh_output include/net/neighbour.h:539 [inline]
>   ip_finish_output2+0x6f8/0xb70 net/ipv4/ip_output.c:228
>   ip_finish_output+0xfb/0x240 net/ipv4/ip_output.c:316
>   NF_HOOK_COND include/linux/netfilter.h:296 [inline]
>   ip_output+0xf3/0x1a0 net/ipv4/ip_output.c:430
>   dst_output include/net/dst.h:451 [inline]
>   ip_local_out net/ipv4/ip_output.c:126 [inline]
>   ip_send_skb+0x6e/0xe0 net/ipv4/ip_output.c:1570
>   udp_send_skb+0x641/0x880 net/ipv4/udp.c:967
>   udp_sendmsg+0x12ea/0x14c0 net/ipv4/udp.c:1254
>   inet_sendmsg+0x5f/0x80 net/ipv4/af_inet.c:819
>   sock_sendmsg_nosec net/socket.c:705 [inline]
>   sock_sendmsg net/socket.c:725 [inline]
>   ____sys_sendmsg+0x39a/0x510 net/socket.c:2413
>   ___sys_sendmsg net/socket.c:2467 [inline]
>   __sys_sendmmsg+0x267/0x4c0 net/socket.c:2553
>   __do_sys_sendmmsg net/socket.c:2582 [inline]
>   __se_sys_sendmmsg net/socket.c:2579 [inline]
>   __x64_sys_sendmmsg+0x53/0x60 net/socket.c:2579
>   do_syscall_x64 arch/x86/entry/common.c:50 [inline]
>   do_syscall_64+0x44/0xd0 arch/x86/entry/common.c:80
>   entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> value changed: 0x00 -> 0x01

I'm not familiar with KCSAN.
Does this mean rx_notify_masked value is changed while another CPU is reading it?

If so, I'm not sure there is a problem with that.
At least we could call napi_schedule() twice, but that just causes one extra napi 
poll due to NAPIF_STATE_MISSED, and it happens very rarely?

Toshiaki Makita