From patchwork Fri Dec 21 17:32:10 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 207866 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 0C4D62C0097 for ; Sat, 22 Dec 2012 04:32:31 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751415Ab2LURcP (ORCPT ); Fri, 21 Dec 2012 12:32:15 -0500 Received: from mail-pa0-f44.google.com ([209.85.220.44]:59745 "EHLO mail-pa0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750983Ab2LURcN (ORCPT ); Fri, 21 Dec 2012 12:32:13 -0500 Received: by mail-pa0-f44.google.com with SMTP id hz11so2959896pad.31 for ; Fri, 21 Dec 2012 09:32:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:subject:from:to:cc:content-type:date:message-id :mime-version:x-mailer:content-transfer-encoding; bh=wNhhv2/DsVjiPHgZ90en1INaMXTnVDaDNopxFazBJ88=; b=DBseavOuZ5u1wHQ27oE1dS0CgIInElSUxMsj9PudN7KaxPcposhEod6uISm0tlXijC 0c0aSWEeLEmgUzzgWppqhC8MQ8mZeoKzPS5nHLmYwnTOyHWshOUCc1z/4w7WFbHTxIWX ZHkUErIbar0hB9LSkco6LxVvAfBnjO00h2zoIiroLqRvTaMOXNGt3tu6L5XJQUoeC/ae hTG2OyXCIPWgAcPv3VCyxlWZfRJ/bKxRQf1fPoBMrUkc6sURGO2PL76ozZ6YQEQFKDo1 COfY6debrXlOEomtHFmZ4A6mwnNJTBJoubBGFFU8kBaeZrYYtaHZEv/2uE6oahueepug Qszg== X-Received: by 10.68.197.197 with SMTP id iw5mr41537899pbc.22.1356111132776; Fri, 21 Dec 2012 09:32:12 -0800 (PST) Received: from ?IPv6:2620:0:1000:3304:224:d7ff:fee3:2a94? ([2620:0:1000:3304:224:d7ff:fee3:2a94]) by mx.google.com with ESMTPS id f10sm4166040pav.18.2012.12.21.09.32.11 (version=SSLv3 cipher=OTHER); Fri, 21 Dec 2012 09:32:12 -0800 (PST) Subject: [PATCH] ipv4: arp: fix a lockdep splat in arp_solicit() From: Eric Dumazet To: David Miller Cc: netdev , Yan Burman , Stephen Hemminger Date: Fri, 21 Dec 2012 09:32:10 -0800 Message-ID: <1356111130.21834.7565.camel@edumazet-glaptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Eric Dumazet Yan Burman reported following lockdep warning : --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ============================================= [ 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 --- 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 ce6fbdf..1169ed4 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)