Skip to contentSkip to conversation

pull request net: 20140515

PATCH New Antonio Quartulli 2 participants 8 messages May 15, 2014 – May 16, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

Hello David,

our debugging campaign continues..

Here I have another small set of fixes intended for net/linux-3.15 and *stable* (please enqueue them).

I know we already sent some fixes and we are running late in the rc cycle, but, as you can see from the stats below, the changes are very very small.

Patch 1 prevents a NULL dereference in batadv_orig_hardif_seq_print_text() along the failure path.

Patch 2 fixes a reference counting imbalance that gets triggered everytime the batman packet fragmentation mechanism is used; this imbalance prevents the netdev object held by batman-adv from being released on shutdown.

Patch 3 fixes the reference counting for the orig_node objects in order to avoid any access after they have been free'd.

Patch 4 fixes the TT local check in DAT (Distributed ARP Table) that is used to avoid sending (self-forged) ARP replies on behalf of other host in the LAN.

Please pull or let me know of any problem!

Thanks a lot,
Antonio

The following changes since commit e84d2f8d2ae33c8215429824e1ecf24cbca9645e:

net: filter: s390: fix JIT address randomization (2014-05-14 16:10:16 -0400)

are available in the git repository at:

git://git.open-mesh.org/linux-merge.git tags/batman-adv-fix-for-davem

for you to fetch changes up to cc2f33860cea0e48ebec096130bd0f7c4bf6e0bc:

batman-adv: fix local TT check for outgoing arp requests in DAT (2014-05-15 20:23:47 +0200)

----------------------------------------------------------------
Include changes:
- fix NULL dereference in batadv_orig_hardif_seq_print_text()
- fix reference counting imbalance when using fragmentation
- avoid access to orig_node objects after they have been free'd
- fix local TT check for outgoing arp requests in DAT

----------------------------------------------------------------
Antonio Quartulli (3):
batman-adv: fix reference counting imbalance while sending fragment
batman-adv: increase orig refcount when storing ref in gw_node
batman-adv: fix local TT check for outgoing arp requests in DAT

Marek Lindner (1):
batman-adv: fix indirect hard_iface NULL dereference

 net/batman-adv/distributed-arp-table.c |  3 +--
 net/batman-adv/fragmentation.c         | 11 ++++++++---
 net/batman-adv/gateway_client.c        | 11 +++++++++--
 net/batman-adv/originator.c            |  3 ++-
 4 files changed, 20 insertions(+), 8 deletions(-)
#1 lore raw

PATCH 1/4 batman-adv: fix indirect hard_iface NULL dereference

From: Marek Lindner <[email protected]>

If hard_iface is NULL and goto out is made batadv_hardif_free_ref() doesn't check for NULL before dereferencing it to get to refcount.

Introduced in cb1c92ec37fb70543d133a1fa7d9b54d6f8a1ecd ("batman-adv: add debugfs support to view multiif tables").

---
 net/batman-adv/originator.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/batman-adv/originator.c +2 -1
index 1785da3..6a48451 100644
@@ -1079,7 +1079,8 @@ int batadv_orig_hardif_seq_print_text(struct seq_file *seq, void *offset)
 	bat_priv->bat_algo_ops->bat_orig_print(bat_priv, seq, hard_iface);
 
 out:
-	batadv_hardif_free_ref(hard_iface);
+	if (hard_iface)
+		batadv_hardif_free_ref(hard_iface);
 	return 0;
 }
 
Signature · 1 line
1.8.5.5
#2 lore raw

PATCH 2/4 batman-adv: fix reference counting imbalance while sending fragment

In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>

In the new fragmentation code the batadv_frag_send_packet() function obtains a reference to the primary_if, but it does not release it upon return.

This reference imbalance prevents the primary_if (and then the related netdevice) to be properly released on shut down.

Fix this by releasing the primary_if in batadv_frag_send_packet().

Introduced by ee75ed88879af88558818a5c6609d85f60ff0df4 ("batman-adv: Fragment and send skbs larger than mtu")

---
 net/batman-adv/fragmentation.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
net/batman-adv/fragmentation.c +8 -3
index bcc4bea..f14e54a 100644
@@ -418,12 +418,13 @@ bool batadv_frag_send_packet(struct sk_buff *skb,
 			     struct batadv_neigh_node *neigh_node)
 {
 	struct batadv_priv *bat_priv;
-	struct batadv_hard_iface *primary_if;
+	struct batadv_hard_iface *primary_if = NULL;
 	struct batadv_frag_packet frag_header;
 	struct sk_buff *skb_fragment;
 	unsigned mtu = neigh_node->if_incoming->net_dev->mtu;
 	unsigned header_size = sizeof(frag_header);
 	unsigned max_fragment_size, max_packet_size;
+	bool ret = false;
 
 	/* To avoid merge and refragmentation at next-hops we never send
 	 * fragments larger than BATADV_FRAG_MAX_FRAG_SIZE
@@ -483,7 +484,11 @@ bool batadv_frag_send_packet(struct sk_buff *skb,
 			   skb->len + ETH_HLEN);
 	batadv_send_skb_packet(skb, neigh_node->if_incoming, neigh_node->addr);
 
-	return true;
+	ret = true;
+
 out_err:
-	return false;
+	if (primary_if)
+		batadv_hardif_free_ref(primary_if);
+
+	return ret;
 }
Signature · 1 line
1.8.5.5
#3 lore raw

PATCH 3/4 batman-adv: increase orig refcount when storing ref in gw_node

In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>

A pointer to the orig_node representing a bat-gateway is stored in the gw_node->orig_node member, but the refcount for such orig_node is never increased.
This leads to memory faults when gw_node->orig_node is accessed and the originator has already been freed.

Fix this by increasing the refcount on gw_node creation and decreasing it on gw_node free.

---
 net/batman-adv/gateway_client.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
net/batman-adv/gateway_client.c +9 -2
index c835e13..90cff58 100644
@@ -42,8 +42,10 @@
 
 static void batadv_gw_node_free_ref(struct batadv_gw_node *gw_node)
 {
-	if (atomic_dec_and_test(&gw_node->refcount))
+	if (atomic_dec_and_test(&gw_node->refcount)) {
+		batadv_orig_node_free_ref(gw_node->orig_node);
 		kfree_rcu(gw_node, rcu);
+	}
 }
 
 static struct batadv_gw_node *
@@ -406,9 +408,14 @@ static void batadv_gw_node_add(struct batadv_priv *bat_priv,
 	if (gateway->bandwidth_down == 0)
 		return;
 
+	if (!atomic_inc_not_zero(&orig_node->refcount))
+		return;
+
 	gw_node = kzalloc(sizeof(*gw_node), GFP_ATOMIC);
-	if (!gw_node)
+	if (!gw_node) {
+		batadv_orig_node_free_ref(orig_node);
 		return;
+	}
 
 	INIT_HLIST_NODE(&gw_node->list);
 	gw_node->orig_node = orig_node;
Signature · 1 line
1.8.5.5
#4 lore raw

PATCH 4/4 batman-adv: fix local TT check for outgoing arp requests in DAT

In reply to Antonio Quartulli

From: Antonio Quartulli <[email protected]>

Change introduced by 88e48d7b3340ef07b108eb8a8b3813dd093cc7f7 ("batman-adv: make DAT drop ARP requests targeting local clients") implements a check that prevents DAT from using the caching mechanism when the client that is supposed to provide a reply to an arp request is local.

However change brought by be1db4f6615b5e6156c807ea8985171c215c2d57 ("batman-adv: make the Distributed ARP Table vlan aware") has not converted the above check into its vlan aware version thus making it useless when the local client is behind a vlan.

Fix the behaviour by properly specifying the vlan when checking for a client being local or not.

---
 net/batman-adv/distributed-arp-table.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
net/batman-adv/distributed-arp-table.c +1 -2
index b25fd64..aa5d494 100644
@@ -940,8 +940,7 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
 		 * additional DAT answer may trigger kernel warnings about
 		 * a packet coming from the wrong port.
 		 */
-		if (batadv_is_my_client(bat_priv, dat_entry->mac_addr,
-					BATADV_NO_FLAGS)) {
+		if (batadv_is_my_client(bat_priv, dat_entry->mac_addr, vid)) {
 			ret = true;
 			goto out;
 		}
Signature · 1 line
1.8.5.5
#6 lore raw
Quoted 6 lines · David Miller

From: Antonio Quartulli <[email protected]>
Date: Thu, 15 May 2014 20:50:48 +0200

Quoted 1 line

Please pull or let me know of any problem!

Pulled, thanks Antonio.

Thanks a lot David. Please enqueue them for stable as well.

Cheers,

Signature · 1 line
Antonio Quartulli
maintainer
#7 lore raw

From: Antonio Quartulli <[email protected]>
Date: Fri, 16 May 2014 23:34:16 +0200

Quoted 9 lines · David Miller
Quoted 6 lines

From: Antonio Quartulli <[email protected]>
Date: Thu, 15 May 2014 20:50:48 +0200

Quoted 1 line

Please pull or let me know of any problem!

Pulled, thanks Antonio.

Thanks a lot David. Please enqueue them for stable as well.

Done, thanks for reminding me :)