| From 88833c4c6d54d1d5f08bfc7a1adebbded27cf03c Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 25 Oct 2021 11:15:55 -0700 |
| Subject: net: annotate data-race in neigh_output() |
| |
| From: Eric Dumazet <edumazet@google.com> |
| |
| [ Upstream commit d18785e213866935b4c3dc0c33c3e18801ce0ce8 ] |
| |
| neigh_output() reads n->nud_state and hh->hh_len locklessly. |
| |
| This is fine, but we need to add annotations and document this. |
| |
| We evaluate skip_cache first to avoid reading these fields |
| if the cache has to by bypassed. |
| |
| syzbot report: |
| |
| BUG: KCSAN: data-race in __neigh_event_send / ip_finish_output2 |
| |
| write to 0xffff88810798a885 of 1 bytes by interrupt on cpu 1: |
| __neigh_event_send+0x40d/0xac0 net/core/neighbour.c:1128 |
| neigh_event_send include/net/neighbour.h:444 [inline] |
| neigh_resolve_output+0x104/0x410 net/core/neighbour.c:1476 |
| neigh_output include/net/neighbour.h:510 [inline] |
| ip_finish_output2+0x80a/0xaa0 net/ipv4/ip_output.c:221 |
| ip_finish_output+0x3b5/0x510 net/ipv4/ip_output.c:309 |
| NF_HOOK_COND include/linux/netfilter.h:296 [inline] |
| ip_output+0xf3/0x1a0 net/ipv4/ip_output.c:423 |
| dst_output include/net/dst.h:450 [inline] |
| ip_local_out+0x164/0x220 net/ipv4/ip_output.c:126 |
| __ip_queue_xmit+0x9d3/0xa20 net/ipv4/ip_output.c:525 |
| ip_queue_xmit+0x34/0x40 net/ipv4/ip_output.c:539 |
| __tcp_transmit_skb+0x142a/0x1a00 net/ipv4/tcp_output.c:1405 |
| tcp_transmit_skb net/ipv4/tcp_output.c:1423 [inline] |
| tcp_xmit_probe_skb net/ipv4/tcp_output.c:4011 [inline] |
| tcp_write_wakeup+0x4a9/0x810 net/ipv4/tcp_output.c:4064 |
| tcp_send_probe0+0x2c/0x2b0 net/ipv4/tcp_output.c:4079 |
| tcp_probe_timer net/ipv4/tcp_timer.c:398 [inline] |
| tcp_write_timer_handler+0x394/0x520 net/ipv4/tcp_timer.c:626 |
| tcp_write_timer+0xb9/0x180 net/ipv4/tcp_timer.c:642 |
| call_timer_fn+0x2e/0x1d0 kernel/time/timer.c:1421 |
| expire_timers+0x135/0x240 kernel/time/timer.c:1466 |
| __run_timers+0x368/0x430 kernel/time/timer.c:1734 |
| run_timer_softirq+0x19/0x30 kernel/time/timer.c:1747 |
| __do_softirq+0x12c/0x26e kernel/softirq.c:558 |
| invoke_softirq kernel/softirq.c:432 [inline] |
| __irq_exit_rcu kernel/softirq.c:636 [inline] |
| irq_exit_rcu+0x4e/0xa0 kernel/softirq.c:648 |
| sysvec_apic_timer_interrupt+0x69/0x80 arch/x86/kernel/apic/apic.c:1097 |
| asm_sysvec_apic_timer_interrupt+0x12/0x20 |
| native_safe_halt arch/x86/include/asm/irqflags.h:51 [inline] |
| arch_safe_halt arch/x86/include/asm/irqflags.h:89 [inline] |
| acpi_safe_halt drivers/acpi/processor_idle.c:109 [inline] |
| acpi_idle_do_entry drivers/acpi/processor_idle.c:553 [inline] |
| acpi_idle_enter+0x258/0x2e0 drivers/acpi/processor_idle.c:688 |
| cpuidle_enter_state+0x2b4/0x760 drivers/cpuidle/cpuidle.c:237 |
| cpuidle_enter+0x3c/0x60 drivers/cpuidle/cpuidle.c:351 |
| call_cpuidle kernel/sched/idle.c:158 [inline] |
| cpuidle_idle_call kernel/sched/idle.c:239 [inline] |
| do_idle+0x1a3/0x250 kernel/sched/idle.c:306 |
| cpu_startup_entry+0x15/0x20 kernel/sched/idle.c:403 |
| secondary_startup_64_no_verify+0xb1/0xbb |
| |
| read to 0xffff88810798a885 of 1 bytes by interrupt on cpu 0: |
| neigh_output include/net/neighbour.h:507 [inline] |
| ip_finish_output2+0x79a/0xaa0 net/ipv4/ip_output.c:221 |
| ip_finish_output+0x3b5/0x510 net/ipv4/ip_output.c:309 |
| NF_HOOK_COND include/linux/netfilter.h:296 [inline] |
| ip_output+0xf3/0x1a0 net/ipv4/ip_output.c:423 |
| dst_output include/net/dst.h:450 [inline] |
| ip_local_out+0x164/0x220 net/ipv4/ip_output.c:126 |
| __ip_queue_xmit+0x9d3/0xa20 net/ipv4/ip_output.c:525 |
| ip_queue_xmit+0x34/0x40 net/ipv4/ip_output.c:539 |
| __tcp_transmit_skb+0x142a/0x1a00 net/ipv4/tcp_output.c:1405 |
| tcp_transmit_skb net/ipv4/tcp_output.c:1423 [inline] |
| tcp_xmit_probe_skb net/ipv4/tcp_output.c:4011 [inline] |
| tcp_write_wakeup+0x4a9/0x810 net/ipv4/tcp_output.c:4064 |
| tcp_send_probe0+0x2c/0x2b0 net/ipv4/tcp_output.c:4079 |
| tcp_probe_timer net/ipv4/tcp_timer.c:398 [inline] |
| tcp_write_timer_handler+0x394/0x520 net/ipv4/tcp_timer.c:626 |
| tcp_write_timer+0xb9/0x180 net/ipv4/tcp_timer.c:642 |
| call_timer_fn+0x2e/0x1d0 kernel/time/timer.c:1421 |
| expire_timers+0x135/0x240 kernel/time/timer.c:1466 |
| __run_timers+0x368/0x430 kernel/time/timer.c:1734 |
| run_timer_softirq+0x19/0x30 kernel/time/timer.c:1747 |
| __do_softirq+0x12c/0x26e kernel/softirq.c:558 |
| invoke_softirq kernel/softirq.c:432 [inline] |
| __irq_exit_rcu kernel/softirq.c:636 [inline] |
| irq_exit_rcu+0x4e/0xa0 kernel/softirq.c:648 |
| sysvec_apic_timer_interrupt+0x69/0x80 arch/x86/kernel/apic/apic.c:1097 |
| asm_sysvec_apic_timer_interrupt+0x12/0x20 |
| native_safe_halt arch/x86/include/asm/irqflags.h:51 [inline] |
| arch_safe_halt arch/x86/include/asm/irqflags.h:89 [inline] |
| acpi_safe_halt drivers/acpi/processor_idle.c:109 [inline] |
| acpi_idle_do_entry drivers/acpi/processor_idle.c:553 [inline] |
| acpi_idle_enter+0x258/0x2e0 drivers/acpi/processor_idle.c:688 |
| cpuidle_enter_state+0x2b4/0x760 drivers/cpuidle/cpuidle.c:237 |
| cpuidle_enter+0x3c/0x60 drivers/cpuidle/cpuidle.c:351 |
| call_cpuidle kernel/sched/idle.c:158 [inline] |
| cpuidle_idle_call kernel/sched/idle.c:239 [inline] |
| do_idle+0x1a3/0x250 kernel/sched/idle.c:306 |
| cpu_startup_entry+0x15/0x20 kernel/sched/idle.c:403 |
| rest_init+0xee/0x100 init/main.c:734 |
| arch_call_rest_init+0xa/0xb |
| start_kernel+0x5e4/0x669 init/main.c:1142 |
| secondary_startup_64_no_verify+0xb1/0xbb |
| |
| value changed: 0x20 -> 0x01 |
| |
| Reported by Kernel Concurrency Sanitizer on: |
| CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.15.0-rc6-syzkaller #0 |
| Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 |
| |
| 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> |
| --- |
| include/net/neighbour.h | 11 ++++++++--- |
| 1 file changed, 8 insertions(+), 3 deletions(-) |
| |
| diff --git a/include/net/neighbour.h b/include/net/neighbour.h |
| index 2be8d6b0dfb69..4232bc8ce3d7d 100644 |
| --- a/include/net/neighbour.h |
| +++ b/include/net/neighbour.h |
| @@ -505,10 +505,15 @@ static inline int neigh_output(struct neighbour *n, struct sk_buff *skb, |
| { |
| const struct hh_cache *hh = &n->hh; |
| |
| - if ((n->nud_state & NUD_CONNECTED) && hh->hh_len && !skip_cache) |
| + /* n->nud_state and hh->hh_len could be changed under us. |
| + * neigh_hh_output() is taking care of the race later. |
| + */ |
| + if (!skip_cache && |
| + (READ_ONCE(n->nud_state) & NUD_CONNECTED) && |
| + READ_ONCE(hh->hh_len)) |
| return neigh_hh_output(hh, skb); |
| - else |
| - return n->output(n, skb); |
| + |
| + return n->output(n, skb); |
| } |
| |
| static inline struct neighbour * |
| -- |
| 2.33.0 |
| |
