1 From: Sven Eckelmann <sven@narfation.org>
2 Date: Sat, 4 Jun 2016 08:52:12 +0200
3 Subject: [PATCH] batman-adv: Fix use-after-free/double-free of tt_req_node
5 The tt_req_node is added and removed from a list inside a spinlock. But the
6 locking is sometimes removed even when the object is still referenced and
7 will be used later via this reference. For example batadv_send_tt_request
8 can create a new tt_req_node (including add to a list) and later
9 re-acquires the lock to remove it from the list and to free it. But at this
10 time another context could have already removed this tt_req_node from the
15 batadv_batman_skb_recv from net_device 0
16 -> batadv_iv_ogm_receive
17 -> batadv_iv_ogm_process
18 -> batadv_iv_ogm_process_per_outif
19 -> batadv_tvlv_ogm_receive
20 -> batadv_tvlv_ogm_receive
21 -> batadv_tvlv_containers_process
22 -> batadv_tvlv_call_handler
23 -> batadv_tt_tvlv_ogm_handler_v1
24 -> batadv_tt_update_orig
25 -> batadv_send_tt_request
26 -> batadv_tt_req_node_new
28 allocates new tt_req_node and adds it to list
34 batadv_batman_skb_recv from net_device 1
35 -> batadv_recv_unicast_tvlv
36 -> batadv_tvlv_containers_process
37 -> batadv_tvlv_call_handler
38 -> batadv_tt_tvlv_unicast_handler_v1
39 -> batadv_handle_tt_response
41 tt_req_node gets removed from list and is freed
46 <- returned to batadv_send_tt_request
48 tt_req_node gets removed from list and is freed
49 MEMORY CORRUPTION/SEGFAULT/...
52 This can only be solved via reference counting to allow multiple contexts
53 to handle the list manipulation while making sure that only the last
54 context holding a reference will free the object.
56 Fixes: cea194d90b11 ("batman-adv: improved client announcement mechanism")
57 Signed-off-by: Sven Eckelmann <sven@narfation.org>
58 Tested-by: Martin Weinelt <martin@darmstadt.freifunk.net>
59 Tested-by: Amadeus Alfa <amadeus@chemnitz.freifunk.net>
61 Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/c3fef3d9ec6e8b882f321ec20f6f2cb2ee906503
63 net/batman-adv/translation-table.c | 37 +++++++++++++++++++++++++++++++++----
64 net/batman-adv/types.h | 2 ++
65 2 files changed, 35 insertions(+), 4 deletions(-)
67 diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
68 index 5ed782b..23fb7ea 100644
69 --- a/net/batman-adv/translation-table.c
70 +++ b/net/batman-adv/translation-table.c
71 @@ -2271,6 +2271,29 @@ static u32 batadv_tt_local_crc(struct batadv_priv *bat_priv,
76 + * batadv_tt_req_node_release - free tt_req node entry
77 + * @ref: kref pointer of the tt req_node entry
79 +static void batadv_tt_req_node_release(struct kref *ref)
81 + struct batadv_tt_req_node *tt_req_node;
83 + tt_req_node = container_of(ref, struct batadv_tt_req_node, refcount);
89 + * batadv_tt_req_node_put - decrement the tt_req_node refcounter and
90 + * possibly release it
91 + * @tt_req_node: tt_req_node to be free'd
93 +static void batadv_tt_req_node_put(struct batadv_tt_req_node *tt_req_node)
95 + kref_put(&tt_req_node->refcount, batadv_tt_req_node_release);
98 static void batadv_tt_req_list_free(struct batadv_priv *bat_priv)
100 struct batadv_tt_req_node *node;
101 @@ -2280,7 +2303,7 @@ static void batadv_tt_req_list_free(struct batadv_priv *bat_priv)
103 hlist_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) {
104 hlist_del_init(&node->list);
106 + batadv_tt_req_node_put(node);
109 spin_unlock_bh(&bat_priv->tt.req_list_lock);
110 @@ -2317,7 +2340,7 @@ static void batadv_tt_req_purge(struct batadv_priv *bat_priv)
111 if (batadv_has_timed_out(node->issued_at,
112 BATADV_TT_REQUEST_TIMEOUT)) {
113 hlist_del_init(&node->list);
115 + batadv_tt_req_node_put(node);
118 spin_unlock_bh(&bat_priv->tt.req_list_lock);
119 @@ -2349,9 +2372,11 @@ batadv_tt_req_node_new(struct batadv_priv *bat_priv,
123 + kref_init(&tt_req_node->refcount);
124 ether_addr_copy(tt_req_node->addr, orig_node->orig);
125 tt_req_node->issued_at = jiffies;
127 + kref_get(&tt_req_node->refcount);
128 hlist_add_head(&tt_req_node->list, &bat_priv->tt.req_list);
130 spin_unlock_bh(&bat_priv->tt.req_list_lock);
131 @@ -2618,9 +2643,13 @@ out:
132 spin_lock_bh(&bat_priv->tt.req_list_lock);
133 /* hlist_del_init() verifies tt_req_node still is in the list */
134 hlist_del_init(&tt_req_node->list);
135 + batadv_tt_req_node_put(tt_req_node);
136 spin_unlock_bh(&bat_priv->tt.req_list_lock);
137 - kfree(tt_req_node);
141 + batadv_tt_req_node_put(tt_req_node);
146 @@ -3056,7 +3085,7 @@ static void batadv_handle_tt_response(struct batadv_priv *bat_priv,
147 if (!batadv_compare_eth(node->addr, resp_src))
149 hlist_del_init(&node->list);
151 + batadv_tt_req_node_put(node);
154 spin_unlock_bh(&bat_priv->tt.req_list_lock);
155 diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
156 index 1e47fbe..d75beef 100644
157 --- a/net/batman-adv/types.h
158 +++ b/net/batman-adv/types.h
159 @@ -1129,11 +1129,13 @@ struct batadv_tt_change_node {
160 * struct batadv_tt_req_node - data to keep track of the tt requests in flight
161 * @addr: mac address address of the originator this request was sent to
162 * @issued_at: timestamp used for purging stale tt requests
163 + * @refcount: number of contexts the object is used by
164 * @list: list node for batadv_priv_tt::req_list
166 struct batadv_tt_req_node {
168 unsigned long issued_at;
169 + struct kref refcount;
170 struct hlist_node list;