Commit 8a989617 authored by Eric Dumazet's avatar Eric Dumazet Committed by Jakub Kicinski

net/packet: annotate data-races around tp->status

Another syzbot report [1] is about tp->status lockless reads
from __packet_get_status()

[1]
BUG: KCSAN: data-race in __packet_rcv_has_room / __packet_set_status

write to 0xffff888117d7c080 of 8 bytes by interrupt on cpu 0:
__packet_set_status+0x78/0xa0 net/packet/af_packet.c:407
tpacket_rcv+0x18bb/0x1a60 net/packet/af_packet.c:2483
deliver_skb net/core/dev.c:2173 [inline]
__netif_receive_skb_core+0x408/0x1e80 net/core/dev.c:5337
__netif_receive_skb_one_core net/core/dev.c:5491 [inline]
__netif_receive_skb+0x57/0x1b0 net/core/dev.c:5607
process_backlog+0x21f/0x380 net/core/dev.c:5935
__napi_poll+0x60/0x3b0 net/core/dev.c:6498
napi_poll net/core/dev.c:6565 [inline]
net_rx_action+0x32b/0x750 net/core/dev.c:6698
__do_softirq+0xc1/0x265 kernel/softirq.c:571
invoke_softirq kernel/softirq.c:445 [inline]
__irq_exit_rcu+0x57/0xa0 kernel/softirq.c:650
sysvec_apic_timer_interrupt+0x6d/0x80 arch/x86/kernel/apic/apic.c:1106
asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:645
smpboot_thread_fn+0x33c/0x4a0 kernel/smpboot.c:112
kthread+0x1d7/0x210 kernel/kthread.c:379
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308

read to 0xffff888117d7c080 of 8 bytes by interrupt on cpu 1:
__packet_get_status net/packet/af_packet.c:436 [inline]
packet_lookup_frame net/packet/af_packet.c:524 [inline]
__tpacket_has_room net/packet/af_packet.c:1255 [inline]
__packet_rcv_has_room+0x3f9/0x450 net/packet/af_packet.c:1298
tpacket_rcv+0x275/0x1a60 net/packet/af_packet.c:2285
deliver_skb net/core/dev.c:2173 [inline]
dev_queue_xmit_nit+0x38a/0x5e0 net/core/dev.c:2243
xmit_one net/core/dev.c:3574 [inline]
dev_hard_start_xmit+0xcf/0x3f0 net/core/dev.c:3594
__dev_queue_xmit+0xefb/0x1d10 net/core/dev.c:4244
dev_queue_xmit include/linux/netdevice.h:3088 [inline]
can_send+0x4eb/0x5d0 net/can/af_can.c:276
bcm_can_tx+0x314/0x410 net/can/bcm.c:302
bcm_tx_timeout_handler+0xdb/0x260
__run_hrtimer kernel/time/hrtimer.c:1685 [inline]
__hrtimer_run_queues+0x217/0x700 kernel/time/hrtimer.c:1749
hrtimer_run_softirq+0xd6/0x120 kernel/time/hrtimer.c:1766
__do_softirq+0xc1/0x265 kernel/softirq.c:571
run_ksoftirqd+0x17/0x20 kernel/softirq.c:939
smpboot_thread_fn+0x30a/0x4a0 kernel/smpboot.c:164
kthread+0x1d7/0x210 kernel/kthread.c:379
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308

value changed: 0x0000000000000000 -> 0x0000000020000081

Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 19 Comm: ksoftirqd/1 Not tainted 6.4.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/27/2023

Fixes: 69e3c75f ("net: TX_RING and packet mmap")
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reviewed-by: default avatarWillem de Bruijn <willemb@google.com>
Link: https://lore.kernel.org/r/20230803145600.2937518-1-edumazet@google.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent a94c16a2
...@@ -401,18 +401,20 @@ static void __packet_set_status(struct packet_sock *po, void *frame, int status) ...@@ -401,18 +401,20 @@ static void __packet_set_status(struct packet_sock *po, void *frame, int status)
{ {
union tpacket_uhdr h; union tpacket_uhdr h;
/* WRITE_ONCE() are paired with READ_ONCE() in __packet_get_status */
h.raw = frame; h.raw = frame;
switch (po->tp_version) { switch (po->tp_version) {
case TPACKET_V1: case TPACKET_V1:
h.h1->tp_status = status; WRITE_ONCE(h.h1->tp_status, status);
flush_dcache_page(pgv_to_page(&h.h1->tp_status)); flush_dcache_page(pgv_to_page(&h.h1->tp_status));
break; break;
case TPACKET_V2: case TPACKET_V2:
h.h2->tp_status = status; WRITE_ONCE(h.h2->tp_status, status);
flush_dcache_page(pgv_to_page(&h.h2->tp_status)); flush_dcache_page(pgv_to_page(&h.h2->tp_status));
break; break;
case TPACKET_V3: case TPACKET_V3:
h.h3->tp_status = status; WRITE_ONCE(h.h3->tp_status, status);
flush_dcache_page(pgv_to_page(&h.h3->tp_status)); flush_dcache_page(pgv_to_page(&h.h3->tp_status));
break; break;
default: default:
...@@ -429,17 +431,19 @@ static int __packet_get_status(const struct packet_sock *po, void *frame) ...@@ -429,17 +431,19 @@ static int __packet_get_status(const struct packet_sock *po, void *frame)
smp_rmb(); smp_rmb();
/* READ_ONCE() are paired with WRITE_ONCE() in __packet_set_status */
h.raw = frame; h.raw = frame;
switch (po->tp_version) { switch (po->tp_version) {
case TPACKET_V1: case TPACKET_V1:
flush_dcache_page(pgv_to_page(&h.h1->tp_status)); flush_dcache_page(pgv_to_page(&h.h1->tp_status));
return h.h1->tp_status; return READ_ONCE(h.h1->tp_status);
case TPACKET_V2: case TPACKET_V2:
flush_dcache_page(pgv_to_page(&h.h2->tp_status)); flush_dcache_page(pgv_to_page(&h.h2->tp_status));
return h.h2->tp_status; return READ_ONCE(h.h2->tp_status);
case TPACKET_V3: case TPACKET_V3:
flush_dcache_page(pgv_to_page(&h.h3->tp_status)); flush_dcache_page(pgv_to_page(&h.h3->tp_status));
return h.h3->tp_status; return READ_ONCE(h.h3->tp_status);
default: default:
WARN(1, "TPACKET version not supported.\n"); WARN(1, "TPACKET version not supported.\n");
BUG(); BUG();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment