Message ID | 20191009161913.18600-1-edumazet@google.com |
---|---|
State | Awaiting Upstream |
Delegated to: | Pablo Neira |
Headers | show |
Series | [net] netfilter: conntrack: avoid possible false sharing | expand |
On Wed, Oct 09, 2019 at 09:19:13AM -0700, Eric Dumazet wrote: > As hinted by KCSAN, we need at least one READ_ONCE() > to prevent a compiler optimization. > > More details on : > https://github.com/google/ktsan/wiki/READ_ONCE-and-WRITE_ONCE#it-may-improve-performance > > sysbot report : > BUG: KCSAN: data-race in __nf_ct_refresh_acct / __nf_ct_refresh_acct > > read to 0xffff888123eb4f08 of 4 bytes by interrupt on cpu 0: > __nf_ct_refresh_acct+0xd4/0x1b0 net/netfilter/nf_conntrack_core.c:1796 > nf_ct_refresh_acct include/net/netfilter/nf_conntrack.h:201 [inline] > nf_conntrack_tcp_packet+0xd40/0x3390 net/netfilter/nf_conntrack_proto_tcp.c:1161 > nf_conntrack_handle_packet net/netfilter/nf_conntrack_core.c:1633 [inline] > nf_conntrack_in+0x410/0xaa0 net/netfilter/nf_conntrack_core.c:1727 > ipv4_conntrack_in+0x27/0x40 net/netfilter/nf_conntrack_proto.c:178 > nf_hook_entry_hookfn include/linux/netfilter.h:135 [inline] > nf_hook_slow+0x83/0x160 net/netfilter/core.c:512 > nf_hook include/linux/netfilter.h:260 [inline] > NF_HOOK include/linux/netfilter.h:303 [inline] > ip_rcv+0x12f/0x1a0 net/ipv4/ip_input.c:523 > __netif_receive_skb_one_core+0xa7/0xe0 net/core/dev.c:5004 > __netif_receive_skb+0x37/0xf0 net/core/dev.c:5118 > netif_receive_skb_internal+0x59/0x190 net/core/dev.c:5208 > napi_skb_finish net/core/dev.c:5671 [inline] > napi_gro_receive+0x28f/0x330 net/core/dev.c:5704 > receive_buf+0x284/0x30b0 drivers/net/virtio_net.c:1061 > virtnet_receive drivers/net/virtio_net.c:1323 [inline] > virtnet_poll+0x436/0x7d0 drivers/net/virtio_net.c:1428 > napi_poll net/core/dev.c:6352 [inline] > net_rx_action+0x3ae/0xa50 net/core/dev.c:6418 > __do_softirq+0x115/0x33f kernel/softirq.c:292 > > write to 0xffff888123eb4f08 of 4 bytes by task 7191 on cpu 1: > __nf_ct_refresh_acct+0xfb/0x1b0 net/netfilter/nf_conntrack_core.c:1797 > nf_ct_refresh_acct include/net/netfilter/nf_conntrack.h:201 [inline] > nf_conntrack_tcp_packet+0xd40/0x3390 net/netfilter/nf_conntrack_proto_tcp.c:1161 > nf_conntrack_handle_packet net/netfilter/nf_conntrack_core.c:1633 [inline] > nf_conntrack_in+0x410/0xaa0 net/netfilter/nf_conntrack_core.c:1727 > ipv4_conntrack_local+0xbe/0x130 net/netfilter/nf_conntrack_proto.c:200 > nf_hook_entry_hookfn include/linux/netfilter.h:135 [inline] > nf_hook_slow+0x83/0x160 net/netfilter/core.c:512 > nf_hook include/linux/netfilter.h:260 [inline] > __ip_local_out+0x1f7/0x2b0 net/ipv4/ip_output.c:114 > ip_local_out+0x31/0x90 net/ipv4/ip_output.c:123 > __ip_queue_xmit+0x3a8/0xa40 net/ipv4/ip_output.c:532 > ip_queue_xmit+0x45/0x60 include/net/ip.h:236 > __tcp_transmit_skb+0xdeb/0x1cd0 net/ipv4/tcp_output.c:1158 > __tcp_send_ack+0x246/0x300 net/ipv4/tcp_output.c:3685 > tcp_send_ack+0x34/0x40 net/ipv4/tcp_output.c:3691 > tcp_cleanup_rbuf+0x130/0x360 net/ipv4/tcp.c:1575 > > Reported by Kernel Concurrency Sanitizer on: > CPU: 1 PID: 7191 Comm: syz-fuzzer Not tainted 5.3.0+ #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 > > Fixes: cc16921351d8 ("netfilter: conntrack: avoid same-timeout update") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: syzbot <syzkaller@googlegroups.com> > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > Cc: Jozsef Kadlecsik <kadlec@netfilter.org> > Cc: Florian Westphal <fw@strlen.de> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org> Thanks. > --- > net/netfilter/nf_conntrack_core.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c > index 0c63120b2db2e1ea9983a6b1ce8d2aefebc29501..5cd610b547e0d1e3463a65ba3627f265c836bdc5 100644 > --- a/net/netfilter/nf_conntrack_core.c > +++ b/net/netfilter/nf_conntrack_core.c > @@ -1792,8 +1792,8 @@ void __nf_ct_refresh_acct(struct nf_conn *ct, > if (nf_ct_is_confirmed(ct)) > extra_jiffies += nfct_time_stamp; > > - if (ct->timeout != extra_jiffies) > - ct->timeout = extra_jiffies; > + if (READ_ONCE(ct->timeout) != extra_jiffies) > + WRITE_ONCE(ct->timeout, extra_jiffies); > acct: > if (do_acct) > nf_ct_acct_update(ct, ctinfo, skb->len); > -- > 2.23.0.581.g78d2f28ef7-goog >
On Wed, 9 Oct 2019 09:19:13 -0700, Eric Dumazet wrote: > As hinted by KCSAN, we need at least one READ_ONCE() > to prevent a compiler optimization. > > More details on : > https://github.com/google/ktsan/wiki/READ_ONCE-and-WRITE_ONCE#it-may-improve-performance > > [...] > > Fixes: cc16921351d8 ("netfilter: conntrack: avoid same-timeout update") > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: syzbot <syzkaller@googlegroups.com> > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > Cc: Jozsef Kadlecsik <kadlec@netfilter.org> > Cc: Florian Westphal <fw@strlen.de> Applied, thank you. Not queuing for stable, please let me know if I should.
On 10/9/19 9:24 PM, Jakub Kicinski wrote: > On Wed, 9 Oct 2019 09:19:13 -0700, Eric Dumazet wrote: >> As hinted by KCSAN, we need at least one READ_ONCE() >> to prevent a compiler optimization. >> >> More details on : >> https://github.com/google/ktsan/wiki/READ_ONCE-and-WRITE_ONCE#it-may-improve-performance >> >> [...] >> >> Fixes: cc16921351d8 ("netfilter: conntrack: avoid same-timeout update") >> Signed-off-by: Eric Dumazet <edumazet@google.com> >> Reported-by: syzbot <syzkaller@googlegroups.com> >> Cc: Pablo Neira Ayuso <pablo@netfilter.org> >> Cc: Jozsef Kadlecsik <kadlec@netfilter.org> >> Cc: Florian Westphal <fw@strlen.de> > > Applied, thank you. > > Not queuing for stable, please let me know if I should. > This is fine really, thanks.
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index 0c63120b2db2e1ea9983a6b1ce8d2aefebc29501..5cd610b547e0d1e3463a65ba3627f265c836bdc5 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -1792,8 +1792,8 @@ void __nf_ct_refresh_acct(struct nf_conn *ct, if (nf_ct_is_confirmed(ct)) extra_jiffies += nfct_time_stamp; - if (ct->timeout != extra_jiffies) - ct->timeout = extra_jiffies; + if (READ_ONCE(ct->timeout) != extra_jiffies) + WRITE_ONCE(ct->timeout, extra_jiffies); acct: if (do_acct) nf_ct_acct_update(ct, ctinfo, skb->len);
As hinted by KCSAN, we need at least one READ_ONCE() to prevent a compiler optimization. More details on : https://github.com/google/ktsan/wiki/READ_ONCE-and-WRITE_ONCE#it-may-improve-performance sysbot report : BUG: KCSAN: data-race in __nf_ct_refresh_acct / __nf_ct_refresh_acct read to 0xffff888123eb4f08 of 4 bytes by interrupt on cpu 0: __nf_ct_refresh_acct+0xd4/0x1b0 net/netfilter/nf_conntrack_core.c:1796 nf_ct_refresh_acct include/net/netfilter/nf_conntrack.h:201 [inline] nf_conntrack_tcp_packet+0xd40/0x3390 net/netfilter/nf_conntrack_proto_tcp.c:1161 nf_conntrack_handle_packet net/netfilter/nf_conntrack_core.c:1633 [inline] nf_conntrack_in+0x410/0xaa0 net/netfilter/nf_conntrack_core.c:1727 ipv4_conntrack_in+0x27/0x40 net/netfilter/nf_conntrack_proto.c:178 nf_hook_entry_hookfn include/linux/netfilter.h:135 [inline] nf_hook_slow+0x83/0x160 net/netfilter/core.c:512 nf_hook include/linux/netfilter.h:260 [inline] NF_HOOK include/linux/netfilter.h:303 [inline] ip_rcv+0x12f/0x1a0 net/ipv4/ip_input.c:523 __netif_receive_skb_one_core+0xa7/0xe0 net/core/dev.c:5004 __netif_receive_skb+0x37/0xf0 net/core/dev.c:5118 netif_receive_skb_internal+0x59/0x190 net/core/dev.c:5208 napi_skb_finish net/core/dev.c:5671 [inline] napi_gro_receive+0x28f/0x330 net/core/dev.c:5704 receive_buf+0x284/0x30b0 drivers/net/virtio_net.c:1061 virtnet_receive drivers/net/virtio_net.c:1323 [inline] virtnet_poll+0x436/0x7d0 drivers/net/virtio_net.c:1428 napi_poll net/core/dev.c:6352 [inline] net_rx_action+0x3ae/0xa50 net/core/dev.c:6418 __do_softirq+0x115/0x33f kernel/softirq.c:292 write to 0xffff888123eb4f08 of 4 bytes by task 7191 on cpu 1: __nf_ct_refresh_acct+0xfb/0x1b0 net/netfilter/nf_conntrack_core.c:1797 nf_ct_refresh_acct include/net/netfilter/nf_conntrack.h:201 [inline] nf_conntrack_tcp_packet+0xd40/0x3390 net/netfilter/nf_conntrack_proto_tcp.c:1161 nf_conntrack_handle_packet net/netfilter/nf_conntrack_core.c:1633 [inline] nf_conntrack_in+0x410/0xaa0 net/netfilter/nf_conntrack_core.c:1727 ipv4_conntrack_local+0xbe/0x130 net/netfilter/nf_conntrack_proto.c:200 nf_hook_entry_hookfn include/linux/netfilter.h:135 [inline] nf_hook_slow+0x83/0x160 net/netfilter/core.c:512 nf_hook include/linux/netfilter.h:260 [inline] __ip_local_out+0x1f7/0x2b0 net/ipv4/ip_output.c:114 ip_local_out+0x31/0x90 net/ipv4/ip_output.c:123 __ip_queue_xmit+0x3a8/0xa40 net/ipv4/ip_output.c:532 ip_queue_xmit+0x45/0x60 include/net/ip.h:236 __tcp_transmit_skb+0xdeb/0x1cd0 net/ipv4/tcp_output.c:1158 __tcp_send_ack+0x246/0x300 net/ipv4/tcp_output.c:3685 tcp_send_ack+0x34/0x40 net/ipv4/tcp_output.c:3691 tcp_cleanup_rbuf+0x130/0x360 net/ipv4/tcp.c:1575 Reported by Kernel Concurrency Sanitizer on: CPU: 1 PID: 7191 Comm: syz-fuzzer Not tainted 5.3.0+ #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Fixes: cc16921351d8 ("netfilter: conntrack: avoid same-timeout update") Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: syzbot <syzkaller@googlegroups.com> Cc: Pablo Neira Ayuso <pablo@netfilter.org> Cc: Jozsef Kadlecsik <kadlec@netfilter.org> Cc: Florian Westphal <fw@strlen.de> --- net/netfilter/nf_conntrack_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)