Commit a2a72c15 authored by Sven Eckelmann's avatar Sven Eckelmann Committed by Kelsey Skunberg

batman-adv: Reduce tt_global hash refcnt only for removed entry

BugLink: https://bugs.launchpad.net/bugs/1868629

commit f131a568 upstream.

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: 7683fdc1 ("batman-adv: protect the local and the global trans-tables with rcu")
Reported-by: default avatarMartin Weinelt <martin@linuxlounge.net>
Signed-off-by: default avatarSven Eckelmann <sven@narfation.org>
Acked-by: default avatarAntonio Quartulli <a@unstable.cc>
Signed-off-by: default avatarSimon Wunderlich <sw@simonwunderlich.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
Signed-off-by: default avatarKelsey Skunberg <kelsey.skunberg@canonical.com>
parent 9648d1c1
...@@ -508,14 +508,26 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv, ...@@ -508,14 +508,26 @@ static void batadv_tt_global_free(struct batadv_priv *bat_priv,
struct batadv_tt_global_entry *tt_global, struct batadv_tt_global_entry *tt_global,
const char *message) const char *message)
{ {
struct batadv_tt_global_entry *tt_removed_entry;
struct hlist_node *tt_removed_node;
batadv_dbg(BATADV_DBG_TT, bat_priv, batadv_dbg(BATADV_DBG_TT, bat_priv,
"Deleting global tt entry %pM (vid: %d): %s\n", "Deleting global tt entry %pM (vid: %d): %s\n",
tt_global->common.addr, tt_global->common.addr,
BATADV_PRINT_VID(tt_global->common.vid), message); BATADV_PRINT_VID(tt_global->common.vid), message);
batadv_hash_remove(bat_priv->tt.global_hash, batadv_compare_tt, tt_removed_node = batadv_hash_remove(bat_priv->tt.global_hash,
batadv_choose_tt, &tt_global->common); batadv_compare_tt,
batadv_tt_global_entry_free_ref(tt_global); 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_free_ref(tt_removed_entry);
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment