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(-)
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)
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); }
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;
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,
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 ... */
b.a.t.m.a.n@lists.open-mesh.org
-
Antonio Quartulli -
Simon Wunderlich