linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Andrew Collins <acollins@cradlepoint.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: [PATCH 4.4 11/34] net: Add netdev all_adj_list refcnt propagation to fix panic
Date: Sun, 13 Nov 2016 12:24:43 +0100	[thread overview]
Message-ID: <20161113112400.559395258@linuxfoundation.org> (raw)
In-Reply-To: <20161113112400.008903838@linuxfoundation.org>

4.4-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Andrew Collins <acollins@cradlepoint.com>


[ Upstream commit 93409033ae653f1c9a949202fb537ab095b2092f ]

This is a respin of a patch to fix a relatively easily reproducible kernel
panic related to the all_adj_list handling for netdevs in recent kernels.

The following sequence of commands will reproduce the issue:

ip link add link eth0 name eth0.100 type vlan id 100
ip link add link eth0 name eth0.200 type vlan id 200
ip link add name testbr type bridge
ip link set eth0.100 master testbr
ip link set eth0.200 master testbr
ip link add link testbr mac0 type macvlan
ip link delete dev testbr

This creates an upper/lower tree of (excuse the poor ASCII art):

            /---eth0.100-eth0
mac0-testbr-
            \---eth0.200-eth0

When testbr is deleted, the all_adj_lists are walked, and eth0 is deleted twice from
the mac0 list. Unfortunately, during setup in __netdev_upper_dev_link, only one
reference to eth0 is added, so this results in a panic.

This change adds reference count propagation so things are handled properly.

Matthias Schiffer reported a similar crash in batman-adv:

https://github.com/freifunk-gluon/gluon/issues/680
https://www.open-mesh.org/issues/247

which this patch also seems to resolve.

Signed-off-by: Andrew Collins <acollins@cradlepoint.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 net/core/dev.c |   68 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 37 insertions(+), 31 deletions(-)

--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5204,6 +5204,7 @@ static inline bool netdev_adjacent_is_ne
 
 static int __netdev_adjacent_dev_insert(struct net_device *dev,
 					struct net_device *adj_dev,
+					u16 ref_nr,
 					struct list_head *dev_list,
 					void *private, bool master)
 {
@@ -5213,7 +5214,7 @@ static int __netdev_adjacent_dev_insert(
 	adj = __netdev_find_adj(adj_dev, dev_list);
 
 	if (adj) {
-		adj->ref_nr++;
+		adj->ref_nr += ref_nr;
 		return 0;
 	}
 
@@ -5223,7 +5224,7 @@ static int __netdev_adjacent_dev_insert(
 
 	adj->dev = adj_dev;
 	adj->master = master;
-	adj->ref_nr = 1;
+	adj->ref_nr = ref_nr;
 	adj->private = private;
 	dev_hold(adj_dev);
 
@@ -5262,6 +5263,7 @@ free_adj:
 
 static void __netdev_adjacent_dev_remove(struct net_device *dev,
 					 struct net_device *adj_dev,
+					 u16 ref_nr,
 					 struct list_head *dev_list)
 {
 	struct netdev_adjacent *adj;
@@ -5274,10 +5276,10 @@ static void __netdev_adjacent_dev_remove
 		BUG();
 	}
 
-	if (adj->ref_nr > 1) {
-		pr_debug("%s to %s ref_nr-- = %d\n", dev->name, adj_dev->name,
-			 adj->ref_nr-1);
-		adj->ref_nr--;
+	if (adj->ref_nr > ref_nr) {
+		pr_debug("%s to %s ref_nr-%d = %d\n", dev->name, adj_dev->name,
+			 ref_nr, adj->ref_nr-ref_nr);
+		adj->ref_nr -= ref_nr;
 		return;
 	}
 
@@ -5296,21 +5298,22 @@ static void __netdev_adjacent_dev_remove
 
 static int __netdev_adjacent_dev_link_lists(struct net_device *dev,
 					    struct net_device *upper_dev,
+					    u16 ref_nr,
 					    struct list_head *up_list,
 					    struct list_head *down_list,
 					    void *private, bool master)
 {
 	int ret;
 
-	ret = __netdev_adjacent_dev_insert(dev, upper_dev, up_list, private,
-					   master);
+	ret = __netdev_adjacent_dev_insert(dev, upper_dev, ref_nr, up_list,
+					   private, master);
 	if (ret)
 		return ret;
 
-	ret = __netdev_adjacent_dev_insert(upper_dev, dev, down_list, private,
-					   false);
+	ret = __netdev_adjacent_dev_insert(upper_dev, dev, ref_nr, down_list,
+					   private, false);
 	if (ret) {
-		__netdev_adjacent_dev_remove(dev, upper_dev, up_list);
+		__netdev_adjacent_dev_remove(dev, upper_dev, ref_nr, up_list);
 		return ret;
 	}
 
@@ -5318,9 +5321,10 @@ static int __netdev_adjacent_dev_link_li
 }
 
 static int __netdev_adjacent_dev_link(struct net_device *dev,
-				      struct net_device *upper_dev)
+				      struct net_device *upper_dev,
+				      u16 ref_nr)
 {
-	return __netdev_adjacent_dev_link_lists(dev, upper_dev,
+	return __netdev_adjacent_dev_link_lists(dev, upper_dev, ref_nr,
 						&dev->all_adj_list.upper,
 						&upper_dev->all_adj_list.lower,
 						NULL, false);
@@ -5328,17 +5332,19 @@ static int __netdev_adjacent_dev_link(st
 
 static void __netdev_adjacent_dev_unlink_lists(struct net_device *dev,
 					       struct net_device *upper_dev,
+					       u16 ref_nr,
 					       struct list_head *up_list,
 					       struct list_head *down_list)
 {
-	__netdev_adjacent_dev_remove(dev, upper_dev, up_list);
-	__netdev_adjacent_dev_remove(upper_dev, dev, down_list);
+	__netdev_adjacent_dev_remove(dev, upper_dev, ref_nr, up_list);
+	__netdev_adjacent_dev_remove(upper_dev, dev, ref_nr, down_list);
 }
 
 static void __netdev_adjacent_dev_unlink(struct net_device *dev,
-					 struct net_device *upper_dev)
+					 struct net_device *upper_dev,
+					 u16 ref_nr)
 {
-	__netdev_adjacent_dev_unlink_lists(dev, upper_dev,
+	__netdev_adjacent_dev_unlink_lists(dev, upper_dev, ref_nr,
 					   &dev->all_adj_list.upper,
 					   &upper_dev->all_adj_list.lower);
 }
@@ -5347,17 +5353,17 @@ static int __netdev_adjacent_dev_link_ne
 						struct net_device *upper_dev,
 						void *private, bool master)
 {
-	int ret = __netdev_adjacent_dev_link(dev, upper_dev);
+	int ret = __netdev_adjacent_dev_link(dev, upper_dev, 1);
 
 	if (ret)
 		return ret;
 
-	ret = __netdev_adjacent_dev_link_lists(dev, upper_dev,
+	ret = __netdev_adjacent_dev_link_lists(dev, upper_dev, 1,
 					       &dev->adj_list.upper,
 					       &upper_dev->adj_list.lower,
 					       private, master);
 	if (ret) {
-		__netdev_adjacent_dev_unlink(dev, upper_dev);
+		__netdev_adjacent_dev_unlink(dev, upper_dev, 1);
 		return ret;
 	}
 
@@ -5367,8 +5373,8 @@ static int __netdev_adjacent_dev_link_ne
 static void __netdev_adjacent_dev_unlink_neighbour(struct net_device *dev,
 						   struct net_device *upper_dev)
 {
-	__netdev_adjacent_dev_unlink(dev, upper_dev);
-	__netdev_adjacent_dev_unlink_lists(dev, upper_dev,
+	__netdev_adjacent_dev_unlink(dev, upper_dev, 1);
+	__netdev_adjacent_dev_unlink_lists(dev, upper_dev, 1,
 					   &dev->adj_list.upper,
 					   &upper_dev->adj_list.lower);
 }
@@ -5420,7 +5426,7 @@ static int __netdev_upper_dev_link(struc
 		list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
 			pr_debug("Interlinking %s with %s, non-neighbour\n",
 				 i->dev->name, j->dev->name);
-			ret = __netdev_adjacent_dev_link(i->dev, j->dev);
+			ret = __netdev_adjacent_dev_link(i->dev, j->dev, i->ref_nr);
 			if (ret)
 				goto rollback_mesh;
 		}
@@ -5430,7 +5436,7 @@ static int __netdev_upper_dev_link(struc
 	list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
 		pr_debug("linking %s's upper device %s with %s\n",
 			 upper_dev->name, i->dev->name, dev->name);
-		ret = __netdev_adjacent_dev_link(dev, i->dev);
+		ret = __netdev_adjacent_dev_link(dev, i->dev, i->ref_nr);
 		if (ret)
 			goto rollback_upper_mesh;
 	}
@@ -5439,7 +5445,7 @@ static int __netdev_upper_dev_link(struc
 	list_for_each_entry(i, &dev->all_adj_list.lower, list) {
 		pr_debug("linking %s's lower device %s with %s\n", dev->name,
 			 i->dev->name, upper_dev->name);
-		ret = __netdev_adjacent_dev_link(i->dev, upper_dev);
+		ret = __netdev_adjacent_dev_link(i->dev, upper_dev, i->ref_nr);
 		if (ret)
 			goto rollback_lower_mesh;
 	}
@@ -5453,7 +5459,7 @@ rollback_lower_mesh:
 	list_for_each_entry(i, &dev->all_adj_list.lower, list) {
 		if (i == to_i)
 			break;
-		__netdev_adjacent_dev_unlink(i->dev, upper_dev);
+		__netdev_adjacent_dev_unlink(i->dev, upper_dev, i->ref_nr);
 	}
 
 	i = NULL;
@@ -5463,7 +5469,7 @@ rollback_upper_mesh:
 	list_for_each_entry(i, &upper_dev->all_adj_list.upper, list) {
 		if (i == to_i)
 			break;
-		__netdev_adjacent_dev_unlink(dev, i->dev);
+		__netdev_adjacent_dev_unlink(dev, i->dev, i->ref_nr);
 	}
 
 	i = j = NULL;
@@ -5475,7 +5481,7 @@ rollback_mesh:
 		list_for_each_entry(j, &upper_dev->all_adj_list.upper, list) {
 			if (i == to_i && j == to_j)
 				break;
-			__netdev_adjacent_dev_unlink(i->dev, j->dev);
+			__netdev_adjacent_dev_unlink(i->dev, j->dev, i->ref_nr);
 		}
 		if (i == to_i)
 			break;
@@ -5559,16 +5565,16 @@ void netdev_upper_dev_unlink(struct net_
 	 */
 	list_for_each_entry(i, &dev->all_adj_list.lower, list)
 		list_for_each_entry(j, &upper_dev->all_adj_list.upper, list)
-			__netdev_adjacent_dev_unlink(i->dev, j->dev);
+			__netdev_adjacent_dev_unlink(i->dev, j->dev, i->ref_nr);
 
 	/* remove also the devices itself from lower/upper device
 	 * list
 	 */
 	list_for_each_entry(i, &dev->all_adj_list.lower, list)
-		__netdev_adjacent_dev_unlink(i->dev, upper_dev);
+		__netdev_adjacent_dev_unlink(i->dev, upper_dev, i->ref_nr);
 
 	list_for_each_entry(i, &upper_dev->all_adj_list.upper, list)
-		__netdev_adjacent_dev_unlink(dev, i->dev);
+		__netdev_adjacent_dev_unlink(dev, i->dev, i->ref_nr);
 
 	call_netdevice_notifiers_info(NETDEV_CHANGEUPPER, dev,
 				      &changeupper_info.info);

  parent reply	other threads:[~2016-11-13 11:39 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20161113112504epcas1p23b8af4aabfbd40c07be5d48fcfd80e44@epcas1p2.samsung.com>
2016-11-13 11:24 ` [PATCH 4.4 00/34] 4.4.32-stable review Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 01/34] tcp: fix overflow in __tcp_retransmit_skb() Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 02/34] net: avoid sk_forward_alloc overflows Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 03/34] tcp: fix wrong checksum calculation on MTU probing Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 04/34] tcp: fix a compile error in DBGUNDO() Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 05/34] ip6_gre: fix flowi6_proto value in ip6gre_xmit_other() Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 06/34] ipmr, ip6mr: fix scheduling while atomic and a deadlock with ipmr_get_route Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 07/34] tg3: Avoid NULL pointer dereference in tg3_io_error_detected() Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 09/34] net: pktgen: fix pkt_size Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 10/34] net/sched: act_vlan: Push skb->data to mac_header prior calling skb_vlan_*() functions Greg Kroah-Hartman
2016-11-14  7:07     ` Shmulik Ladkani
2016-11-13 11:24   ` Greg Kroah-Hartman [this message]
2016-11-13 11:24   ` [PATCH 4.4 12/34] packet: call fanout_release, while UNREGISTERING a netdev Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 13/34] netlink: do not enter direct reclaim from netlink_dump() Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 14/34] ipv6: tcp: restore IP6CB for pktoptions skbs Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 15/34] ip6_tunnel: fix ip6_tnl_lookup Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 16/34] ipv6: correctly add local routes when lo goes up Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 17/34] net: pktgen: remove rcu locking in pktgen_change_name() Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 18/34] bridge: multicast: restore perm router ports on multicast enable Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 19/34] rtnetlink: Add rtnexthop offload flag to compare mask Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 21/34] ipv4: disable BH in set_ping_group_range() Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 22/34] ipv4: use the right lock for ping_group_range Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 23/34] net: sctp, forbid negative length Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 24/34] udp: fix IP_CHECKSUM handling Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 25/34] net sched filters: fix notification of filter delete with proper handle Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 26/34] sctp: validate chunk len before actually using it Greg Kroah-Hartman
2016-11-13 11:24   ` [PATCH 4.4 27/34] packet: on direct_xmit, limit tso and csum to supported devices Greg Kroah-Hartman
2016-11-13 11:25   ` [PATCH 4.4 32/34] drm/amdgpu: fix DP mode validation Greg Kroah-Hartman
2016-11-13 11:25   ` [PATCH 4.4 34/34] drm/radeon: " Greg Kroah-Hartman
2016-11-13 20:41   ` [PATCH 4.4 00/34] 4.4.32-stable review Guenter Roeck
     [not found]   ` <5828d40c.212dc20a.9adc7.8f2f@mx.google.com>
2016-11-14  7:53     ` Greg Kroah-Hartman
2016-11-14 16:47   ` Shuah Khan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161113112400.559395258@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=acollins@cradlepoint.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).