| From 61a9fc57cb475a9283158e79bdde8e3307151abf 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_local hash refcnt only for removed entry |
| |
| [ Upstream commit 3d65b9accab4a7ed5038f6df403fbd5e298398c7 ] |
| |
| 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_local_remove 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: ef72706a0543 ("batman-adv: protect tt_local_entry from concurrent delete events") |
| Signed-off-by: Sven Eckelmann <sven@narfation.org> |
| Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de> |
| Signed-off-by: Sasha Levin (Microsoft) <sashal@kernel.org> |
| --- |
| net/batman-adv/translation-table.c | 14 +++++++++----- |
| 1 file changed, 9 insertions(+), 5 deletions(-) |
| |
| diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c |
| index 8dcd4968cde7..2ee87d3ca6d0 100644 |
| --- a/net/batman-adv/translation-table.c |
| +++ b/net/batman-adv/translation-table.c |
| @@ -1337,9 +1337,10 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr, |
| unsigned short vid, const char *message, |
| bool roaming) |
| { |
| + struct batadv_tt_local_entry *tt_removed_entry; |
| struct batadv_tt_local_entry *tt_local_entry; |
| u16 flags, curr_flags = BATADV_NO_FLAGS; |
| - void *tt_entry_exists; |
| + struct hlist_node *tt_removed_node; |
| |
| tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid); |
| if (!tt_local_entry) |
| @@ -1368,15 +1369,18 @@ u16 batadv_tt_local_remove(struct batadv_priv *bat_priv, const u8 *addr, |
| */ |
| batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL); |
| |
| - tt_entry_exists = batadv_hash_remove(bat_priv->tt.local_hash, |
| + tt_removed_node = batadv_hash_remove(bat_priv->tt.local_hash, |
| batadv_compare_tt, |
| batadv_choose_tt, |
| &tt_local_entry->common); |
| - if (!tt_entry_exists) |
| + if (!tt_removed_node) |
| goto out; |
| |
| - /* extra call to free the local tt entry */ |
| - batadv_tt_local_entry_put(tt_local_entry); |
| + /* drop reference of remove hash entry */ |
| + tt_removed_entry = hlist_entry(tt_removed_node, |
| + struct batadv_tt_local_entry, |
| + common.hash_entry); |
| + batadv_tt_local_entry_put(tt_removed_entry); |
| |
| out: |
| if (tt_local_entry) |
| -- |
| 2.20.1 |
| |