[B.A.T.M.A.N.] [PATCH-maint 0/3] fix multiif regressions

[B.A.T.M.A.N.] [PATCH-maint 0/3] fix multiif regressions

[Simon Wunderlich]

From: Simon Wunderlich <simon@open-mesh.com>

There are some regressions introduced my network wide multi interface
optimization patchset which are adressed by this patchset. All of them
fix some kind of leaks caused reference counter imbalances, which may
lead to problems when an interface is removed which was previously used
by batman-adv.

Thanks Antonio for your help tracking that down!

Cheers,
	Simon

Simon Wunderlich (3):
  batman-adv: fix neigh_ifinfo imbalance
  batman-adv: fix neigh reference imbalance
  batman-adv: always run purge_orig_neighbors

 bat_iv_ogm.c |    2 ++
 originator.c |    7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
1.7.10.4

[B.A.T.M.A.N.] [PATCH-maint 1/3] batman-adv: fix neigh_ifinfo imbalance

[Simon Wunderlich]

From: Simon Wunderlich <simon@open-mesh.com>

The neigh_ifinfo object must be freed if it has been used in
batadv_iv_ogm_process_per_outif().

This is a regression introduced by
9bb33b8d88e318c4879d37d06ad28e3e018b9036 ("batman-adv: split tq
information in neigh_node struct")

Reported-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
---
 bat_iv_ogm.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 8323bce..d074d06 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -1545,6 +1545,8 @@ out_neigh:
 	if ((orig_neigh_node) && (!is_single_hop_neigh))
 		batadv_orig_node_free_ref(orig_neigh_node);
 out:
+	if (router_ifinfo)
+		batadv_neigh_ifinfo_free_ref(router_ifinfo);
 	if (router)
 		batadv_neigh_node_free_ref(router);
 	if (router_router)
-- 
1.7.10.4

[B.A.T.M.A.N.] [PATCH-maint 2/3] batman-adv: fix neigh reference imbalance

[Simon Wunderlich]

From: Simon Wunderlich <simon@open-mesh.com>

When an interface is removed from batman-adv, the orig_ifinfo of a
orig_node may be removed without releasing the router first.
This will prevent the reference for the neighbor pointed at by the
orig_ifinfo->router to be released, and this leak may result in
reference leaks for the interface used by this neighbor. Fix that.

This is a regression introduced by
de6bcc76ea84fecb136f8c8f5ba1862e4a13f06b ("batman-adv: split out router
from orig_node").

Reported-by: Antonio Quartulli <antonio@meshcoding.com>
Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
---
 originator.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/originator.c b/originator.c
index 8539416..2fd6678 100644
--- a/originator.c
+++ b/originator.c
@@ -506,6 +506,8 @@ static void batadv_orig_ifinfo_free_rcu(struct rcu_head *rcu)
 	if (orig_ifinfo->if_outgoing != BATADV_IF_DEFAULT)
 		batadv_hardif_free_ref_now(orig_ifinfo->if_outgoing);
 
+	if (orig_ifinfo->router)
+		batadv_neigh_node_free_ref_now(orig_ifinfo->router);
 	kfree(orig_ifinfo);
 }
 
-- 
1.7.10.4

[B.A.T.M.A.N.] [PATCH-maint 3/3] batman-adv: always run purge_orig_neighbors

[Simon Wunderlich]

From: Simon Wunderlich <simon@open-mesh.com>

The current code will not execute batadv_purge_orig_neighbors() when an
orig_ifinfo has already been purged. However we need to run it in any
case. Fix that.

This is a regression introduced by
de6bcc76ea84fecb136f8c8f5ba1862e4a13f06b ("batman-adv: split out router
from orig_node")

Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
---
 originator.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/originator.c b/originator.c
index 2fd6678..3821981 100644
--- a/originator.c
+++ b/originator.c
@@ -854,7 +854,7 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv,
 {
 	struct batadv_neigh_node *best_neigh_node;
 	struct batadv_hard_iface *hard_iface;
-	bool changed;
+	bool changed, changed_neigh;
 
 	if (batadv_has_timed_out(orig_node->last_seen,
 				 2 * BATADV_PURGE_TIMEOUT)) {
@@ -865,7 +865,8 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv,
 		return true;
 	}
 	changed = batadv_purge_orig_ifinfo(bat_priv, orig_node);
-	changed = changed || batadv_purge_orig_neighbors(bat_priv, orig_node);
+	changed_neigh = batadv_purge_orig_neighbors(bat_priv, orig_node);
+	changed |= changed_neigh;
 
 	if (!changed)
 		return false;
-- 
1.7.10.4

Re: [B.A.T.M.A.N.] [PATCH-maint 3/3] batman-adv: always run purge_orig_neighbors

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 946 bytes --]

Hi Simon,

On 19/03/14 11:33, Simon Wunderlich wrote:
> @@ -854,7 +854,7 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv,
>  {
>  	struct batadv_neigh_node *best_neigh_node;
>  	struct batadv_hard_iface *hard_iface;
> -	bool changed;
> +	bool changed, changed_neigh;
>  
>  	if (batadv_has_timed_out(orig_node->last_seen,
>  				 2 * BATADV_PURGE_TIMEOUT)) {
> @@ -865,7 +865,8 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv,
>  		return true;
>  	}
>  	changed = batadv_purge_orig_ifinfo(bat_priv, orig_node);
> -	changed = changed || batadv_purge_orig_neighbors(bat_priv, orig_node);
> +	changed_neigh = batadv_purge_orig_neighbors(bat_priv, orig_node);
> +	changed |= changed_neigh;
>  
>  	if (!changed)

Maybe we should just remove the |= and use this if condition:
	
	if (!changed && !changed_neigh)

?

The |= now is just useless..

Cheers,

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

[B.A.T.M.A.N.] [PATCH-maintv2 3/3] batman-adv: always run purge_orig_neighbors

[Simon Wunderlich]

From: Simon Wunderlich <simon@open-mesh.com>

The current code will not execute batadv_purge_orig_neighbors() when an
orig_ifinfo has already been purged. However we need to run it in any
case. Fix that.

This is a regression introduced by
de6bcc76ea84fecb136f8c8f5ba1862e4a13f06b ("batman-adv: split out router
from orig_node")

Signed-off-by: Simon Wunderlich <simon@open-mesh.com>
---
Changes to PATCHv1:

 * check two change variables separately
---
 originator.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/originator.c b/originator.c
index 2fd6678..9e29f02 100644
--- a/originator.c
+++ b/originator.c
@@ -854,7 +854,7 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv,
 {
 	struct batadv_neigh_node *best_neigh_node;
 	struct batadv_hard_iface *hard_iface;
-	bool changed;
+	bool changed_ifinfo, changed_neigh;
 
 	if (batadv_has_timed_out(orig_node->last_seen,
 				 2 * BATADV_PURGE_TIMEOUT)) {
@@ -864,10 +864,10 @@ static bool batadv_purge_orig_node(struct batadv_priv *bat_priv,
 			   jiffies_to_msecs(orig_node->last_seen));
 		return true;
 	}
-	changed = batadv_purge_orig_ifinfo(bat_priv, orig_node);
-	changed = changed || batadv_purge_orig_neighbors(bat_priv, orig_node);
+	changed_ifinfo = batadv_purge_orig_ifinfo(bat_priv, orig_node);
+	changed_neigh = batadv_purge_orig_neighbors(bat_priv, orig_node);
 
-	if (!changed)
+	if (!changed_ifinfo && !changed_neigh)
 		return false;
 
 	/* first for NULL ... */
-- 
1.7.10.4