173 lines
5.9 KiB
Diff
173 lines
5.9 KiB
Diff
From: Sven Eckelmann <sven@narfation.org>
|
|
Date: Sat, 4 Jun 2016 08:52:12 +0200
|
|
Subject: [PATCH] batman-adv: Fix use-after-free/double-free of tt_req_node
|
|
|
|
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_node
|
|
|
|
CPU#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>
|
|
Tested-by: Amadeus Alfa <amadeus@chemnitz.freifunk.net>
|
|
|
|
Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/c3fef3d9ec6e8b882f321ec20f6f2cb2ee906503
|
|
---
|
|
net/batman-adv/translation-table.c | 37 +++++++++++++++++++++++++++++++++----
|
|
net/batman-adv/types.h | 2 ++
|
|
2 files changed, 35 insertions(+), 4 deletions(-)
|
|
|
|
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
|
|
index 5ed782b..23fb7ea 100644
|
|
--- a/net/batman-adv/translation-table.c
|
|
+++ b/net/batman-adv/translation-table.c
|
|
@@ -2271,6 +2271,29 @@ static u32 batadv_tt_local_crc(struct batadv_priv *bat_priv,
|
|
return crc;
|
|
}
|
|
|
|
+/**
|
|
+ * batadv_tt_req_node_release - free tt_req node entry
|
|
+ * @ref: kref pointer of the tt req_node entry
|
|
+ */
|
|
+static void batadv_tt_req_node_release(struct kref *ref)
|
|
+{
|
|
+ struct batadv_tt_req_node *tt_req_node;
|
|
+
|
|
+ tt_req_node = container_of(ref, struct batadv_tt_req_node, refcount);
|
|
+
|
|
+ kfree(tt_req_node);
|
|
+}
|
|
+
|
|
+/**
|
|
+ * batadv_tt_req_node_put - decrement the tt_req_node refcounter and
|
|
+ * possibly release it
|
|
+ * @tt_req_node: tt_req_node to be free'd
|
|
+ */
|
|
+static void batadv_tt_req_node_put(struct batadv_tt_req_node *tt_req_node)
|
|
+{
|
|
+ kref_put(&tt_req_node->refcount, batadv_tt_req_node_release);
|
|
+}
|
|
+
|
|
static void batadv_tt_req_list_free(struct batadv_priv *bat_priv)
|
|
{
|
|
struct batadv_tt_req_node *node;
|
|
@@ -2280,7 +2303,7 @@ static void batadv_tt_req_list_free(struct batadv_priv *bat_priv)
|
|
|
|
hlist_for_each_entry_safe(node, safe, &bat_priv->tt.req_list, list) {
|
|
hlist_del_init(&node->list);
|
|
- kfree(node);
|
|
+ batadv_tt_req_node_put(node);
|
|
}
|
|
|
|
spin_unlock_bh(&bat_priv->tt.req_list_lock);
|
|
@@ -2317,7 +2340,7 @@ static void batadv_tt_req_purge(struct batadv_priv *bat_priv)
|
|
if (batadv_has_timed_out(node->issued_at,
|
|
BATADV_TT_REQUEST_TIMEOUT)) {
|
|
hlist_del_init(&node->list);
|
|
- kfree(node);
|
|
+ batadv_tt_req_node_put(node);
|
|
}
|
|
}
|
|
spin_unlock_bh(&bat_priv->tt.req_list_lock);
|
|
@@ -2349,9 +2372,11 @@ batadv_tt_req_node_new(struct batadv_priv *bat_priv,
|
|
if (!tt_req_node)
|
|
goto unlock;
|
|
|
|
+ kref_init(&tt_req_node->refcount);
|
|
ether_addr_copy(tt_req_node->addr, orig_node->orig);
|
|
tt_req_node->issued_at = jiffies;
|
|
|
|
+ kref_get(&tt_req_node->refcount);
|
|
hlist_add_head(&tt_req_node->list, &bat_priv->tt.req_list);
|
|
unlock:
|
|
spin_unlock_bh(&bat_priv->tt.req_list_lock);
|
|
@@ -2618,9 +2643,13 @@ out:
|
|
spin_lock_bh(&bat_priv->tt.req_list_lock);
|
|
/* hlist_del_init() verifies tt_req_node still is in the list */
|
|
hlist_del_init(&tt_req_node->list);
|
|
+ batadv_tt_req_node_put(tt_req_node);
|
|
spin_unlock_bh(&bat_priv->tt.req_list_lock);
|
|
- kfree(tt_req_node);
|
|
}
|
|
+
|
|
+ if (tt_req_node)
|
|
+ batadv_tt_req_node_put(tt_req_node);
|
|
+
|
|
kfree(tvlv_tt_data);
|
|
return ret;
|
|
}
|
|
@@ -3056,7 +3085,7 @@ static void batadv_handle_tt_response(struct batadv_priv *bat_priv,
|
|
if (!batadv_compare_eth(node->addr, resp_src))
|
|
continue;
|
|
hlist_del_init(&node->list);
|
|
- kfree(node);
|
|
+ batadv_tt_req_node_put(node);
|
|
}
|
|
|
|
spin_unlock_bh(&bat_priv->tt.req_list_lock);
|
|
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
|
|
index 1e47fbe..d75beef 100644
|
|
--- a/net/batman-adv/types.h
|
|
+++ b/net/batman-adv/types.h
|
|
@@ -1129,11 +1129,13 @@ struct batadv_tt_change_node {
|
|
* struct batadv_tt_req_node - data to keep track of the tt requests in flight
|
|
* @addr: mac address address of the originator this request was sent to
|
|
* @issued_at: timestamp used for purging stale tt requests
|
|
+ * @refcount: number of contexts the object is used by
|
|
* @list: list node for batadv_priv_tt::req_list
|
|
*/
|
|
struct batadv_tt_req_node {
|
|
u8 addr[ETH_ALEN];
|
|
unsigned long issued_at;
|
|
+ struct kref refcount;
|
|
struct hlist_node list;
|
|
};
|
|
|