On Saturday, June 04, 2016 08:52:12 Sven Eckelmann wrote:
The tt_req_node is added and removed from a list inside a spinlock. But the locking is sometimes removed even when the object is still referenced and will be used later via this reference. For example batadv_send_tt_request can create a new tt_req_node (including add to a list) and later re-acquires the lock to remove it from the list and to free it. But at this time another context could have already removed this tt_req_node from the list and freed it.
CPU#0
batadv_batman_skb_recv from net_device 0 -> batadv_iv_ogm_receive -> batadv_iv_ogm_process -> batadv_iv_ogm_process_per_outif -> batadv_tvlv_ogm_receive -> batadv_tvlv_ogm_receive -> batadv_tvlv_containers_process -> batadv_tvlv_call_handler -> batadv_tt_tvlv_ogm_handler_v1 -> batadv_tt_update_orig -> batadv_send_tt_request -> batadv_tt_req_node_new spin_lock(...) allocates new tt_req_node and adds it to list spin_unlock(...) return tt_req_nodeCPU#1
batadv_batman_skb_recv from net_device 1 -> batadv_recv_unicast_tvlv -> batadv_tvlv_containers_process -> batadv_tvlv_call_handler -> batadv_tt_tvlv_unicast_handler_v1 -> batadv_handle_tt_response spin_lock(...) tt_req_node gets removed from list and is freed spin_unlock(...)CPU#0
<- returned to batadv_send_tt_request spin_lock(...) tt_req_node gets removed from list and is freed MEMORY CORRUPTION/SEGFAULT/... spin_unlock(...)This can only be solved via reference counting to allow multiple contexts to handle the list manipulation while making sure that only the last context holding a reference will free the object.
Fixes: cea194d90b11 ("batman-adv: improved client announcement mechanism") Signed-off-by: Sven Eckelmann sven@narfation.org Tested-by: Martin Weinelt martin@darmstadt.freifunk.net
v3:
- add wrapper function batadv_tt_req_node_put for kref_put(....)
v2:
- fixed list->object in commit message
- add example what could have gone wrong in commit message
net/batman-adv/translation-table.c | 37 +++++++++++++++++++++++++++++++++---- net/batman-adv/types.h | 2 ++ 2 files changed, 35 insertions(+), 4 deletions(-)
Applied in revision c3fef3d.
Thanks, Marek