Skip to content

Commit 93da8d7

Browse files
edumazetdavem330
authored andcommitted
wireguard: use DEV_STATS_INC()
wg_xmit() can be called concurrently, KCSAN reported [1] some device stats updates can be lost. Use DEV_STATS_INC() for this unlikely case. [1] BUG: KCSAN: data-race in wg_xmit / wg_xmit read-write to 0xffff888104239160 of 8 bytes by task 1375 on cpu 0: wg_xmit+0x60f/0x680 drivers/net/wireguard/device.c:231 __netdev_start_xmit include/linux/netdevice.h:4918 [inline] netdev_start_xmit include/linux/netdevice.h:4932 [inline] xmit_one net/core/dev.c:3543 [inline] dev_hard_start_xmit+0x11b/0x3f0 net/core/dev.c:3559 ... read-write to 0xffff888104239160 of 8 bytes by task 1378 on cpu 1: wg_xmit+0x60f/0x680 drivers/net/wireguard/device.c:231 __netdev_start_xmit include/linux/netdevice.h:4918 [inline] netdev_start_xmit include/linux/netdevice.h:4932 [inline] xmit_one net/core/dev.c:3543 [inline] dev_hard_start_xmit+0x11b/0x3f0 net/core/dev.c:3559 ... v2: also change wg_packet_consume_data_done() (Hangbin Liu) and wg_packet_purge_staged_packets() Fixes: e7096c1 ("net: WireGuard secure network tunnel") Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Jason A. Donenfeld <Jason@zx2c4.com> Cc: Hangbin Liu <liuhangbin@gmail.com> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: Hangbin Liu <liuhangbin@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 8ba2c45 commit 93da8d7

File tree

3 files changed

+10
-9
lines changed

3 files changed

+10
-9
lines changed

drivers/net/wireguard/device.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ static netdev_tx_t wg_xmit(struct sk_buff *skb, struct net_device *dev)
210210
*/
211211
while (skb_queue_len(&peer->staged_packet_queue) > MAX_STAGED_PACKETS) {
212212
dev_kfree_skb(__skb_dequeue(&peer->staged_packet_queue));
213-
++dev->stats.tx_dropped;
213+
DEV_STATS_INC(dev, tx_dropped);
214214
}
215215
skb_queue_splice_tail(&packets, &peer->staged_packet_queue);
216216
spin_unlock_bh(&peer->staged_packet_queue.lock);
@@ -228,7 +228,7 @@ static netdev_tx_t wg_xmit(struct sk_buff *skb, struct net_device *dev)
228228
else if (skb->protocol == htons(ETH_P_IPV6))
229229
icmpv6_ndo_send(skb, ICMPV6_DEST_UNREACH, ICMPV6_ADDR_UNREACH, 0);
230230
err:
231-
++dev->stats.tx_errors;
231+
DEV_STATS_INC(dev, tx_errors);
232232
kfree_skb(skb);
233233
return ret;
234234
}

drivers/net/wireguard/receive.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -416,20 +416,20 @@ static void wg_packet_consume_data_done(struct wg_peer *peer,
416416
net_dbg_skb_ratelimited("%s: Packet has unallowed src IP (%pISc) from peer %llu (%pISpfsc)\n",
417417
dev->name, skb, peer->internal_id,
418418
&peer->endpoint.addr);
419-
++dev->stats.rx_errors;
420-
++dev->stats.rx_frame_errors;
419+
DEV_STATS_INC(dev, rx_errors);
420+
DEV_STATS_INC(dev, rx_frame_errors);
421421
goto packet_processed;
422422
dishonest_packet_type:
423423
net_dbg_ratelimited("%s: Packet is neither ipv4 nor ipv6 from peer %llu (%pISpfsc)\n",
424424
dev->name, peer->internal_id, &peer->endpoint.addr);
425-
++dev->stats.rx_errors;
426-
++dev->stats.rx_frame_errors;
425+
DEV_STATS_INC(dev, rx_errors);
426+
DEV_STATS_INC(dev, rx_frame_errors);
427427
goto packet_processed;
428428
dishonest_packet_size:
429429
net_dbg_ratelimited("%s: Packet has incorrect size from peer %llu (%pISpfsc)\n",
430430
dev->name, peer->internal_id, &peer->endpoint.addr);
431-
++dev->stats.rx_errors;
432-
++dev->stats.rx_length_errors;
431+
DEV_STATS_INC(dev, rx_errors);
432+
DEV_STATS_INC(dev, rx_length_errors);
433433
goto packet_processed;
434434
packet_processed:
435435
dev_kfree_skb(skb);

drivers/net/wireguard/send.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,8 @@ static void wg_packet_create_data(struct wg_peer *peer, struct sk_buff *first)
333333
void wg_packet_purge_staged_packets(struct wg_peer *peer)
334334
{
335335
spin_lock_bh(&peer->staged_packet_queue.lock);
336-
peer->device->dev->stats.tx_dropped += peer->staged_packet_queue.qlen;
336+
DEV_STATS_ADD(peer->device->dev, tx_dropped,
337+
peer->staged_packet_queue.qlen);
337338
__skb_queue_purge(&peer->staged_packet_queue);
338339
spin_unlock_bh(&peer->staged_packet_queue.lock);
339340
}

0 commit comments

Comments
 (0)