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
signature.asc
Description: This is a digitally signed message part.
