From 9650388b5c56578fdccc79c57a8c82fb92b8e7f1 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Fri, 21 Dec 2012 07:32:10 +0000 Subject: ipv4: arp: fix a lockdep splat in arp_solicit() Yan Burman reported following lockdep warning : ============================================= [ INFO: possible recursive locking detected ] 3.7.0+ #24 Not tainted --------------------------------------------- swapper/1/0 is trying to acquire lock: (&n->lock){++--..}, at: [] __neigh_event_send +0x2e/0x2f0 but task is already holding lock: (&n->lock){++--..}, at: [] arp_solicit+0x1d4/0x280 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&n->lock); lock(&n->lock); *** DEADLOCK *** May be due to missing lock nesting notation 4 locks held by swapper/1/0: #0: (((&n->timer))){+.-...}, at: [] call_timer_fn+0x0/0x1c0 #1: (&n->lock){++--..}, at: [] arp_solicit +0x1d4/0x280 #2: (rcu_read_lock_bh){.+....}, at: [] dev_queue_xmit+0x0/0x5d0 #3: (rcu_read_lock_bh){.+....}, at: [] ip_finish_output+0x13e/0x640 stack backtrace: Pid: 0, comm: swapper/1 Not tainted 3.7.0+ #24 Call Trace: [] validate_chain+0xdcc/0x11f0 [] ? __lock_acquire+0x440/0xc30 [] ? kmem_cache_free+0xe5/0x1c0 [] __lock_acquire+0x440/0xc30 [] ? inet_getpeer+0x40/0x600 [] ? __lock_acquire+0x440/0xc30 [] ? __neigh_event_send+0x2e/0x2f0 [] lock_acquire+0x95/0x140 [] ? __neigh_event_send+0x2e/0x2f0 [] ? __lock_acquire+0x440/0xc30 [] _raw_write_lock_bh+0x3b/0x50 [] ? __neigh_event_send+0x2e/0x2f0 [] __neigh_event_send+0x2e/0x2f0 [] neigh_resolve_output+0x16b/0x270 [] ip_finish_output+0x34d/0x640 [] ? ip_finish_output+0x13e/0x640 [] ? vxlan_xmit+0x556/0xbec [vxlan] [] ip_output+0x80/0xf0 [] ip_local_out+0x28/0x80 [] vxlan_xmit+0x66a/0xbec [vxlan] [] ? vxlan_xmit+0x556/0xbec [vxlan] [] ? skb_gso_segment+0x2b0/0x2b0 [] ? _raw_spin_unlock_irqrestore+0x65/0x80 [] ? dev_queue_xmit_nit+0x207/0x270 [] dev_hard_start_xmit+0x298/0x5d0 [] dev_queue_xmit+0x2f3/0x5d0 [] ? dev_hard_start_xmit+0x5d0/0x5d0 [] arp_xmit+0x58/0x60 [] arp_send+0x3b/0x40 [] arp_solicit+0x204/0x280 [] ? neigh_add+0x310/0x310 [] neigh_probe+0x45/0x70 [] neigh_timer_handler+0x1a0/0x2a0 [] call_timer_fn+0x7f/0x1c0 [] ? detach_if_pending+0x120/0x120 [] run_timer_softirq+0x238/0x2b0 [] ? neigh_add+0x310/0x310 [] __do_softirq+0x101/0x280 [] call_softirq+0x1c/0x30 [] do_softirq+0x85/0xc0 [] irq_exit+0x9e/0xc0 [] smp_apic_timer_interrupt+0x68/0xa0 [] apic_timer_interrupt+0x6f/0x80 [] ? mwait_idle+0xa4/0x1c0 [] ? mwait_idle+0x9b/0x1c0 [] cpu_idle+0x89/0xe0 [] start_secondary+0x1b2/0x1b6 Bug is from arp_solicit(), releasing the neigh lock after arp_send() In case of vxlan, we eventually need to write lock a neigh lock later. Its a false positive, but we can get rid of it without lockdep annotations. We can instead use neigh_ha_snapshot() helper. Reported-by: Yan Burman Signed-off-by: Eric Dumazet Acked-by: Stephen Hemminger Signed-off-by: David S. Miller --- net/ipv4/arp.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c index ce6fbdfd40b8..1169ed41d587 100644 --- a/net/ipv4/arp.c +++ b/net/ipv4/arp.c @@ -321,7 +321,7 @@ static void arp_error_report(struct neighbour *neigh, struct sk_buff *skb) static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb) { __be32 saddr = 0; - u8 *dst_ha = NULL; + u8 dst_ha[MAX_ADDR_LEN]; struct net_device *dev = neigh->dev; __be32 target = *(__be32 *)neigh->primary_key; int probes = atomic_read(&neigh->probes); @@ -363,9 +363,9 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb) if (probes < 0) { if (!(neigh->nud_state & NUD_VALID)) pr_debug("trying to ucast probe in NUD_INVALID\n"); - dst_ha = neigh->ha; - read_lock_bh(&neigh->lock); + neigh_ha_snapshot(dst_ha, neigh, dev); } else { + memset(dst_ha, 0, dev->addr_len); probes -= neigh->parms->app_probes; if (probes < 0) { #ifdef CONFIG_ARPD @@ -377,8 +377,6 @@ static void arp_solicit(struct neighbour *neigh, struct sk_buff *skb) arp_send(ARPOP_REQUEST, ETH_P_ARP, target, dev, saddr, dst_ha, dev->dev_addr, NULL); - if (dst_ha) - read_unlock_bh(&neigh->lock); } static int arp_ignore(struct in_device *in_dev, __be32 sip, __be32 tip) -- cgit v1.2.3