[maint] batman-adv: fix reference counting imbalance while sending fragment

Message ID 1398254716-22089-1-git-send-email-antonio@meshcoding.com (mailing list archive)
State Accepted, archived
Commit 113f264b60bc4f50011a8f736d74f91ff7da11f1
Headers

Commit Message

Antonio Quartulli April 23, 2014, 12:05 p.m. UTC
  From: Antonio Quartulli <antonio@open-mesh.com>

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 db56e4ecf5c2b179a0101138eacc2ec52b6ef45d
("batman-adv: Fragment and send skbs larger than mtu")

Cc: Martin Hundebøll <martin@hundeboll.net>
Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 fragmentation.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
  

Comments

Martin Hundebøll April 23, 2014, 4:40 p.m. UTC | #1
Hi Antonio,

Good catch!

On 2014-04-23 05:05, Antonio Quartulli wrote:
> From: Antonio Quartulli <antonio@open-mesh.com>
> 
> 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 db56e4ecf5c2b179a0101138eacc2ec52b6ef45d
> ("batman-adv: Fragment and send skbs larger than mtu")
> 
> Cc: Martin Hundebøll <martin@hundeboll.net>
> Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>

Acked-by: Martin Hundebøll <martin@hundeboll.net>

// Martin

> ---
>  fragmentation.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/fragmentation.c b/fragmentation.c
> index 88df9b1..cc1cfd6 100644
> --- a/fragmentation.c
> +++ b/fragmentation.c
> @@ -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;
>  }
  
Marek Lindner May 2, 2014, 5:15 a.m. UTC | #2
On Wednesday 23 April 2014 09:40:05 martin@hundeboll.net wrote:
> On 2014-04-23 05:05, Antonio Quartulli wrote:
> > From: Antonio Quartulli <antonio@open-mesh.com>
> >
> > 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 db56e4ecf5c2b179a0101138eacc2ec52b6ef45d
> > ("batman-adv: Fragment and send skbs larger than mtu")
> >
> > Cc: Martin Hundebøll <martin@hundeboll.net>
> > Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
> 
> Acked-by: Martin Hundebøll <martin@hundeboll.net>

Applied in revision 113f264.

Thanks,
Marek
  

Patch

diff --git a/fragmentation.c b/fragmentation.c
index 88df9b1..cc1cfd6 100644
--- a/fragmentation.c
+++ b/fragmentation.c
@@ -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;
 }