Re: [B.A.T.M.A.N.] [PATCH next] batman-adv: avoid potential race condition when adding a new neighbour

On 27/01/14 09:10, Marek Lindner wrote:

On Friday 24 January 2014 09:58:03 Antonio Quartulli wrote:

...

@@ -281,14 +281,23 @@ batadv_iv_ogm_neigh_new(struct batadv_hard_iface *hard_iface, neigh_node->orig_node = orig_neigh; neigh_node->if_incoming = hard_iface;

• batadv_dbg(BATADV_DBG_BATMAN, bat_priv,

•    "Creating new neighbor %pM for orig_node %pM on interface %s\n",
  

•    neigh_addr, orig_node->orig, hard_iface->net_dev->name);
  

• spin_lock_bh(&orig_node->neigh_list_lock);
• hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list);

• tmp_neigh_node = batadv_neigh_node_get(orig_node, hard_iface,

• 			       neigh_addr);
  

• if (!tmp_neigh_node) {

• hlist_add_head_rcu(&neigh_node->list, &orig_node->neigh_list);
  

• } else {

• kfree(neigh_node);
  

• neigh_node = tmp_neigh_node;
  

• }

What about hard_iface->refcount ?

True. hard_iface must be released if we found an already existent neighbour. I'll put a batadv_hardif_free_ref(hard_iface); right after the kfree(neigh_node);.

...

/**

• • batadv_neigh_node_get - retrieve a neighbour from the list
• • @orig_node: originator which the neighbour belongs to
• • @hard_iface: the interface where this neighbour is connected to
• • @addr: the address of the neighbour
• • Looks for and possibly return a neighbour belonging to this originator

list + * which is connected through the provided hard interface.

'Looks and return' does not work. Either 'look and return' or 'looks and returns'.

Yap, thanks!

I'll send v2 soon!