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