From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH v2 net] net: fix deadlock while clearing neighbor proxy table Date: Thu, 12 Apr 2018 22:02:13 -0400 (EDT) Message-ID: <20180412.220213.1847984035228162004.davem@davemloft.net> References: <20180412084655.12607-1-w.bumiller@proxmox.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, yoshfuji@linux-ipv6.org To: w.bumiller@proxmox.com Return-path: Received: from shards.monkeyblade.net ([184.105.139.130]:60732 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753112AbeDMCCO (ORCPT ); Thu, 12 Apr 2018 22:02:14 -0400 In-Reply-To: <20180412084655.12607-1-w.bumiller@proxmox.com> Sender: netdev-owner@vger.kernel.org List-ID: From: Wolfgang Bumiller Date: Thu, 12 Apr 2018 10:46:55 +0200 > 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 > --- > Changes to v1: > * Renamed 'pneigh_ifdown' to 'pneigh_ifdown_and_unlock'. Applied and queued up for -stable. > Btw. I'm not actually sure how much sense the Fixes tag makes as the > commit itself isn't wrong, it just happens to be the most easily > triggerable code path there (and I can't definitively rule out others, > given that the "sending something over the network with the lock held > will deadlock" comment at the top of the file is also from the initial > commit I'd expect other backtraces to be possible from out of that > function) and the other affected lines are mostly the initial git > commit... Understood.