diff mbox

[v2] batman-adv: fix NULL pointer deref in batadv_find_best_neighbor

Message ID 1388080621-15040-1-git-send-email-sw@simonwunderlich.de (mailing list archive)
State Accepted, archived
Headers

Commit Message

Simon Wunderlich Dec. 26, 2013, 5:57 p.m. UTC 
  If there is no best neighbor, don't dereference the NULL pointer. Try to
acquire the neighbor node right in the loop instead. There also was a
logic bug, finding the worst instead of the best neighbor.

Introduced by 9bb33b8d88e318c4879d37d06ad28e3e018b9036 ("batman-adv:
split tq information in neigh_node struct")

Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 originator.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Comments

Marek Lindner Dec. 28, 2013, 8:39 a.m. UTC | #1 
On Thursday 26 December 2013 18:57:01 Simon Wunderlich wrote:
> If there is no best neighbor, don't dereference the NULL pointer. Try to
> acquire the neighbor node right in the loop instead. There also was a
> logic bug, finding the worst instead of the best neighbor.
> 
> Introduced by 9bb33b8d88e318c4879d37d06ad28e3e018b9036 ("batman-adv:
> split tq information in neigh_node struct")
> 
> Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
> ---
>  originator.c |   19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)

Applied in revision 183150b.

Thanks,
Marek
diff mbox

Patch

diff --git a/originator.c b/originator.c
index 2243003..4a5d027 100644
--- a/originator.c
+++ b/originator.c
@@ -783,14 +783,19 @@  batadv_find_best_neighbor(struct batadv_priv *bat_priv,
 	struct batadv_algo_ops *bao = bat_priv->bat_algo_ops;
 
 	rcu_read_lock();
-	hlist_for_each_entry_rcu(neigh, &orig_node->neigh_list, list)
-		if (!best ||
-		    (bao->bat_neigh_cmp(neigh, if_outgoing,
-					best, if_outgoing) <= 0))
-			best = neigh;
+	hlist_for_each_entry_rcu(neigh, &orig_node->neigh_list, list) {
+		if (best && (bao->bat_neigh_cmp(neigh, if_outgoing,
+						best, if_outgoing) <= 0))
+			continue;
 
-	if (!atomic_inc_not_zero(&best->refcount))
-		best = NULL;
+		if (!atomic_inc_not_zero(&neigh->refcount))
+			continue;
+
+		if (best)
+			batadv_neigh_node_free_ref(best);
+
+		best = neigh;
+	}
 	rcu_read_unlock();
 
 	return best;