| From beb2793bde0067d9a12b29cab7323c9cc906f4e2 Mon Sep 17 00:00:00 2001 |
| From: Sven Eckelmann <sven@narfation.org> |
| Date: Sat, 23 Feb 2019 14:27:10 +0100 |
| Subject: batman-adv: Reduce tt_global hash refcnt only for removed entry |
| |
| [ Upstream commit f131a56880d10932931e74773fb8702894a94a75 ] |
| |
| The batadv_hash_remove is a function which searches the hashtable for an |
| entry using a needle, a hashtable bucket selection function and a compare |
| function. It will lock the bucket list and delete an entry when the compare |
| function matches it with the needle. It returns the pointer to the |
| hlist_node which matches or NULL when no entry matches the needle. |
| |
| The batadv_tt_global_free is not itself protected in anyway to avoid that |
| any other function is modifying the hashtable between the search for the |
| entry and the call to batadv_hash_remove. It can therefore happen that the |
| entry either doesn't exist anymore or an entry was deleted which is not the |
| same object as the needle. In such an situation, the reference counter (for |
| the reference stored in the hashtable) must not be reduced for the needle. |
| Instead the reference counter of the actually removed entry has to be |
| reduced. |
| |
| Otherwise the reference counter will underflow and the object might be |
| freed before all its references were dropped. The kref helpers reported |
| this problem as: |
| |
| refcount_t: underflow; use-after-free. |
| |
| Fixes: 7683fdc1e886 ("batman-adv: protect the local and the global trans-tables with rcu") |
| Reported-by: Martin Weinelt <martin@linuxlounge.net> |
| Signed-off-by: Sven Eckelmann <sven@narfation.org> |
| Acked-by: Antonio Quartulli <a@unstable.cc> |
| Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de> |
| Signed-off-by: Sasha Levin (Microsoft) <sashal@kernel.org> |
| --- |
| net/batman-adv/translation-table.c | 18 +++++++++++++++--- |
| 1 file changed, 15 insertions(+), 3 deletions(-) |
| |
| diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c |
| index 2ee87d3ca6d0..6ec0e67be560 100644 |
| --- a/net/batman-adv/translation-table.c |
| +++ b/net/batman-adv/translation-table.c |
| @@ -616,14 +616,26 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv, |
| struct batadv_tt_global_entry *tt_global, |
| const char *message) |
| { |
| + struct batadv_tt_global_entry *tt_removed_entry; |
| + struct hlist_node *tt_removed_node; |
| + |
| batadv_dbg(BATADV_DBG_TT, bat_priv, |
| "Deleting global tt entry %pM (vid: %d): %s\n", |
| tt_global->common.addr, |
| batadv_print_vid(tt_global->common.vid), message); |
| |
| - batadv_hash_remove(bat_priv->tt.global_hash, batadv_compare_tt, |
| - batadv_choose_tt, &tt_global->common); |
| - batadv_tt_global_entry_put(tt_global); |
| + tt_removed_node = batadv_hash_remove(bat_priv->tt.global_hash, |
| + batadv_compare_tt, |
| + batadv_choose_tt, |
| + &tt_global->common); |
| + if (!tt_removed_node) |
| + return; |
| + |
| + /* drop reference of remove hash entry */ |
| + tt_removed_entry = hlist_entry(tt_removed_node, |
| + struct batadv_tt_global_entry, |
| + common.hash_entry); |
| + batadv_tt_global_entry_put(tt_removed_entry); |
| } |
| |
| /** |
| -- |
| 2.20.1 |
| |