Skip to content

Commit 3900f92

Browse files
edumazetgregkh
authored andcommitted
net: add annotations on hh->hh_len lockless accesses
[ Upstream commit c305c6a ] KCSAN reported a data-race [1] While we can use READ_ONCE() on the read sides, we need to make sure hh->hh_len is written last. [1] BUG: KCSAN: data-race in eth_header_cache / neigh_resolve_output write to 0xffff8880b9dedcb8 of 4 bytes by task 29760 on cpu 0: eth_header_cache+0xa9/0xd0 net/ethernet/eth.c:247 neigh_hh_init net/core/neighbour.c:1463 [inline] neigh_resolve_output net/core/neighbour.c:1480 [inline] neigh_resolve_output+0x415/0x470 net/core/neighbour.c:1470 neigh_output include/net/neighbour.h:511 [inline] ip6_finish_output2+0x7a2/0xec0 net/ipv6/ip6_output.c:116 __ip6_finish_output net/ipv6/ip6_output.c:142 [inline] __ip6_finish_output+0x2d7/0x330 net/ipv6/ip6_output.c:127 ip6_finish_output+0x41/0x160 net/ipv6/ip6_output.c:152 NF_HOOK_COND include/linux/netfilter.h:294 [inline] ip6_output+0xf2/0x280 net/ipv6/ip6_output.c:175 dst_output include/net/dst.h:436 [inline] NF_HOOK include/linux/netfilter.h:305 [inline] ndisc_send_skb+0x459/0x5f0 net/ipv6/ndisc.c:505 ndisc_send_ns+0x207/0x430 net/ipv6/ndisc.c:647 rt6_probe_deferred+0x98/0xf0 net/ipv6/route.c:615 process_one_work+0x3d4/0x890 kernel/workqueue.c:2269 worker_thread+0xa0/0x800 kernel/workqueue.c:2415 kthread+0x1d4/0x200 drivers/block/aoe/aoecmd.c:1253 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:352 read to 0xffff8880b9dedcb8 of 4 bytes by task 29572 on cpu 1: neigh_resolve_output net/core/neighbour.c:1479 [inline] neigh_resolve_output+0x113/0x470 net/core/neighbour.c:1470 neigh_output include/net/neighbour.h:511 [inline] ip6_finish_output2+0x7a2/0xec0 net/ipv6/ip6_output.c:116 __ip6_finish_output net/ipv6/ip6_output.c:142 [inline] __ip6_finish_output+0x2d7/0x330 net/ipv6/ip6_output.c:127 ip6_finish_output+0x41/0x160 net/ipv6/ip6_output.c:152 NF_HOOK_COND include/linux/netfilter.h:294 [inline] ip6_output+0xf2/0x280 net/ipv6/ip6_output.c:175 dst_output include/net/dst.h:436 [inline] NF_HOOK include/linux/netfilter.h:305 [inline] ndisc_send_skb+0x459/0x5f0 net/ipv6/ndisc.c:505 ndisc_send_ns+0x207/0x430 net/ipv6/ndisc.c:647 rt6_probe_deferred+0x98/0xf0 net/ipv6/route.c:615 process_one_work+0x3d4/0x890 kernel/workqueue.c:2269 worker_thread+0xa0/0x800 kernel/workqueue.c:2415 kthread+0x1d4/0x200 drivers/block/aoe/aoecmd.c:1253 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:352 Reported by Kernel Concurrency Sanitizer on: CPU: 1 PID: 29572 Comm: kworker/1:4 Not tainted 5.4.0-rc6+ #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: events rt6_probe_deferred Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent b8233f7 commit 3900f92

File tree

4 files changed

+14
-5
lines changed

4 files changed

+14
-5
lines changed

drivers/firewire/net.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,11 @@ static int fwnet_header_cache(const struct neighbour *neigh,
250250
h = (struct fwnet_header *)((u8 *)hh->hh_data + HH_DATA_OFF(sizeof(*h)));
251251
h->h_proto = type;
252252
memcpy(h->h_dest, neigh->ha, net->addr_len);
253-
hh->hh_len = FWNET_HLEN;
253+
254+
/* Pairs with the READ_ONCE() in neigh_resolve_output(),
255+
* neigh_hh_output() and neigh_update_hhs().
256+
*/
257+
smp_store_release(&hh->hh_len, FWNET_HLEN);
254258

255259
return 0;
256260
}

include/net/neighbour.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ static inline int neigh_hh_output(const struct hh_cache *hh, struct sk_buff *skb
467467

468468
do {
469469
seq = read_seqbegin(&hh->hh_lock);
470-
hh_len = hh->hh_len;
470+
hh_len = READ_ONCE(hh->hh_len);
471471
if (likely(hh_len <= HH_DATA_MOD)) {
472472
hh_alen = HH_DATA_MOD;
473473

net/core/neighbour.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1194,7 +1194,7 @@ static void neigh_update_hhs(struct neighbour *neigh)
11941194

11951195
if (update) {
11961196
hh = &neigh->hh;
1197-
if (hh->hh_len) {
1197+
if (READ_ONCE(hh->hh_len)) {
11981198
write_seqlock_bh(&hh->hh_lock);
11991199
update(hh, neigh->dev, neigh->ha);
12001200
write_sequnlock_bh(&hh->hh_lock);
@@ -1473,7 +1473,7 @@ int neigh_resolve_output(struct neighbour *neigh, struct sk_buff *skb)
14731473
struct net_device *dev = neigh->dev;
14741474
unsigned int seq;
14751475

1476-
if (dev->header_ops->cache && !neigh->hh.hh_len)
1476+
if (dev->header_ops->cache && !READ_ONCE(neigh->hh.hh_len))
14771477
neigh_hh_init(neigh);
14781478

14791479
do {

net/ethernet/eth.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,12 @@ int eth_header_cache(const struct neighbour *neigh, struct hh_cache *hh, __be16
244244
eth->h_proto = type;
245245
memcpy(eth->h_source, dev->dev_addr, ETH_ALEN);
246246
memcpy(eth->h_dest, neigh->ha, ETH_ALEN);
247-
hh->hh_len = ETH_HLEN;
247+
248+
/* Pairs with READ_ONCE() in neigh_resolve_output(),
249+
* neigh_hh_output() and neigh_update_hhs().
250+
*/
251+
smp_store_release(&hh->hh_len, ETH_HLEN);
252+
248253
return 0;
249254
}
250255
EXPORT_SYMBOL(eth_header_cache);

0 commit comments

Comments
 (0)