Commit 15c2ed75 authored by Sven Eckelmann's avatar Sven Eckelmann Committed by Simon Wunderlich

batman-adv: Fix reference leak in batadv_find_router

The replacement of last_bonding_candidate in batadv_orig_node has to be an
atomic operation. Otherwise it is possible that the reference counter of a
batadv_orig_ifinfo is reduced which was no longer the
last_bonding_candidate when the new candidate is added. This can either
lead to an invalid memory access or to reference leaks which make it
impossible to an interface which was added to batman-adv.

Fixes: f3b3d901 ("batman-adv: add bonding again")
Signed-off-by: default avatarSven Eckelmann <sven@narfation.org>
Signed-off-by: default avatarMarek Lindner <mareklindner@neomailbox.ch>
Signed-off-by: default avatarSimon Wunderlich <sw@simonwunderlich.de>
parent 3db0decf
...@@ -455,6 +455,29 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv, ...@@ -455,6 +455,29 @@ static int batadv_check_unicast_packet(struct batadv_priv *bat_priv,
return 0; return 0;
} }
/**
* batadv_last_bonding_replace - Replace last_bonding_candidate of orig_node
* @orig_node: originator node whose bonding candidates should be replaced
* @new_candidate: new bonding candidate or NULL
*/
static void
batadv_last_bonding_replace(struct batadv_orig_node *orig_node,
struct batadv_orig_ifinfo *new_candidate)
{
struct batadv_orig_ifinfo *old_candidate;
spin_lock_bh(&orig_node->neigh_list_lock);
old_candidate = orig_node->last_bonding_candidate;
if (new_candidate)
kref_get(&new_candidate->refcount);
orig_node->last_bonding_candidate = new_candidate;
spin_unlock_bh(&orig_node->neigh_list_lock);
if (old_candidate)
batadv_orig_ifinfo_put(old_candidate);
}
/** /**
* batadv_find_router - find a suitable router for this originator * batadv_find_router - find a suitable router for this originator
* @bat_priv: the bat priv with all the soft interface information * @bat_priv: the bat priv with all the soft interface information
...@@ -562,10 +585,6 @@ batadv_find_router(struct batadv_priv *bat_priv, ...@@ -562,10 +585,6 @@ batadv_find_router(struct batadv_priv *bat_priv,
} }
rcu_read_unlock(); rcu_read_unlock();
/* last_bonding_candidate is reset below, remove the old reference. */
if (orig_node->last_bonding_candidate)
batadv_orig_ifinfo_put(orig_node->last_bonding_candidate);
/* After finding candidates, handle the three cases: /* After finding candidates, handle the three cases:
* 1) there is a next candidate, use that * 1) there is a next candidate, use that
* 2) there is no next candidate, use the first of the list * 2) there is no next candidate, use the first of the list
...@@ -574,21 +593,28 @@ batadv_find_router(struct batadv_priv *bat_priv, ...@@ -574,21 +593,28 @@ batadv_find_router(struct batadv_priv *bat_priv,
if (next_candidate) { if (next_candidate) {
batadv_neigh_node_put(router); batadv_neigh_node_put(router);
/* remove references to first candidate, we don't need it. */ kref_get(&next_candidate_router->refcount);
if (first_candidate) {
batadv_neigh_node_put(first_candidate_router);
batadv_orig_ifinfo_put(first_candidate);
}
router = next_candidate_router; router = next_candidate_router;
orig_node->last_bonding_candidate = next_candidate; batadv_last_bonding_replace(orig_node, next_candidate);
} else if (first_candidate) { } else if (first_candidate) {
batadv_neigh_node_put(router); batadv_neigh_node_put(router);
/* refcounting has already been done in the loop above. */ kref_get(&first_candidate_router->refcount);
router = first_candidate_router; router = first_candidate_router;
orig_node->last_bonding_candidate = first_candidate; batadv_last_bonding_replace(orig_node, first_candidate);
} else { } else {
orig_node->last_bonding_candidate = NULL; batadv_last_bonding_replace(orig_node, NULL);
}
/* cleanup of candidates */
if (first_candidate) {
batadv_neigh_node_put(first_candidate_router);
batadv_orig_ifinfo_put(first_candidate);
}
if (next_candidate) {
batadv_neigh_node_put(next_candidate_router);
batadv_orig_ifinfo_put(next_candidate);
} }
return router; return router;
......
...@@ -330,7 +330,9 @@ struct batadv_orig_node { ...@@ -330,7 +330,9 @@ struct batadv_orig_node {
DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE); DECLARE_BITMAP(bcast_bits, BATADV_TQ_LOCAL_WINDOW_SIZE);
u32 last_bcast_seqno; u32 last_bcast_seqno;
struct hlist_head neigh_list; struct hlist_head neigh_list;
/* neigh_list_lock protects: neigh_list and router */ /* neigh_list_lock protects: neigh_list, ifinfo_list,
* last_bonding_candidate and router
*/
spinlock_t neigh_list_lock; spinlock_t neigh_list_lock;
struct hlist_node hash_entry; struct hlist_node hash_entry;
struct batadv_priv *bat_priv; struct batadv_priv *bat_priv;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment