255 lines
10 KiB
Diff
255 lines
10 KiB
Diff
From 7f220ed1f063be00833bd34a013c8f3f45884031 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>
|
|
Date: Tue, 16 Jun 2015 17:10:26 +0200
|
|
Subject: [PATCH 07/17] batman-adv: Fix potential synchronization issues in
|
|
mcast tvlv handler
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
So far the mcast tvlv handler did not anticipate the processing of
|
|
multiple incoming OGMs from the same originator at the same time. This
|
|
can lead to various issues:
|
|
|
|
* Broken refcounting: For instance two mcast handlers might both assume
|
|
that an originator just got multicast capabilities and will together
|
|
wrongly decrease mcast.num_disabled by two, potentially leading to
|
|
an integer underflow.
|
|
|
|
* Potential kernel panic on hlist_del_rcu(): Two mcast handlers might
|
|
one after another try to do an
|
|
hlist_del_rcu(&orig->mcast_want_all_*_node). The second one will
|
|
cause memory corruption / crashes.
|
|
(Reported by: Sven Eckelmann <sven@narfation.org>)
|
|
|
|
Right in the beginning the code path makes assumptions about the current
|
|
multicast related state of an originator and bases all updates on that. The
|
|
easiest and least error prune way to fix the issues in this case is to
|
|
serialize multiple mcast handler invocations with a spinlock.
|
|
|
|
Fixes: 77ec494490d6 ("batman-adv: Announce new capability via multicast TVLV")
|
|
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
|
|
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
|
|
---
|
|
multicast.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++-------------
|
|
originator.c | 4 ++++
|
|
types.h | 3 +++
|
|
3 files changed, 56 insertions(+), 13 deletions(-)
|
|
|
|
diff --git a/multicast.c b/multicast.c
|
|
index 00612bf..b75bcc3 100644
|
|
--- a/multicast.c
|
|
+++ b/multicast.c
|
|
@@ -565,19 +565,26 @@ batadv_mcast_forw_mode(struct batadv_priv *bat_priv, struct sk_buff *skb,
|
|
*
|
|
* If the BATADV_MCAST_WANT_ALL_UNSNOOPABLES flag of this originator,
|
|
* orig, has toggled then this method updates counter and list accordingly.
|
|
+ *
|
|
+ * Caller needs to hold orig->mcast_handler_lock.
|
|
*/
|
|
static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
|
|
struct batadv_orig_node *orig,
|
|
uint8_t mcast_flags)
|
|
{
|
|
+ struct hlist_node *node = &orig->mcast_want_all_unsnoopables_node;
|
|
+ struct hlist_head *head = &bat_priv->mcast.want_all_unsnoopables_list;
|
|
+
|
|
/* switched from flag unset to set */
|
|
if (mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&
|
|
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES)) {
|
|
atomic_inc(&bat_priv->mcast.num_want_all_unsnoopables);
|
|
|
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
|
- hlist_add_head_rcu(&orig->mcast_want_all_unsnoopables_node,
|
|
- &bat_priv->mcast.want_all_unsnoopables_list);
|
|
+ /* flag checks above + mcast_handler_lock prevents this */
|
|
+ BUG_ON(!hlist_unhashed(node));
|
|
+
|
|
+ hlist_add_head_rcu(node, head);
|
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
|
/* switched from flag set to unset */
|
|
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES) &&
|
|
@@ -585,7 +592,10 @@ static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
|
|
atomic_dec(&bat_priv->mcast.num_want_all_unsnoopables);
|
|
|
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
|
- hlist_del_rcu(&orig->mcast_want_all_unsnoopables_node);
|
|
+ /* flag checks above + mcast_handler_lock prevents this */
|
|
+ BUG_ON(hlist_unhashed(node));
|
|
+
|
|
+ hlist_del_init_rcu(node);
|
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
|
}
|
|
}
|
|
@@ -598,19 +608,26 @@ static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
|
|
*
|
|
* If the BATADV_MCAST_WANT_ALL_IPV4 flag of this originator, orig, has
|
|
* toggled then this method updates counter and list accordingly.
|
|
+ *
|
|
+ * Caller needs to hold orig->mcast_handler_lock.
|
|
*/
|
|
static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
|
|
struct batadv_orig_node *orig,
|
|
uint8_t mcast_flags)
|
|
{
|
|
+ struct hlist_node *node = &orig->mcast_want_all_ipv4_node;
|
|
+ struct hlist_head *head = &bat_priv->mcast.want_all_ipv4_list;
|
|
+
|
|
/* switched from flag unset to set */
|
|
if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV4 &&
|
|
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV4)) {
|
|
atomic_inc(&bat_priv->mcast.num_want_all_ipv4);
|
|
|
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
|
- hlist_add_head_rcu(&orig->mcast_want_all_ipv4_node,
|
|
- &bat_priv->mcast.want_all_ipv4_list);
|
|
+ /* flag checks above + mcast_handler_lock prevents this */
|
|
+ BUG_ON(!hlist_unhashed(node));
|
|
+
|
|
+ hlist_add_head_rcu(node, head);
|
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
|
/* switched from flag set to unset */
|
|
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV4) &&
|
|
@@ -618,7 +635,10 @@ static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
|
|
atomic_dec(&bat_priv->mcast.num_want_all_ipv4);
|
|
|
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
|
- hlist_del_rcu(&orig->mcast_want_all_ipv4_node);
|
|
+ /* flag checks above + mcast_handler_lock prevents this */
|
|
+ BUG_ON(hlist_unhashed(node));
|
|
+
|
|
+ hlist_del_init_rcu(node);
|
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
|
}
|
|
}
|
|
@@ -631,19 +651,26 @@ static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
|
|
*
|
|
* If the BATADV_MCAST_WANT_ALL_IPV6 flag of this originator, orig, has
|
|
* toggled then this method updates counter and list accordingly.
|
|
+ *
|
|
+ * Caller needs to hold orig->mcast_handler_lock.
|
|
*/
|
|
static void batadv_mcast_want_ipv6_update(struct batadv_priv *bat_priv,
|
|
struct batadv_orig_node *orig,
|
|
uint8_t mcast_flags)
|
|
{
|
|
+ struct hlist_node *node = &orig->mcast_want_all_ipv6_node;
|
|
+ struct hlist_head *head = &bat_priv->mcast.want_all_ipv6_list;
|
|
+
|
|
/* switched from flag unset to set */
|
|
if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV6 &&
|
|
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV6)) {
|
|
atomic_inc(&bat_priv->mcast.num_want_all_ipv6);
|
|
|
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
|
- hlist_add_head_rcu(&orig->mcast_want_all_ipv6_node,
|
|
- &bat_priv->mcast.want_all_ipv6_list);
|
|
+ /* flag checks above + mcast_handler_lock prevents this */
|
|
+ BUG_ON(!hlist_unhashed(node));
|
|
+
|
|
+ hlist_add_head_rcu(node, head);
|
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
|
/* switched from flag set to unset */
|
|
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV6) &&
|
|
@@ -651,7 +678,10 @@ static void batadv_mcast_want_ipv6_update(struct batadv_priv *bat_priv,
|
|
atomic_dec(&bat_priv->mcast.num_want_all_ipv6);
|
|
|
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
|
- hlist_del_rcu(&orig->mcast_want_all_ipv6_node);
|
|
+ /* flag checks above + mcast_handler_lock prevents this */
|
|
+ BUG_ON(hlist_unhashed(node));
|
|
+
|
|
+ hlist_del_init_rcu(node);
|
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
|
}
|
|
}
|
|
@@ -674,6 +704,11 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
|
|
uint8_t mcast_flags = BATADV_NO_FLAGS;
|
|
bool orig_initialized;
|
|
|
|
+ if (orig_mcast_enabled && tvlv_value &&
|
|
+ (tvlv_value_len >= sizeof(mcast_flags)))
|
|
+ mcast_flags = *(uint8_t *)tvlv_value;
|
|
+
|
|
+ spin_lock_bh(&orig->mcast_handler_lock);
|
|
orig_initialized = orig->capa_initialized & BATADV_ORIG_CAPA_HAS_MCAST;
|
|
|
|
/* If mcast support is turned on decrease the disabled mcast node
|
|
@@ -698,15 +733,12 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
|
|
|
|
set_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized);
|
|
|
|
- if (orig_mcast_enabled && tvlv_value &&
|
|
- (tvlv_value_len >= sizeof(mcast_flags)))
|
|
- mcast_flags = *(uint8_t *)tvlv_value;
|
|
-
|
|
batadv_mcast_want_unsnoop_update(bat_priv, orig, mcast_flags);
|
|
batadv_mcast_want_ipv4_update(bat_priv, orig, mcast_flags);
|
|
batadv_mcast_want_ipv6_update(bat_priv, orig, mcast_flags);
|
|
|
|
orig->mcast_flags = mcast_flags;
|
|
+ spin_unlock_bh(&orig->mcast_handler_lock);
|
|
}
|
|
|
|
/**
|
|
@@ -740,6 +772,8 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig)
|
|
{
|
|
struct batadv_priv *bat_priv = orig->bat_priv;
|
|
|
|
+ spin_lock_bh(&orig->mcast_handler_lock);
|
|
+
|
|
if (!(orig->capabilities & BATADV_ORIG_CAPA_HAS_MCAST) &&
|
|
orig->capa_initialized & BATADV_ORIG_CAPA_HAS_MCAST)
|
|
atomic_dec(&bat_priv->mcast.num_disabled);
|
|
@@ -747,4 +781,6 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig)
|
|
batadv_mcast_want_unsnoop_update(bat_priv, orig, BATADV_NO_FLAGS);
|
|
batadv_mcast_want_ipv4_update(bat_priv, orig, BATADV_NO_FLAGS);
|
|
batadv_mcast_want_ipv6_update(bat_priv, orig, BATADV_NO_FLAGS);
|
|
+
|
|
+ spin_unlock_bh(&orig->mcast_handler_lock);
|
|
}
|
|
diff --git a/originator.c b/originator.c
|
|
index e3900e4..a2ba182 100644
|
|
--- a/originator.c
|
|
+++ b/originator.c
|
|
@@ -658,11 +658,15 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv,
|
|
INIT_HLIST_HEAD(&orig_node->neigh_list);
|
|
INIT_LIST_HEAD(&orig_node->vlan_list);
|
|
INIT_HLIST_HEAD(&orig_node->ifinfo_list);
|
|
+ INIT_HLIST_NODE(&orig_node->mcast_want_all_unsnoopables_node);
|
|
+ INIT_HLIST_NODE(&orig_node->mcast_want_all_ipv4_node);
|
|
+ INIT_HLIST_NODE(&orig_node->mcast_want_all_ipv6_node);
|
|
spin_lock_init(&orig_node->bcast_seqno_lock);
|
|
spin_lock_init(&orig_node->neigh_list_lock);
|
|
spin_lock_init(&orig_node->tt_buff_lock);
|
|
spin_lock_init(&orig_node->tt_lock);
|
|
spin_lock_init(&orig_node->vlan_list_lock);
|
|
+ spin_lock_init(&orig_node->mcast_handler_lock);
|
|
|
|
batadv_nc_init_orig(orig_node);
|
|
|
|
diff --git a/types.h b/types.h
|
|
index c6ec558..65dc6bf 100644
|
|
--- a/types.h
|
|
+++ b/types.h
|
|
@@ -204,6 +204,7 @@ struct batadv_orig_bat_iv {
|
|
* @batadv_dat_addr_t: address of the orig node in the distributed hash
|
|
* @last_seen: time when last packet from this node was received
|
|
* @bcast_seqno_reset: time when the broadcast seqno window was reset
|
|
+ * @mcast_handler_lock: synchronizes mcast-capability and -flag changes
|
|
* @mcast_flags: multicast flags announced by the orig node
|
|
* @mcast_want_all_unsnoop_node: a list node for the
|
|
* mcast.want_all_unsnoopables list
|
|
@@ -251,6 +252,8 @@ struct batadv_orig_node {
|
|
unsigned long last_seen;
|
|
unsigned long bcast_seqno_reset;
|
|
#ifdef CONFIG_BATMAN_ADV_MCAST
|
|
+ /* synchronizes mcast tvlv specific orig changes */
|
|
+ spinlock_t mcast_handler_lock;
|
|
uint8_t mcast_flags;
|
|
struct hlist_node mcast_want_all_unsnoopables_node;
|
|
struct hlist_node mcast_want_all_ipv4_node;
|
|
--
|
|
2.1.4
|
|
|