Re: [B.A.T.M.A.N.] [PATCHv14 5/6] batman-adv: Add IPv4 link-local/IPv6-ll-all-nodes multicast support

On Saturday 15 February 2014 17:47:53 Linus Lüssing wrote:

+static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,

• 			     struct batadv_orig_node *orig,
  

• 			     uint8_t mcast_flags)
  

+{

• /* switched from flag unset to set */
• if (mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&

•    !(orig->mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES)) {
  

• atomic_inc(&bat_priv->mcast.num_want_all_unsnoopables);
  

• spin_lock_bh(&bat_priv->mcast.want_lists_lock);
  

• hlist_add_head_rcu(&orig->mcast_want_all_unsnoopables_node,
  

• 		   &bat_priv->mcast.want_all_unsnoopables_list);
  

• spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
  

• /* switched from flag set to unset */
• } else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES) &&

•    orig->mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES) {
  

• atomic_dec(&bat_priv->mcast.num_want_all_unsnoopables);
  

• spin_lock_bh(&bat_priv->mcast.want_lists_lock);
  

• hlist_del_rcu(&orig->mcast_want_all_unsnoopables_node);
  

• spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
  

• }

Looks unsafe. How do you make sure that it is still in the list and not already already removed in the meantime? hlist_del_rcu does not re-initialize the pointers (only POISON them when CONFIG_DEBUG_LIST is activated). Thus calling hlist_del_rcu again on an object not in the list either kills other data structures or causes an Oops.

There are more *list_del* related problems of that type in your code. This is just one example I've picked.

See http://www.open-mesh.org/issues/217#note-5

Kind regards, Sven