From patchwork Tue Apr 10 09:15:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Wolfgang Bumiller X-Patchwork-Id: 896551 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=proxmox.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 40L1r06n3Bz9s15 for ; Tue, 10 Apr 2018 19:21:04 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751991AbeDJJVC (ORCPT ); Tue, 10 Apr 2018 05:21:02 -0400 Received: from proxmox-new.maurer-it.com ([212.186.127.180]:59270 "EHLO proxmox-new.maurer-it.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751630AbeDJJVC (ORCPT ); Tue, 10 Apr 2018 05:21:02 -0400 X-Greylist: delayed 344 seconds by postgrey-1.27 at vger.kernel.org; Tue, 10 Apr 2018 05:21:01 EDT Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 98ABD40727; Tue, 10 Apr 2018 11:15:16 +0200 (CEST) From: Wolfgang Bumiller To: netdev@vger.kernel.org Cc: "David S. Miller" , Hideaki YOSHIFUJI Subject: [PATCH linux] net: fix deadlock while clearing neighbor proxy table Date: Tue, 10 Apr 2018 11:15:14 +0200 Message-Id: <20180410091514.28704-1-w.bumiller@proxmox.com> X-Mailer: git-send-email 2.11.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org When coming from ndisc_netdev_event() in net/ipv6/ndisc.c, neigh_ifdown() is called with &nd_tbl, locking this while clearing the proxy neighbor entries when eg. deleting an interface. Calling the table's pndisc_destructor() with the lock still held, however, can cause a deadlock: When a multicast listener is available an IGMP packet of type ICMPV6_MGM_REDUCTION may be sent out. When reaching ip6_finish_output2(), if no neighbor entry for the target address is found, __neigh_create() is called with &nd_tbl, which it'll want to lock. Move the elements into their own list, then unlock the table and perform the destruction. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199289 Fixes: 6fd6ce2056de ("ipv6: Do not depend on rt->n in ip6_finish_output2().") Signed-off-by: Wolfgang Bumiller --- I do feel bad about moving the unlock call. Perhaps returning the freelist and dealing with it in neigh_ifdown() would be better? net/core/neighbour.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 7b7a14abba28..601df647588c 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -292,7 +292,6 @@ int neigh_ifdown(struct neigh_table *tbl, struct net_device *dev) write_lock_bh(&tbl->lock); neigh_flush_dev(tbl, dev); pneigh_ifdown(tbl, dev); - write_unlock_bh(&tbl->lock); del_timer_sync(&tbl->proxy_timer); pneigh_queue_purge(&tbl->proxy_queue); @@ -683,7 +682,7 @@ int pneigh_delete(struct neigh_table *tbl, struct net *net, const void *pkey, static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev) { - struct pneigh_entry *n, **np; + struct pneigh_entry *n, **np, *freelist = NULL; u32 h; for (h = 0; h <= PNEIGH_HASHMASK; h++) { @@ -691,16 +690,23 @@ static int pneigh_ifdown(struct neigh_table *tbl, struct net_device *dev) while ((n = *np) != NULL) { if (!dev || n->dev == dev) { *np = n->next; - if (tbl->pdestructor) - tbl->pdestructor(n); - if (n->dev) - dev_put(n->dev); - kfree(n); + n->next = freelist; + freelist = n; continue; } np = &n->next; } } + write_unlock_bh(&tbl->lock); + while ((n = freelist)) { + freelist = n->next; + n->next = NULL; + if (tbl->pdestructor) + tbl->pdestructor(n); + if (n->dev) + dev_put(n->dev); + kfree(n); + } return -ENOENT; }