]>
Commit | Line | Data |
---|---|---|
6d2fc1d5 SL |
1 | From beb2793bde0067d9a12b29cab7323c9cc906f4e2 Mon Sep 17 00:00:00 2001 |
2 | From: Sven Eckelmann <sven@narfation.org> | |
3 | Date: Sat, 23 Feb 2019 14:27:10 +0100 | |
4 | Subject: batman-adv: Reduce tt_global hash refcnt only for removed entry | |
5 | ||
6 | [ Upstream commit f131a56880d10932931e74773fb8702894a94a75 ] | |
7 | ||
8 | The batadv_hash_remove is a function which searches the hashtable for an | |
9 | entry using a needle, a hashtable bucket selection function and a compare | |
10 | function. It will lock the bucket list and delete an entry when the compare | |
11 | function matches it with the needle. It returns the pointer to the | |
12 | hlist_node which matches or NULL when no entry matches the needle. | |
13 | ||
14 | The batadv_tt_global_free is not itself protected in anyway to avoid that | |
15 | any other function is modifying the hashtable between the search for the | |
16 | entry and the call to batadv_hash_remove. It can therefore happen that the | |
17 | entry either doesn't exist anymore or an entry was deleted which is not the | |
18 | same object as the needle. In such an situation, the reference counter (for | |
19 | the reference stored in the hashtable) must not be reduced for the needle. | |
20 | Instead the reference counter of the actually removed entry has to be | |
21 | reduced. | |
22 | ||
23 | Otherwise the reference counter will underflow and the object might be | |
24 | freed before all its references were dropped. The kref helpers reported | |
25 | this problem as: | |
26 | ||
27 | refcount_t: underflow; use-after-free. | |
28 | ||
29 | Fixes: 7683fdc1e886 ("batman-adv: protect the local and the global trans-tables with rcu") | |
30 | Reported-by: Martin Weinelt <martin@linuxlounge.net> | |
31 | Signed-off-by: Sven Eckelmann <sven@narfation.org> | |
32 | Acked-by: Antonio Quartulli <a@unstable.cc> | |
33 | Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de> | |
34 | Signed-off-by: Sasha Levin (Microsoft) <sashal@kernel.org> | |
35 | --- | |
36 | net/batman-adv/translation-table.c | 18 +++++++++++++++--- | |
37 | 1 file changed, 15 insertions(+), 3 deletions(-) | |
38 | ||
39 | diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c | |
40 | index 2ee87d3ca6d0..6ec0e67be560 100644 | |
41 | --- a/net/batman-adv/translation-table.c | |
42 | +++ b/net/batman-adv/translation-table.c | |
43 | @@ -616,14 +616,26 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv, | |
44 | struct batadv_tt_global_entry *tt_global, | |
45 | const char *message) | |
46 | { | |
47 | + struct batadv_tt_global_entry *tt_removed_entry; | |
48 | + struct hlist_node *tt_removed_node; | |
49 | + | |
50 | batadv_dbg(BATADV_DBG_TT, bat_priv, | |
51 | "Deleting global tt entry %pM (vid: %d): %s\n", | |
52 | tt_global->common.addr, | |
53 | batadv_print_vid(tt_global->common.vid), message); | |
54 | ||
55 | - batadv_hash_remove(bat_priv->tt.global_hash, batadv_compare_tt, | |
56 | - batadv_choose_tt, &tt_global->common); | |
57 | - batadv_tt_global_entry_put(tt_global); | |
58 | + tt_removed_node = batadv_hash_remove(bat_priv->tt.global_hash, | |
59 | + batadv_compare_tt, | |
60 | + batadv_choose_tt, | |
61 | + &tt_global->common); | |
62 | + if (!tt_removed_node) | |
63 | + return; | |
64 | + | |
65 | + /* drop reference of remove hash entry */ | |
66 | + tt_removed_entry = hlist_entry(tt_removed_node, | |
67 | + struct batadv_tt_global_entry, | |
68 | + common.hash_entry); | |
69 | + batadv_tt_global_entry_put(tt_removed_entry); | |
70 | } | |
71 | ||
72 | /** | |
73 | -- | |
74 | 2.20.1 | |
75 |