b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08
@ 2016-07-08  9:49 Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 1/7] batman-adv: Avoid nullptr dereference in bla after vlan_insert_tag Simon Wunderlich
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Simon Wunderlich @ 2016-07-08  9:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n

Hi David,

here are some more bugfix patches which we would like to have integrated
into net, if that is still possible!

Please pull or let me know of any problem!

Thank you,
      Simon

Sven Eckelmann (7):
  batman-adv: Avoid nullptr dereference in bla after vlan_insert_tag
  batman-adv: Avoid nullptr dereference in dat after vlan_insert_tag
  batman-adv: Fix orig_node_vlan leak on orig_node_release
  batman-adv: Fix non-atomic bla_claim::backbone_gw access
  batman-adv: Fix reference leak in batadv_find_router
  batman-adv: Free last_bonding_candidate on release of orig_node
  batman-adv: Fix speedy join in gateway client mode

 net/batman-adv/bridge_loop_avoidance.c | 116 ++++++++++++++++++++++++++-------
 net/batman-adv/distributed-arp-table.c |  10 ++-
 net/batman-adv/originator.c            |  15 +++++
 net/batman-adv/routing.c               |  52 +++++++++++----
 net/batman-adv/send.c                  |   4 +-
 net/batman-adv/types.h                 |   6 +-
 6 files changed, 161 insertions(+), 42 deletions(-)

-- 
2.8.1

The following changes since commit 5b4d10f5e0369ed79434593b7cd8e85eebbe473f:

  qlcnic: use the correct ring in qlcnic_83xx_process_rcv_ring_diag() (2016-06-29 05:46:16 -0400)

are available in the git repository at:

  git://git.open-mesh.org/linux-merge.git tags/batadv-net-for-davem-20160708

for you to fetch changes up to d1fe176ca51fa3cb35f70c1d876d9a090e9befce:

  batman-adv: Fix speedy join in gateway client mode (2016-07-06 16:03:40 +0200)

----------------------------------------------------------------
Here are a couple batman-adv bugfix patches, all by Sven Eckelmann:

 - Fix possible NULL pointer dereference for vlan_insert_tag (two patches)

 - Fix reference handling in some features, which may lead to reference
   leaks or invalid memory access (four patches)

 - Fix speedy join: DHCP packets handled by the gateway feature should
   be sent with 4-address unicast instead of 3-address unicast to make
   speedy join work. This fixes/speeds up DHCP assignment for clients
   which join a mesh for the first time. (one patch)

----------------------------------------------------------------
Sven Eckelmann (7):
      batman-adv: Avoid nullptr dereference in bla after vlan_insert_tag
      batman-adv: Avoid nullptr dereference in dat after vlan_insert_tag
      batman-adv: Fix orig_node_vlan leak on orig_node_release
      batman-adv: Fix non-atomic bla_claim::backbone_gw access
      batman-adv: Fix reference leak in batadv_find_router
      batman-adv: Free last_bonding_candidate on release of orig_node
      batman-adv: Fix speedy join in gateway client mode

 net/batman-adv/bridge_loop_avoidance.c | 116 ++++++++++++++++++++++++++-------
 net/batman-adv/distributed-arp-table.c |  10 ++-
 net/batman-adv/originator.c            |  15 +++++
 net/batman-adv/routing.c               |  52 +++++++++++----
 net/batman-adv/send.c                  |   4 +-
 net/batman-adv/types.h                 |   6 +-
 6 files changed, 161 insertions(+), 42 deletions(-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [B.A.T.M.A.N.] [PATCH 1/7] batman-adv: Avoid nullptr dereference in bla after vlan_insert_tag
  2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
@ 2016-07-08  9:49 ` Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 2/7] batman-adv: Avoid nullptr dereference in dat " Simon Wunderlich
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2016-07-08  9:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

From: Sven Eckelmann <sven@narfation.org>

vlan_insert_tag can return NULL on errors. The bridge loop avoidance code
therefore has to check the return value of vlan_insert_tag for NULL before
it can safely operate on this pointer.

Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bridge_loop_avoidance.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 748a9ea..7129780 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -418,9 +418,12 @@ static void batadv_bla_send_claim(struct batadv_priv *bat_priv, u8 *mac,
 		break;
 	}
 
-	if (vid & BATADV_VLAN_HAS_TAG)
+	if (vid & BATADV_VLAN_HAS_TAG) {
 		skb = vlan_insert_tag(skb, htons(ETH_P_8021Q),
 				      vid & VLAN_VID_MASK);
+		if (!skb)
+			goto out;
+	}
 
 	skb_reset_mac_header(skb);
 	skb->protocol = eth_type_trans(skb, soft_iface);
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [B.A.T.M.A.N.] [PATCH 2/7] batman-adv: Avoid nullptr dereference in dat after vlan_insert_tag
  2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 1/7] batman-adv: Avoid nullptr dereference in bla after vlan_insert_tag Simon Wunderlich
@ 2016-07-08  9:49 ` Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 3/7] batman-adv: Fix orig_node_vlan leak on orig_node_release Simon Wunderlich
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2016-07-08  9:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

From: Sven Eckelmann <sven@narfation.org>

vlan_insert_tag can return NULL on errors. The distributed arp table code
therefore has to check the return value of vlan_insert_tag for NULL before
it can safely operate on this pointer.

Fixes: be1db4f6615b ("batman-adv: make the Distributed ARP Table vlan aware")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/distributed-arp-table.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 278800a..aee3b39 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -1009,9 +1009,12 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
 		if (!skb_new)
 			goto out;
 
-		if (vid & BATADV_VLAN_HAS_TAG)
+		if (vid & BATADV_VLAN_HAS_TAG) {
 			skb_new = vlan_insert_tag(skb_new, htons(ETH_P_8021Q),
 						  vid & VLAN_VID_MASK);
+			if (!skb_new)
+				goto out;
+		}
 
 		skb_reset_mac_header(skb_new);
 		skb_new->protocol = eth_type_trans(skb_new,
@@ -1089,9 +1092,12 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
 	 */
 	skb_reset_mac_header(skb_new);
 
-	if (vid & BATADV_VLAN_HAS_TAG)
+	if (vid & BATADV_VLAN_HAS_TAG) {
 		skb_new = vlan_insert_tag(skb_new, htons(ETH_P_8021Q),
 					  vid & VLAN_VID_MASK);
+		if (!skb_new)
+			goto out;
+	}
 
 	/* To preserve backwards compatibility, the node has choose the outgoing
 	 * format based on the incoming request packet type. The assumption is
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [B.A.T.M.A.N.] [PATCH 3/7] batman-adv: Fix orig_node_vlan leak on orig_node_release
  2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 1/7] batman-adv: Avoid nullptr dereference in bla after vlan_insert_tag Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 2/7] batman-adv: Avoid nullptr dereference in dat " Simon Wunderlich
@ 2016-07-08  9:49 ` Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 4/7] batman-adv: Fix non-atomic bla_claim::backbone_gw access Simon Wunderlich
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2016-07-08  9:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

From: Sven Eckelmann <sven@narfation.org>

batadv_orig_node_new uses batadv_orig_node_vlan_new to allocate a new
batadv_orig_node_vlan and add it to batadv_orig_node::vlan_list. References
to this list have also to be cleaned when the batadv_orig_node is removed.

Fixes: 7ea7b4a14275 ("batman-adv: make the TT CRC logic VLAN specific")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/originator.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index 7f51bc2..fe2fcda 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -765,6 +765,7 @@ static void batadv_orig_node_release(struct kref *ref)
 	struct batadv_neigh_node *neigh_node;
 	struct batadv_orig_node *orig_node;
 	struct batadv_orig_ifinfo *orig_ifinfo;
+	struct batadv_orig_node_vlan *vlan;
 
 	orig_node = container_of(ref, struct batadv_orig_node, refcount);
 
@@ -784,6 +785,13 @@ static void batadv_orig_node_release(struct kref *ref)
 	}
 	spin_unlock_bh(&orig_node->neigh_list_lock);
 
+	spin_lock_bh(&orig_node->vlan_list_lock);
+	hlist_for_each_entry_safe(vlan, node_tmp, &orig_node->vlan_list, list) {
+		hlist_del_rcu(&vlan->list);
+		batadv_orig_node_vlan_put(vlan);
+	}
+	spin_unlock_bh(&orig_node->vlan_list_lock);
+
 	/* Free nc_nodes */
 	batadv_nc_purge_orig(orig_node->bat_priv, orig_node, NULL);
 
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [B.A.T.M.A.N.] [PATCH 4/7] batman-adv: Fix non-atomic bla_claim::backbone_gw access
  2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
                   ` (2 preceding siblings ...)
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 3/7] batman-adv: Fix orig_node_vlan leak on orig_node_release Simon Wunderlich
@ 2016-07-08  9:49 ` Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 5/7] batman-adv: Fix reference leak in batadv_find_router Simon Wunderlich
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2016-07-08  9:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

From: Sven Eckelmann <sven@narfation.org>

The pointer batadv_bla_claim::backbone_gw can be changed at any time.
Therefore, access to it must be protected to ensure that two function
accessing the same backbone_gw are actually accessing the same. This is
especially important when the crc_lock is used or when the backbone_gw of a
claim is exchanged.

Not doing so leads to invalid memory access and/or reference leaks.

Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
Fixes: 5a1dd8a4773d ("batman-adv: lock crc access in bridge loop avoidance")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bridge_loop_avoidance.c | 111 ++++++++++++++++++++++++++-------
 net/batman-adv/types.h                 |   2 +
 2 files changed, 90 insertions(+), 23 deletions(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 7129780..825a5cd 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -177,10 +177,21 @@ static void batadv_backbone_gw_put(struct batadv_bla_backbone_gw *backbone_gw)
 static void batadv_claim_release(struct kref *ref)
 {
 	struct batadv_bla_claim *claim;
+	struct batadv_bla_backbone_gw *old_backbone_gw;
 
 	claim = container_of(ref, struct batadv_bla_claim, refcount);
 
-	batadv_backbone_gw_put(claim->backbone_gw);
+	spin_lock_bh(&claim->backbone_lock);
+	old_backbone_gw = claim->backbone_gw;
+	claim->backbone_gw = NULL;
+	spin_unlock_bh(&claim->backbone_lock);
+
+	spin_lock_bh(&old_backbone_gw->crc_lock);
+	old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
+	spin_unlock_bh(&old_backbone_gw->crc_lock);
+
+	batadv_backbone_gw_put(old_backbone_gw);
+
 	kfree_rcu(claim, rcu);
 }
 
@@ -677,8 +688,10 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
 				 const u8 *mac, const unsigned short vid,
 				 struct batadv_bla_backbone_gw *backbone_gw)
 {
+	struct batadv_bla_backbone_gw *old_backbone_gw;
 	struct batadv_bla_claim *claim;
 	struct batadv_bla_claim search_claim;
+	bool remove_crc = false;
 	int hash_added;
 
 	ether_addr_copy(search_claim.addr, mac);
@@ -692,8 +705,10 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
 			return;
 
 		ether_addr_copy(claim->addr, mac);
+		spin_lock_init(&claim->backbone_lock);
 		claim->vid = vid;
 		claim->lasttime = jiffies;
+		kref_get(&backbone_gw->refcount);
 		claim->backbone_gw = backbone_gw;
 
 		kref_init(&claim->refcount);
@@ -721,15 +736,26 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
 			   "bla_add_claim(): changing ownership for %pM, vid %d\n",
 			   mac, BATADV_PRINT_VID(vid));
 
-		spin_lock_bh(&claim->backbone_gw->crc_lock);
-		claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
-		spin_unlock_bh(&claim->backbone_gw->crc_lock);
-		batadv_backbone_gw_put(claim->backbone_gw);
+		remove_crc = true;
 	}
-	/* set (new) backbone gw */
+
+	/* replace backbone_gw atomically and adjust reference counters */
+	spin_lock_bh(&claim->backbone_lock);
+	old_backbone_gw = claim->backbone_gw;
 	kref_get(&backbone_gw->refcount);
 	claim->backbone_gw = backbone_gw;
+	spin_unlock_bh(&claim->backbone_lock);
+
+	if (remove_crc) {
+		/* remove claim address from old backbone_gw */
+		spin_lock_bh(&old_backbone_gw->crc_lock);
+		old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
+		spin_unlock_bh(&old_backbone_gw->crc_lock);
+	}
 
+	batadv_backbone_gw_put(old_backbone_gw);
+
+	/* add claim address to new backbone_gw */
 	spin_lock_bh(&backbone_gw->crc_lock);
 	backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
 	spin_unlock_bh(&backbone_gw->crc_lock);
@@ -740,6 +766,26 @@ claim_free_ref:
 }
 
 /**
+ * batadv_bla_claim_get_backbone_gw - Get valid reference for backbone_gw of
+ *  claim
+ * @claim: claim whose backbone_gw should be returned
+ *
+ * Return: valid reference to claim::backbone_gw
+ */
+static struct batadv_bla_backbone_gw *
+batadv_bla_claim_get_backbone_gw(struct batadv_bla_claim *claim)
+{
+	struct batadv_bla_backbone_gw *backbone_gw;
+
+	spin_lock_bh(&claim->backbone_lock);
+	backbone_gw = claim->backbone_gw;
+	kref_get(&backbone_gw->refcount);
+	spin_unlock_bh(&claim->backbone_lock);
+
+	return backbone_gw;
+}
+
+/**
  * batadv_bla_del_claim - delete a claim from the claim hash
  * @bat_priv: the bat priv with all the soft interface information
  * @mac: mac address of the claim to be removed
@@ -763,10 +809,6 @@ static void batadv_bla_del_claim(struct batadv_priv *bat_priv,
 			   batadv_choose_claim, claim);
 	batadv_claim_put(claim); /* reference from the hash is gone */
 
-	spin_lock_bh(&claim->backbone_gw->crc_lock);
-	claim->backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
-	spin_unlock_bh(&claim->backbone_gw->crc_lock);
-
 	/* don't need the reference from hash_find() anymore */
 	batadv_claim_put(claim);
 }
@@ -1219,6 +1261,7 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
 				    struct batadv_hard_iface *primary_if,
 				    int now)
 {
+	struct batadv_bla_backbone_gw *backbone_gw;
 	struct batadv_bla_claim *claim;
 	struct hlist_head *head;
 	struct batadv_hashtable *hash;
@@ -1233,14 +1276,17 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(claim, head, hash_entry) {
+			backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
 			if (now)
 				goto purge_now;
-			if (!batadv_compare_eth(claim->backbone_gw->orig,
+
+			if (!batadv_compare_eth(backbone_gw->orig,
 						primary_if->net_dev->dev_addr))
-				continue;
+				goto skip;
+
 			if (!batadv_has_timed_out(claim->lasttime,
 						  BATADV_BLA_CLAIM_TIMEOUT))
-				continue;
+				goto skip;
 
 			batadv_dbg(BATADV_DBG_BLA, bat_priv,
 				   "bla_purge_claims(): %pM, vid %d, time out\n",
@@ -1248,8 +1294,10 @@ static void batadv_bla_purge_claims(struct batadv_priv *bat_priv,
 
 purge_now:
 			batadv_handle_unclaim(bat_priv, primary_if,
-					      claim->backbone_gw->orig,
+					      backbone_gw->orig,
 					      claim->addr, claim->vid);
+skip:
+			batadv_backbone_gw_put(backbone_gw);
 		}
 		rcu_read_unlock();
 	}
@@ -1760,9 +1808,11 @@ batadv_bla_loopdetect_check(struct batadv_priv *bat_priv, struct sk_buff *skb,
 bool batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 		   unsigned short vid, bool is_bcast)
 {
+	struct batadv_bla_backbone_gw *backbone_gw;
 	struct ethhdr *ethhdr;
 	struct batadv_bla_claim search_claim, *claim = NULL;
 	struct batadv_hard_iface *primary_if;
+	bool own_claim;
 	bool ret;
 
 	ethhdr = eth_hdr(skb);
@@ -1797,8 +1847,12 @@ bool batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 	}
 
 	/* if it is our own claim ... */
-	if (batadv_compare_eth(claim->backbone_gw->orig,
-			       primary_if->net_dev->dev_addr)) {
+	backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
+	own_claim = batadv_compare_eth(backbone_gw->orig,
+				       primary_if->net_dev->dev_addr);
+	batadv_backbone_gw_put(backbone_gw);
+
+	if (own_claim) {
 		/* ... allow it in any case */
 		claim->lasttime = jiffies;
 		goto allow;
@@ -1862,7 +1916,9 @@ bool batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 {
 	struct ethhdr *ethhdr;
 	struct batadv_bla_claim search_claim, *claim = NULL;
+	struct batadv_bla_backbone_gw *backbone_gw;
 	struct batadv_hard_iface *primary_if;
+	bool client_roamed;
 	bool ret = false;
 
 	primary_if = batadv_primary_if_get_selected(bat_priv);
@@ -1892,8 +1948,12 @@ bool batadv_bla_tx(struct batadv_priv *bat_priv, struct sk_buff *skb,
 		goto allow;
 
 	/* check if we are responsible. */
-	if (batadv_compare_eth(claim->backbone_gw->orig,
-			       primary_if->net_dev->dev_addr)) {
+	backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
+	client_roamed = batadv_compare_eth(backbone_gw->orig,
+					   primary_if->net_dev->dev_addr);
+	batadv_backbone_gw_put(backbone_gw);
+
+	if (client_roamed) {
 		/* if yes, the client has roamed and we have
 		 * to unclaim it.
 		 */
@@ -1941,6 +2001,7 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
 	struct net_device *net_dev = (struct net_device *)seq->private;
 	struct batadv_priv *bat_priv = netdev_priv(net_dev);
 	struct batadv_hashtable *hash = bat_priv->bla.claim_hash;
+	struct batadv_bla_backbone_gw *backbone_gw;
 	struct batadv_bla_claim *claim;
 	struct batadv_hard_iface *primary_if;
 	struct hlist_head *head;
@@ -1965,17 +2026,21 @@ int batadv_bla_claim_table_seq_print_text(struct seq_file *seq, void *offset)
 
 		rcu_read_lock();
 		hlist_for_each_entry_rcu(claim, head, hash_entry) {
-			is_own = batadv_compare_eth(claim->backbone_gw->orig,
+			backbone_gw = batadv_bla_claim_get_backbone_gw(claim);
+
+			is_own = batadv_compare_eth(backbone_gw->orig,
 						    primary_addr);
 
-			spin_lock_bh(&claim->backbone_gw->crc_lock);
-			backbone_crc = claim->backbone_gw->crc;
-			spin_unlock_bh(&claim->backbone_gw->crc_lock);
+			spin_lock_bh(&backbone_gw->crc_lock);
+			backbone_crc = backbone_gw->crc;
+			spin_unlock_bh(&backbone_gw->crc_lock);
 			seq_printf(seq, " * %pM on %5d by %pM [%c] (%#.4x)\n",
 				   claim->addr, BATADV_PRINT_VID(claim->vid),
-				   claim->backbone_gw->orig,
+				   backbone_gw->orig,
 				   (is_own ? 'x' : ' '),
 				   backbone_crc);
+
+			batadv_backbone_gw_put(backbone_gw);
 		}
 		rcu_read_unlock();
 	}
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index ba846b0..0051222 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -1042,6 +1042,7 @@ struct batadv_bla_backbone_gw {
  * @addr: mac address of claimed non-mesh client
  * @vid: vlan id this client was detected on
  * @backbone_gw: pointer to backbone gw claiming this client
+ * @backbone_lock: lock protecting backbone_gw pointer
  * @lasttime: last time we heard of claim (locals only)
  * @hash_entry: hlist node for batadv_priv_bla::claim_hash
  * @refcount: number of contexts the object is used
@@ -1051,6 +1052,7 @@ struct batadv_bla_claim {
 	u8 addr[ETH_ALEN];
 	unsigned short vid;
 	struct batadv_bla_backbone_gw *backbone_gw;
+	spinlock_t backbone_lock; /* protects backbone_gw */
 	unsigned long lasttime;
 	struct hlist_node hash_entry;
 	struct rcu_head rcu;
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [B.A.T.M.A.N.] [PATCH 5/7] batman-adv: Fix reference leak in batadv_find_router
  2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
                   ` (3 preceding siblings ...)
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 4/7] batman-adv: Fix non-atomic bla_claim::backbone_gw access Simon Wunderlich
@ 2016-07-08  9:49 ` Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 6/7] batman-adv: Free last_bonding_candidate on release of orig_node Simon Wunderlich
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2016-07-08  9:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

From: Sven Eckelmann <sven@narfation.org>

The replacement of last_bonding_candidate in batadv_orig_node has to be an
atomic operation. Otherwise it is possible that the reference counter of a
batadv_orig_ifinfo is reduced which was no longer the
last_bonding_candidate when the new candidate is added. This can either
lead to an invalid memory access or to reference leaks which make it
impossible to an interface which was added to batman-adv.

Fixes: f3b3d9018975 ("batman-adv: add bonding again")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/routing.c | 52 ++++++++++++++++++++++++++++++++++++------------
 net/batman-adv/types.h   |  4 +++-
 2 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 6c2901a..bfac086 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -456,6 +456,29 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv,
 }
 
 /**
+ * batadv_last_bonding_replace - Replace last_bonding_candidate of orig_node
+ * @orig_node: originator node whose bonding candidates should be replaced
+ * @new_candidate: new bonding candidate or NULL
+ */
+static void
+batadv_last_bonding_replace(struct batadv_orig_node *orig_node,
+			    struct batadv_orig_ifinfo *new_candidate)
+{
+	struct batadv_orig_ifinfo *old_candidate;
+
+	spin_lock_bh(&orig_node->neigh_list_lock);
+	old_candidate = orig_node->last_bonding_candidate;
+
+	if (new_candidate)
+		kref_get(&new_candidate->refcount);
+	orig_node->last_bonding_candidate = new_candidate;
+	spin_unlock_bh(&orig_node->neigh_list_lock);
+
+	if (old_candidate)
+		batadv_orig_ifinfo_put(old_candidate);
+}
+
+/**
  * batadv_find_router - find a suitable router for this originator
  * @bat_priv: the bat priv with all the soft interface information
  * @orig_node: the destination node
@@ -562,10 +585,6 @@ next:
 	}
 	rcu_read_unlock();
 
-	/* last_bonding_candidate is reset below, remove the old reference. */
-	if (orig_node->last_bonding_candidate)
-		batadv_orig_ifinfo_put(orig_node->last_bonding_candidate);
-
 	/* After finding candidates, handle the three cases:
 	 * 1) there is a next candidate, use that
 	 * 2) there is no next candidate, use the first of the list
@@ -574,21 +593,28 @@ next:
 	if (next_candidate) {
 		batadv_neigh_node_put(router);
 
-		/* remove references to first candidate, we don't need it. */
-		if (first_candidate) {
-			batadv_neigh_node_put(first_candidate_router);
-			batadv_orig_ifinfo_put(first_candidate);
-		}
+		kref_get(&next_candidate_router->refcount);
 		router = next_candidate_router;
-		orig_node->last_bonding_candidate = next_candidate;
+		batadv_last_bonding_replace(orig_node, next_candidate);
 	} else if (first_candidate) {
 		batadv_neigh_node_put(router);
 
-		/* refcounting has already been done in the loop above. */
+		kref_get(&first_candidate_router->refcount);
 		router = first_candidate_router;
-		orig_node->last_bonding_candidate = first_candidate;
+		batadv_last_bonding_replace(orig_node, first_candidate);
 	} else {
-		orig_node->last_bonding_candidate = NULL;
+		batadv_last_bonding_replace(orig_node, NULL);
+	}
+
+	/* cleanup of candidates */
+	if (first_candidate) {
+		batadv_neigh_node_put(first_candidate_router);
+		batadv_orig_ifinfo_put(first_candidate);
+	}
+
+	if (next_candidate) {
+		batadv_neigh_node_put(next_candidate_router);
+		batadv_orig_ifinfo_put(next_candidate);
 	}
 
 	return router;
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 0051222..74d865a 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -330,7 +330,9 @@ struct batadv_orig_node {
 	DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
 	u32 last_bcast_seqno;
 	struct hlist_head neigh_list;
-	/* neigh_list_lock protects: neigh_list and router */
+	/* neigh_list_lock protects: neigh_list, ifinfo_list,
+	 * last_bonding_candidate and router
+	 */
 	spinlock_t neigh_list_lock;
 	struct hlist_node hash_entry;
 	struct batadv_priv *bat_priv;
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [B.A.T.M.A.N.] [PATCH 6/7] batman-adv: Free last_bonding_candidate on release of orig_node
  2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
                   ` (4 preceding siblings ...)
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 5/7] batman-adv: Fix reference leak in batadv_find_router Simon Wunderlich
@ 2016-07-08  9:49 ` Simon Wunderlich
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 7/7] batman-adv: Fix speedy join in gateway client mode Simon Wunderlich
  2016-07-11 19:30 ` [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 David Miller
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2016-07-08  9:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

From: Sven Eckelmann <sven@narfation.org>

The orig_ifinfo reference counter for last_bonding_candidate in
batadv_orig_node has to be reduced when an originator node is released.
Otherwise the orig_ifinfo is leaked and the reference counter the netdevice
is not reduced correctly.

Fixes: f3b3d9018975 ("batman-adv: add bonding again")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/originator.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index fe2fcda..ab8c4f9 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -766,6 +766,7 @@ static void batadv_orig_node_release(struct kref *ref)
 	struct batadv_orig_node *orig_node;
 	struct batadv_orig_ifinfo *orig_ifinfo;
 	struct batadv_orig_node_vlan *vlan;
+	struct batadv_orig_ifinfo *last_candidate;
 
 	orig_node = container_of(ref, struct batadv_orig_node, refcount);
 
@@ -783,8 +784,14 @@ static void batadv_orig_node_release(struct kref *ref)
 		hlist_del_rcu(&orig_ifinfo->list);
 		batadv_orig_ifinfo_put(orig_ifinfo);
 	}
+
+	last_candidate = orig_node->last_bonding_candidate;
+	orig_node->last_bonding_candidate = NULL;
 	spin_unlock_bh(&orig_node->neigh_list_lock);
 
+	if (last_candidate)
+		batadv_orig_ifinfo_put(last_candidate);
+
 	spin_lock_bh(&orig_node->vlan_list_lock);
 	hlist_for_each_entry_safe(vlan, node_tmp, &orig_node->vlan_list, list) {
 		hlist_del_rcu(&vlan->list);
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [B.A.T.M.A.N.] [PATCH 7/7] batman-adv: Fix speedy join in gateway client mode
  2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
                   ` (5 preceding siblings ...)
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 6/7] batman-adv: Free last_bonding_candidate on release of orig_node Simon Wunderlich
@ 2016-07-08  9:49 ` Simon Wunderlich
  2016-07-11 19:30 ` [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 David Miller
  7 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2016-07-08  9:49 UTC (permalink / raw)
  To: davem; +Cc: netdev, b.a.t.m.a.n, Marek Lindner

From: Sven Eckelmann <sven@narfation.org>

Speedy join only works when the received packet is either broadcast or an
4addr unicast packet. Thus packets converted from broadcast to unicast via
the gateway handling code have to be converted to 4addr packets to allow
the receiving gateway server to add the sender address as temporary entry
to the translation table.

Not doing it will make the batman-adv gateway server drop the DHCP response
in many situations because it doesn't yet have the TT entry for the
destination of the DHCP response.

Fixes: 371351731e9c ("batman-adv: change interface_rx to get orig node")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Acked-by: Antonio Quartulli <a@unstable.cc>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/send.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index f2f1256..0103976 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -424,8 +424,8 @@ int batadv_send_skb_via_gw(struct batadv_priv *bat_priv, struct sk_buff *skb,
 	struct batadv_orig_node *orig_node;
 
 	orig_node = batadv_gw_get_selected_orig(bat_priv);
-	return batadv_send_skb_unicast(bat_priv, skb, BATADV_UNICAST, 0,
-				       orig_node, vid);
+	return batadv_send_skb_unicast(bat_priv, skb, BATADV_UNICAST_4ADDR,
+				       BATADV_P_DATA, orig_node, vid);
 }
 
 void batadv_schedule_bat_ogm(struct batadv_hard_iface *hard_iface)
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08
  2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
                   ` (6 preceding siblings ...)
  2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 7/7] batman-adv: Fix speedy join in gateway client mode Simon Wunderlich
@ 2016-07-11 19:30 ` David Miller
  7 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2016-07-11 19:30 UTC (permalink / raw)
  To: sw; +Cc: netdev, b.a.t.m.a.n

From: Simon Wunderlich <sw@simonwunderlich.de>
Date: Fri,  8 Jul 2016 11:49:15 +0200

> here are some more bugfix patches which we would like to have integrated
> into net, if that is still possible!
> 
> Please pull or let me know of any problem!

Pulled, thanks Simon.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-07-11 19:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-08  9:49 [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 Simon Wunderlich
2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 1/7] batman-adv: Avoid nullptr dereference in bla after vlan_insert_tag Simon Wunderlich
2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 2/7] batman-adv: Avoid nullptr dereference in dat " Simon Wunderlich
2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 3/7] batman-adv: Fix orig_node_vlan leak on orig_node_release Simon Wunderlich
2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 4/7] batman-adv: Fix non-atomic bla_claim::backbone_gw access Simon Wunderlich
2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 5/7] batman-adv: Fix reference leak in batadv_find_router Simon Wunderlich
2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 6/7] batman-adv: Free last_bonding_candidate on release of orig_node Simon Wunderlich
2016-07-08  9:49 ` [B.A.T.M.A.N.] [PATCH 7/7] batman-adv: Fix speedy join in gateway client mode Simon Wunderlich
2016-07-11 19:30 ` [B.A.T.M.A.N.] [PATCH 0/7] pull request for net: batman-adv 2016-07-08 David Miller

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).