On Fri, Jun 10, 2016 at 06:14:01PM +0200, Sven Eckelmann wrote:
batadv_send_skb_to_orig can return -1 to signal that the skb was not consumed. tp_meter has then to free the skb to avoid a memory leak.
Fixes: 98d7a766b645 ("batman-adv: throughput meter implementation") Signed-off-by: Sven Eckelmann sven@narfation.org
Antonio, this is not really tested. Could you please review it and tell me if I may have missed something.
net/batman-adv/tp_meter.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c index bf6bffb..2333777 100644 --- a/net/batman-adv/tp_meter.c +++ b/net/batman-adv/tp_meter.c @@ -1206,6 +1206,9 @@ static int batadv_tp_send_ack(struct batadv_priv *bat_priv, const u8 *dst,
/* send the ack */ r = batadv_send_skb_to_orig(skb, orig_node, NULL);
- if (r == -1)
kfree_skb(skb);
Good catch, Sven !
However, how about changing the patch this way ?
--- a/net/batman-adv/tp_meter.c +++ b/net/batman-adv/tp_meter.c @@ -1206,7 +1206,7 @@ static int batadv_tp_send_ack(struct batadv_priv *bat_priv, const u8 *dst,
/* send the ack */ r = batadv_send_skb_to_orig(skb, orig_node, NULL); - if (r == -1) + if ((r == -1) || !dev_xmit_complete(res)) kfree_skb(skb);
if (unlikely(r < 0) || (r == NET_XMIT_DROP)) {
this is the same check we do in batadv_send_skb_unicast().
Cheers,