b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCH] batman-adv: skbs have to be freed using kfree_skb()
@ 2011-06-14 16:01 Antonio Quartulli
  2011-06-15 18:56 ` Marek Lindner
  0 siblings, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2011-06-14 16:01 UTC (permalink / raw)
  To: B.A.T.M.A.N

The skb structures have always to be freed using kfree_skb() and not the
simple kfree()

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 routing.c           |    2 +-
 translation-table.c |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/routing.c b/routing.c
index 30d0f73..41d065e 100644
--- a/routing.c
+++ b/routing.c
@@ -1299,7 +1299,7 @@ int recv_roam_adv(struct sk_buff *skb, struct hard_iface *recv_if)
 	orig_node_free_ref(orig_node);
 	ret = NET_RX_SUCCESS;
 out:
-	kfree(skb);
+	kfree_skb(skb);
 	return ret;
 }
 
diff --git a/translation-table.c b/translation-table.c
index adecf1b..373ee81 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1199,7 +1199,7 @@ out:
 	if (primary_if)
 		hardif_free_ref(primary_if);
 	if (!ret)
-		kfree(skb);
+		kfree_skb(skb);
 	return ret;
 
 }
@@ -1312,7 +1312,7 @@ out:
 	if (primary_if)
 		hardif_free_ref(primary_if);
 	if (!ret)
-		kfree(skb);
+		kfree_skb(skb);
 	/* This packet was for me, so it doesn't need to be re-routed */
 	return true;
 }
-- 
1.7.3.4


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

* Re: [B.A.T.M.A.N.] [PATCH] batman-adv: skbs have to be freed using kfree_skb()
  2011-06-14 16:01 [B.A.T.M.A.N.] [PATCH] batman-adv: skbs have to be freed using kfree_skb() Antonio Quartulli
@ 2011-06-15 18:56 ` Marek Lindner
  2011-06-15 22:32   ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: the skb has not to be freed in recv_roam_adv() Antonio Quartulli
  2011-06-15 22:32   ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: skbs have to be freed using kfree_skb() Antonio Quartulli
  0 siblings, 2 replies; 6+ messages in thread
From: Marek Lindner @ 2011-06-15 18:56 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Tuesday, June 14, 2011 06:01:39 PM Antonio Quartulli wrote:
> @@ -1299,7 +1299,7 @@ int recv_roam_adv(struct sk_buff *skb, struct
> hard_iface *recv_if) orig_node_free_ref(orig_node);
>         ret = NET_RX_SUCCESS;
>  out:
> -       kfree(skb);
> +       kfree_skb(skb);
>         return ret;
>  }
>  

I'd say recv_roam_adv() suffers from the same problem you fixed in 
recv_tt_query() with your previous patch. Wouldn't you say so ?

Cheers,
Marek

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

* [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: the skb has not to be freed in recv_roam_adv()
  2011-06-15 18:56 ` Marek Lindner
@ 2011-06-15 22:32   ` Antonio Quartulli
  2011-06-16 10:16     ` Marek Lindner
  2011-06-15 22:32   ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: skbs have to be freed using kfree_skb() Antonio Quartulli
  1 sibling, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2011-06-15 22:32 UTC (permalink / raw)
  To: B.A.T.M.A.N

In recv_roam_adv(), in case of error the skb is freed and then
NET_RX_DROP is returned. This makes the caller function wrongly invoke
kfree_skb() again. To avoid this double free recv_tt_query() has to
always return NET_RX_DROP and not to free the skb.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 routing.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/routing.c b/routing.c
index 2222288..0ce090c 100644
--- a/routing.c
+++ b/routing.c
@@ -1259,7 +1259,6 @@ int recv_roam_adv(struct sk_buff *skb, struct hard_iface *recv_if)
 	struct roam_adv_packet *roam_adv_packet;
 	struct orig_node *orig_node;
 	struct ethhdr *ethhdr;
-	int ret = NET_RX_DROP;
 
 	/* drop packet if it has not necessary minimum size */
 	if (unlikely(!pskb_may_pull(skb, sizeof(struct roam_adv_packet))))
@@ -1297,10 +1296,9 @@ int recv_roam_adv(struct sk_buff *skb, struct hard_iface *recv_if)
 	bat_priv->tt_poss_change = true;
 
 	orig_node_free_ref(orig_node);
-	ret = NET_RX_SUCCESS;
 out:
-	kfree(skb);
-	return ret;
+	/* returning NET_RX_DROP will make the caller function kfree the skb */
+	return NET_RX_DROP;
 }
 
 /* find a suitable router for this originator, and use
-- 
1.7.3.4


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

* [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: skbs have to be freed using kfree_skb()
  2011-06-15 18:56 ` Marek Lindner
  2011-06-15 22:32   ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: the skb has not to be freed in recv_roam_adv() Antonio Quartulli
@ 2011-06-15 22:32   ` Antonio Quartulli
  2011-06-16 10:22     ` Marek Lindner
  1 sibling, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2011-06-15 22:32 UTC (permalink / raw)
  To: B.A.T.M.A.N

The skb structures have always to be freed using kfree_skb() and not the
simple kfree()

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---
 translation-table.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/translation-table.c b/translation-table.c
index adecf1b..373ee81 100644
--- a/translation-table.c
+++ b/translation-table.c
@@ -1199,7 +1199,7 @@ out:
 	if (primary_if)
 		hardif_free_ref(primary_if);
 	if (!ret)
-		kfree(skb);
+		kfree_skb(skb);
 	return ret;
 
 }
@@ -1312,7 +1312,7 @@ out:
 	if (primary_if)
 		hardif_free_ref(primary_if);
 	if (!ret)
-		kfree(skb);
+		kfree_skb(skb);
 	/* This packet was for me, so it doesn't need to be re-routed */
 	return true;
 }
-- 
1.7.3.4


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

* Re: [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: the skb has not to be freed in recv_roam_adv()
  2011-06-15 22:32   ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: the skb has not to be freed in recv_roam_adv() Antonio Quartulli
@ 2011-06-16 10:16     ` Marek Lindner
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Lindner @ 2011-06-16 10:16 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Thursday, June 16, 2011 12:32:24 AM Antonio Quartulli wrote:
> In recv_roam_adv(), in case of error the skb is freed and then
> NET_RX_DROP is returned. This makes the caller function wrongly invoke
> kfree_skb() again. To avoid this double free recv_tt_query() has to
> always return NET_RX_DROP and not to free the skb.

Applied in revision ad0432c.

Thanks,
Marek

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

* Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: skbs have to be freed using kfree_skb()
  2011-06-15 22:32   ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: skbs have to be freed using kfree_skb() Antonio Quartulli
@ 2011-06-16 10:22     ` Marek Lindner
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Lindner @ 2011-06-16 10:22 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Thursday, June 16, 2011 12:32:25 AM Antonio Quartulli wrote:
> The skb structures have always to be freed using kfree_skb() and not the
> simple kfree()

Applied in revision 6f0ff74.

Thanks,
Marek

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

end of thread, other threads:[~2011-06-16 10:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-14 16:01 [B.A.T.M.A.N.] [PATCH] batman-adv: skbs have to be freed using kfree_skb() Antonio Quartulli
2011-06-15 18:56 ` Marek Lindner
2011-06-15 22:32   ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: the skb has not to be freed in recv_roam_adv() Antonio Quartulli
2011-06-16 10:16     ` Marek Lindner
2011-06-15 22:32   ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: skbs have to be freed using kfree_skb() Antonio Quartulli
2011-06-16 10:22     ` Marek Lindner

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