From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 496A6C433F5 for ; Mon, 30 May 2022 13:48:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238092AbiE3NsH (ORCPT ); Mon, 30 May 2022 09:48:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49232 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237993AbiE3NnM (ORCPT ); Mon, 30 May 2022 09:43:12 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EFC6A9A9B0; Mon, 30 May 2022 06:32:07 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 5102460F27; Mon, 30 May 2022 13:32:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 61A66C36AE5; Mon, 30 May 2022 13:32:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1653917526; bh=icdT0kA0pbvTfOxTdquU1wQ7pPlbGcQlXh4mcDcVDi0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Kn9rWWfXMJGdtxWJ+4Ln9vNGYqxgnygI4lbiayqdnTmRI5z3U6fxXksh+ILQRvS8R MVI9b0q6WO+vScyGVOJKzckQqIl7kGV9WS32cxBA/r/MComSPbOXRt+RiAQvWY+rny aPX+mfUPuckvzS2/rYh7RtG3TPZWKRq1cBh6nPbnrKy/pUPDXVkKlVB8aom+EMU0wX zOY0+CAQqn/maTKsOUmkyj7DcNex6UmlMeqh3jscoZut0YVUSX+xxKaHP1243hS7QS S05tqjJpVupF3SEgyksW1wInMVV3dsh71ITQs4QGw54bE4oHhAmUVrJkf5GGc1Jq33 VlqZwpDttkkXg== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Niels Dossche , Paolo Abeni , Jakub Kicinski , Sasha Levin , davem@davemloft.net, edumazet@google.com, yoshfuji@linux-ipv6.org, dsahern@kernel.org, netdev@vger.kernel.org Subject: [PATCH AUTOSEL 5.17 012/135] ipv6: fix locking issues with loops over idev->addr_list Date: Mon, 30 May 2022 09:29:30 -0400 Message-Id: <20220530133133.1931716-12-sashal@kernel.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220530133133.1931716-1-sashal@kernel.org> References: <20220530133133.1931716-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Niels Dossche [ Upstream commit 51454ea42c1ab4e0c2828bb0d4d53957976980de ] idev->addr_list needs to be protected by idev->lock. However, it is not always possible to do so while iterating and performing actions on inet6_ifaddr instances. For example, multiple functions (like addrconf_{join,leave}_anycast) eventually call down to other functions that acquire the idev->lock. The current code temporarily unlocked the idev->lock during the loops, which can cause race conditions. Moving the locks up is also not an appropriate solution as the ordering of lock acquisition will be inconsistent with for example mc_lock. This solution adds an additional field to inet6_ifaddr that is used to temporarily add the instances to a temporary list while holding idev->lock. The temporary list can then be traversed without holding idev->lock. This change was done in two places. In addrconf_ifdown, the list_for_each_entry_safe variant of the list loop is also no longer necessary as there is no deletion within that specific loop. Suggested-by: Paolo Abeni Signed-off-by: Niels Dossche Acked-by: Paolo Abeni Link: https://lore.kernel.org/r/20220403231523.45843-1-dossche.niels@gmail.com Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- include/net/if_inet6.h | 8 ++++++++ net/ipv6/addrconf.c | 30 ++++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h index f026cf08a8e8..471461023443 100644 --- a/include/net/if_inet6.h +++ b/include/net/if_inet6.h @@ -64,6 +64,14 @@ struct inet6_ifaddr { struct hlist_node addr_lst; struct list_head if_list; + /* + * Used to safely traverse idev->addr_list in process context + * if the idev->lock needed to protect idev->addr_list cannot be held. + * In that case, add the items to this list temporarily and iterate + * without holding idev->lock. + * See addrconf_ifdown and dev_forward_change. + */ + struct list_head if_list_aux; struct list_head tmp_list; struct inet6_ifaddr *ifpub; diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 4df84013c4e6..0a9e03465001 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -800,6 +800,7 @@ static void dev_forward_change(struct inet6_dev *idev) { struct net_device *dev; struct inet6_ifaddr *ifa; + LIST_HEAD(tmp_addr_list); if (!idev) return; @@ -818,14 +819,24 @@ static void dev_forward_change(struct inet6_dev *idev) } } + read_lock_bh(&idev->lock); list_for_each_entry(ifa, &idev->addr_list, if_list) { if (ifa->flags&IFA_F_TENTATIVE) continue; + list_add_tail(&ifa->if_list_aux, &tmp_addr_list); + } + read_unlock_bh(&idev->lock); + + while (!list_empty(&tmp_addr_list)) { + ifa = list_first_entry(&tmp_addr_list, + struct inet6_ifaddr, if_list_aux); + list_del(&ifa->if_list_aux); if (idev->cnf.forwarding) addrconf_join_anycast(ifa); else addrconf_leave_anycast(ifa); } + inet6_netconf_notify_devconf(dev_net(dev), RTM_NEWNETCONF, NETCONFA_FORWARDING, dev->ifindex, &idev->cnf); @@ -3730,7 +3741,8 @@ static int addrconf_ifdown(struct net_device *dev, bool unregister) unsigned long event = unregister ? NETDEV_UNREGISTER : NETDEV_DOWN; struct net *net = dev_net(dev); struct inet6_dev *idev; - struct inet6_ifaddr *ifa, *tmp; + struct inet6_ifaddr *ifa; + LIST_HEAD(tmp_addr_list); bool keep_addr = false; bool was_ready; int state, i; @@ -3822,16 +3834,23 @@ static int addrconf_ifdown(struct net_device *dev, bool unregister) write_lock_bh(&idev->lock); } - list_for_each_entry_safe(ifa, tmp, &idev->addr_list, if_list) { + list_for_each_entry(ifa, &idev->addr_list, if_list) + list_add_tail(&ifa->if_list_aux, &tmp_addr_list); + write_unlock_bh(&idev->lock); + + while (!list_empty(&tmp_addr_list)) { struct fib6_info *rt = NULL; bool keep; + ifa = list_first_entry(&tmp_addr_list, + struct inet6_ifaddr, if_list_aux); + list_del(&ifa->if_list_aux); + addrconf_del_dad_work(ifa); keep = keep_addr && (ifa->flags & IFA_F_PERMANENT) && !addr_is_local(&ifa->addr); - write_unlock_bh(&idev->lock); spin_lock_bh(&ifa->lock); if (keep) { @@ -3862,15 +3881,14 @@ static int addrconf_ifdown(struct net_device *dev, bool unregister) addrconf_leave_solict(ifa->idev, &ifa->addr); } - write_lock_bh(&idev->lock); if (!keep) { + write_lock_bh(&idev->lock); list_del_rcu(&ifa->if_list); + write_unlock_bh(&idev->lock); in6_ifa_put(ifa); } } - write_unlock_bh(&idev->lock); - /* Step 5: Discard anycast and multicast list */ if (unregister) { ipv6_ac_destroy_dev(idev); -- 2.35.1