[B.A.T.M.A.N.] [PATCH] batman-adv: Don't free unicast skb on failure

[B.A.T.M.A.N.] [PATCH] batman-adv: Don't free unicast skb on failure

[Sven Eckelmann]

The soft-interface transmission function expects that the unicast skb is
still available when a send failed. Otherwise 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>
---
 unicast.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/unicast.c b/unicast.c
index 84b204b..796d9c1 100644
--- a/unicast.c
+++ b/unicast.c
@@ -264,6 +264,5 @@ int unicast_send_skb(struct sk_buff *skb, struct bat_priv *bat_priv)
 unlock:
 	spin_unlock_irqrestore(&bat_priv->orig_hash_lock, flags);
 dropped:
-	kfree_skb(skb);
 	return 1;
 }
-- 
1.7.1

[B.A.T.M.A.N.] [PATCHv2] batman-adv: Don't double free unicast skb on failure

[Sven Eckelmann]

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

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;
 }
-- 
1.7.1

Re: [B.A.T.M.A.N.] [PATCHv2] batman-adv: Don't double free unicast skb on failure

[Marek Lindner]

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