[v2] batman-adv: Don't double free unicast skb on failure

Message ID 1283640270-30498-1-git-send-email-sven.eckelmann@gmx.de (mailing list archive)
State Accepted, archived
Commit dde523b5e82ba9dc160e4b95dbc95c303fe04843
Headers

Commit Message

Sven Eckelmann Sept. 4, 2010, 10:44 p.m. UTC
  The soft-interface transmission function expects that the unicast skb is
still available when a send failed. This is not true on failed search
for a router. Thus we would try to free the skb twice and create many
different and hard to debug memory access failures due to access on not
(anymore) allocated memory.

Reported-by: Andreas Langer <an.langer@gmx.de>
Signed-off-by: Sven Eckelmann <sven.eckelmann@gmx.de>
---
I decided that it makes more sense that the called function frees the data.
This makes it similar to dev_queue_xmit or our send_skb_packet. This also
fixes the problem with fragmented unicast packets

 soft-interface.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
  

Comments

Marek Lindner Sept. 4, 2010, 10:57 p.m. UTC | #1
On Sunday 05 September 2010 00:44:30 Sven Eckelmann wrote:
> The soft-interface transmission function expects that the unicast skb is
> still available when a send failed. This is not true on failed search
> for a router. Thus we would try to free the skb twice and create many
> different and hard to debug memory access failures due to access on not
> (anymore) allocated memory.

Applied in revision 1786.

Thanks,
Marek
  

Patch

diff --git a/soft-interface.c b/soft-interface.c
index 38134ae..47e5ada 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -182,7 +182,7 @@  int interface_tx(struct sk_buff *skb, struct net_device *soft_iface)
 	} else {
 		ret = unicast_send_skb(skb, bat_priv);
 		if (ret != 0)
-			goto dropped;
+			goto dropped_freed;
 	}
 
 	bat_priv->stats.tx_packets++;
@@ -190,8 +190,9 @@  int interface_tx(struct sk_buff *skb, struct net_device *soft_iface)
 	goto end;
 
 dropped:
-	bat_priv->stats.tx_dropped++;
 	kfree_skb(skb);
+dropped_freed:
+	bat_priv->stats.tx_dropped++;
 end:
 	return NETDEV_TX_OK;
 }