Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net] netfilter: conntrack: avoid possible false sharing
@ 2019-10-09 16:19 Eric Dumazet
  2019-10-09 20:58 ` Pablo Neira Ayuso
  2019-10-10  4:24 ` Jakub Kicinski
  0 siblings, 2 replies; 4+ messages in thread
From: Eric Dumazet @ 2019-10-09 16:19 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
  Cc: netfilter-devel, netdev, Eric Dumazet, Eric Dumazet, syzbot,
	Jozsef Kadlecsik

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(-)

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


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

* Re: [PATCH net] netfilter: conntrack: avoid possible false sharing
  2019-10-09 16:19 [PATCH net] netfilter: conntrack: avoid possible false sharing Eric Dumazet
@ 2019-10-09 20:58 ` Pablo Neira Ayuso
  2019-10-10  4:24 ` Jakub Kicinski
  1 sibling, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2019-10-09 20:58 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Jozsef Kadlecsik, Florian Westphal, netfilter-devel, netdev,
	Eric Dumazet, syzbot, Jozsef Kadlecsik

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
> 

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

* Re: [PATCH net] netfilter: conntrack: avoid possible false sharing
  2019-10-09 16:19 [PATCH net] netfilter: conntrack: avoid possible false sharing Eric Dumazet
  2019-10-09 20:58 ` Pablo Neira Ayuso
@ 2019-10-10  4:24 ` Jakub Kicinski
  2019-10-10  4:30   ` Eric Dumazet
  1 sibling, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2019-10-10  4:24 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	netfilter-devel, netdev, Eric Dumazet, syzbot, Jozsef Kadlecsik

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.

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

* Re: [PATCH net] netfilter: conntrack: avoid possible false sharing
  2019-10-10  4:24 ` Jakub Kicinski
@ 2019-10-10  4:30   ` Eric Dumazet
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2019-10-10  4:30 UTC (permalink / raw)
  To: Jakub Kicinski, Eric Dumazet
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	netfilter-devel, netdev, Eric Dumazet, syzbot, Jozsef Kadlecsik



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.

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 16:19 [PATCH net] netfilter: conntrack: avoid possible false sharing Eric Dumazet
2019-10-09 20:58 ` Pablo Neira Ayuso
2019-10-10  4:24 ` Jakub Kicinski
2019-10-10  4:30   ` Eric Dumazet

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org netdev@archiver.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox