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

[Antonio Quartulli <antonio@meshcoding.com>]

[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]

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!

-- 
Antonio Quartulli


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]