10 Jun
2015
10 Jun
'15
8:57 a.m.
On Tuesday 23 April 2013 21:39:57 Marek Lindner wrote:
+/**
- batadv_tvlv_handler_unregister - unregister tvlv handler based on the
- provided type and version (both need to match)
- @bat_priv: the bat priv with all the soft interface information
- @type: tvlv handler type to be unregistered
- @version: tvlv handler version to be unregistered
- */
+void batadv_tvlv_handler_unregister(struct batadv_priv *bat_priv,
uint8_t type, uint8_t version)+{
- struct batadv_tvlv_handler *tvlv_handler;
- tvlv_handler = batadv_tvlv_handler_get(bat_priv, type, version);
- if (!tvlv_handler)
return;- batadv_tvlv_handler_free_ref(tvlv_handler);
- spin_lock_bh(&bat_priv->tvlv.handler_list_lock);
- hlist_del_rcu(&tvlv_handler->list);
- spin_unlock_bh(&bat_priv->tvlv.handler_list_lock);
- batadv_tvlv_handler_free_ref(tvlv_handler);
+}
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