diff mbox

batman-adv: fix NULL pointer deref in batadv_find_best_neighbor

Message ID 1387294513-12541-1-git-send-email-sw@simonwunderlich.de (mailing list archive)
State Superseded, archived
Headers

Commit Message

Simon Wunderlich Dec. 17, 2013, 3:35 p.m. UTC 
  If there is no best neighbor, don't dereference the NULL pointer.

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

Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 originator.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marek Lindner Dec. 25, 2013, 2:15 p.m. UTC | #1 
On Tuesday 17 December 2013 16:35:13 Simon Wunderlich wrote:
> If there is no best neighbor, don't dereference the NULL pointer.
> 
> Introduced by 9bb33b8d88e318c4879d37d06ad28e3e018b9036 ("batman-adv:
> split tq information in neigh_node struct")
> 
> Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
> ---
>  originator.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/originator.c b/originator.c
> index 2243003..9fcde58 100644
> --- a/originator.c
> +++ b/originator.c
> @@ -789,7 +789,7 @@ batadv_find_best_neighbor(struct batadv_priv *bat_priv,
>  					best, if_outgoing) <= 0))
>  			best = neigh;
> 
> -	if (!atomic_inc_not_zero(&best->refcount))
> +	if (best && !atomic_inc_not_zero(&best->refcount))
>  		best = NULL;
>  	rcu_read_unlock();

Although this is a valid fix the approach could be improved. The for-loop 
should check whether the counter can be increased. Otherwise we might select a 
best-neighbor that is being purged and end up with no best despite having 
alternatives.

Cheers,
Marek
diff mbox

Patch

diff --git a/originator.c b/originator.c
index 2243003..9fcde58 100644
--- a/originator.c
+++ b/originator.c
@@ -789,7 +789,7 @@  batadv_find_best_neighbor(struct batadv_priv *bat_priv,
 					best, if_outgoing) <= 0))
 			best = neigh;
 
-	if (!atomic_inc_not_zero(&best->refcount))
+	if (best && !atomic_inc_not_zero(&best->refcount))
 		best = NULL;
 	rcu_read_unlock();