batman-adv: Fix potential synchronization issues in mcast tvlv handler
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: 60432d756c
("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>
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
This commit is contained in:
parent
9c936e3f4c
commit
8a4023c5b5
|
@ -20,6 +20,7 @@
|
||||||
|
|
||||||
#include <linux/atomic.h>
|
#include <linux/atomic.h>
|
||||||
#include <linux/bitops.h>
|
#include <linux/bitops.h>
|
||||||
|
#include <linux/bug.h>
|
||||||
#include <linux/byteorder/generic.h>
|
#include <linux/byteorder/generic.h>
|
||||||
#include <linux/errno.h>
|
#include <linux/errno.h>
|
||||||
#include <linux/etherdevice.h>
|
#include <linux/etherdevice.h>
|
||||||
|
@ -589,19 +590,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,
|
* If the BATADV_MCAST_WANT_ALL_UNSNOOPABLES flag of this originator,
|
||||||
* orig, has toggled then this method updates counter and list accordingly.
|
* 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,
|
static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
|
||||||
struct batadv_orig_node *orig,
|
struct batadv_orig_node *orig,
|
||||||
uint8_t mcast_flags)
|
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 */
|
/* switched from flag unset to set */
|
||||||
if (mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&
|
if (mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES &&
|
||||||
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES)) {
|
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES)) {
|
||||||
atomic_inc(&bat_priv->mcast.num_want_all_unsnoopables);
|
atomic_inc(&bat_priv->mcast.num_want_all_unsnoopables);
|
||||||
|
|
||||||
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
hlist_add_head_rcu(&orig->mcast_want_all_unsnoopables_node,
|
/* flag checks above + mcast_handler_lock prevents this */
|
||||||
&bat_priv->mcast.want_all_unsnoopables_list);
|
WARN_ON(!hlist_unhashed(node));
|
||||||
|
|
||||||
|
hlist_add_head_rcu(node, head);
|
||||||
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
/* switched from flag set to unset */
|
/* switched from flag set to unset */
|
||||||
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES) &&
|
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_UNSNOOPABLES) &&
|
||||||
|
@ -609,7 +617,10 @@ static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
|
||||||
atomic_dec(&bat_priv->mcast.num_want_all_unsnoopables);
|
atomic_dec(&bat_priv->mcast.num_want_all_unsnoopables);
|
||||||
|
|
||||||
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
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 */
|
||||||
|
WARN_ON(hlist_unhashed(node));
|
||||||
|
|
||||||
|
hlist_del_init_rcu(node);
|
||||||
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -622,19 +633,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
|
* If the BATADV_MCAST_WANT_ALL_IPV4 flag of this originator, orig, has
|
||||||
* toggled then this method updates counter and list accordingly.
|
* 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,
|
static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
|
||||||
struct batadv_orig_node *orig,
|
struct batadv_orig_node *orig,
|
||||||
uint8_t mcast_flags)
|
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 */
|
/* switched from flag unset to set */
|
||||||
if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV4 &&
|
if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV4 &&
|
||||||
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV4)) {
|
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV4)) {
|
||||||
atomic_inc(&bat_priv->mcast.num_want_all_ipv4);
|
atomic_inc(&bat_priv->mcast.num_want_all_ipv4);
|
||||||
|
|
||||||
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
hlist_add_head_rcu(&orig->mcast_want_all_ipv4_node,
|
/* flag checks above + mcast_handler_lock prevents this */
|
||||||
&bat_priv->mcast.want_all_ipv4_list);
|
WARN_ON(!hlist_unhashed(node));
|
||||||
|
|
||||||
|
hlist_add_head_rcu(node, head);
|
||||||
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
/* switched from flag set to unset */
|
/* switched from flag set to unset */
|
||||||
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV4) &&
|
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV4) &&
|
||||||
|
@ -642,7 +660,10 @@ static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
|
||||||
atomic_dec(&bat_priv->mcast.num_want_all_ipv4);
|
atomic_dec(&bat_priv->mcast.num_want_all_ipv4);
|
||||||
|
|
||||||
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
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 */
|
||||||
|
WARN_ON(hlist_unhashed(node));
|
||||||
|
|
||||||
|
hlist_del_init_rcu(node);
|
||||||
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -655,19 +676,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
|
* If the BATADV_MCAST_WANT_ALL_IPV6 flag of this originator, orig, has
|
||||||
* toggled then this method updates counter and list accordingly.
|
* 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,
|
static void batadv_mcast_want_ipv6_update(struct batadv_priv *bat_priv,
|
||||||
struct batadv_orig_node *orig,
|
struct batadv_orig_node *orig,
|
||||||
uint8_t mcast_flags)
|
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 */
|
/* switched from flag unset to set */
|
||||||
if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV6 &&
|
if (mcast_flags & BATADV_MCAST_WANT_ALL_IPV6 &&
|
||||||
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV6)) {
|
!(orig->mcast_flags & BATADV_MCAST_WANT_ALL_IPV6)) {
|
||||||
atomic_inc(&bat_priv->mcast.num_want_all_ipv6);
|
atomic_inc(&bat_priv->mcast.num_want_all_ipv6);
|
||||||
|
|
||||||
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
hlist_add_head_rcu(&orig->mcast_want_all_ipv6_node,
|
/* flag checks above + mcast_handler_lock prevents this */
|
||||||
&bat_priv->mcast.want_all_ipv6_list);
|
WARN_ON(!hlist_unhashed(node));
|
||||||
|
|
||||||
|
hlist_add_head_rcu(node, head);
|
||||||
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
/* switched from flag set to unset */
|
/* switched from flag set to unset */
|
||||||
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV6) &&
|
} else if (!(mcast_flags & BATADV_MCAST_WANT_ALL_IPV6) &&
|
||||||
|
@ -675,7 +703,10 @@ static void batadv_mcast_want_ipv6_update(struct batadv_priv *bat_priv,
|
||||||
atomic_dec(&bat_priv->mcast.num_want_all_ipv6);
|
atomic_dec(&bat_priv->mcast.num_want_all_ipv6);
|
||||||
|
|
||||||
spin_lock_bh(&bat_priv->mcast.want_lists_lock);
|
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 */
|
||||||
|
WARN_ON(hlist_unhashed(node));
|
||||||
|
|
||||||
|
hlist_del_init_rcu(node);
|
||||||
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -698,6 +729,11 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
|
||||||
uint8_t mcast_flags = BATADV_NO_FLAGS;
|
uint8_t mcast_flags = BATADV_NO_FLAGS;
|
||||||
bool orig_initialized;
|
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 = test_bit(BATADV_ORIG_CAPA_HAS_MCAST,
|
orig_initialized = test_bit(BATADV_ORIG_CAPA_HAS_MCAST,
|
||||||
&orig->capa_initialized);
|
&orig->capa_initialized);
|
||||||
|
|
||||||
|
@ -723,15 +759,12 @@ static void batadv_mcast_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
|
||||||
|
|
||||||
set_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized);
|
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_unsnoop_update(bat_priv, orig, mcast_flags);
|
||||||
batadv_mcast_want_ipv4_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);
|
batadv_mcast_want_ipv6_update(bat_priv, orig, mcast_flags);
|
||||||
|
|
||||||
orig->mcast_flags = mcast_flags;
|
orig->mcast_flags = mcast_flags;
|
||||||
|
spin_unlock_bh(&orig->mcast_handler_lock);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -765,6 +798,8 @@ void batadv_mcast_purge_orig(struct batadv_orig_node *orig)
|
||||||
{
|
{
|
||||||
struct batadv_priv *bat_priv = orig->bat_priv;
|
struct batadv_priv *bat_priv = orig->bat_priv;
|
||||||
|
|
||||||
|
spin_lock_bh(&orig->mcast_handler_lock);
|
||||||
|
|
||||||
if (!test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities) &&
|
if (!test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capabilities) &&
|
||||||
test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized))
|
test_bit(BATADV_ORIG_CAPA_HAS_MCAST, &orig->capa_initialized))
|
||||||
atomic_dec(&bat_priv->mcast.num_disabled);
|
atomic_dec(&bat_priv->mcast.num_disabled);
|
||||||
|
@ -772,4 +807,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_unsnoop_update(bat_priv, orig, BATADV_NO_FLAGS);
|
||||||
batadv_mcast_want_ipv4_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);
|
batadv_mcast_want_ipv6_update(bat_priv, orig, BATADV_NO_FLAGS);
|
||||||
|
|
||||||
|
spin_unlock_bh(&orig->mcast_handler_lock);
|
||||||
}
|
}
|
||||||
|
|
|
@ -696,8 +696,13 @@ struct batadv_orig_node *batadv_orig_node_new(struct batadv_priv *bat_priv,
|
||||||
orig_node->last_seen = jiffies;
|
orig_node->last_seen = jiffies;
|
||||||
reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
|
reset_time = jiffies - 1 - msecs_to_jiffies(BATADV_RESET_PROTECTION_MS);
|
||||||
orig_node->bcast_seqno_reset = reset_time;
|
orig_node->bcast_seqno_reset = reset_time;
|
||||||
|
|
||||||
#ifdef CONFIG_BATMAN_ADV_MCAST
|
#ifdef CONFIG_BATMAN_ADV_MCAST
|
||||||
orig_node->mcast_flags = BATADV_NO_FLAGS;
|
orig_node->mcast_flags = BATADV_NO_FLAGS;
|
||||||
|
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->mcast_handler_lock);
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
/* create a vlan object for the "untagged" LAN */
|
/* create a vlan object for the "untagged" LAN */
|
||||||
|
|
|
@ -221,6 +221,7 @@ struct batadv_orig_bat_iv {
|
||||||
* @batadv_dat_addr_t: address of the orig node in the distributed hash
|
* @batadv_dat_addr_t: address of the orig node in the distributed hash
|
||||||
* @last_seen: time when last packet from this node was received
|
* @last_seen: time when last packet from this node was received
|
||||||
* @bcast_seqno_reset: time when the broadcast seqno window was reset
|
* @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_flags: multicast flags announced by the orig node
|
||||||
* @mcast_want_all_unsnoop_node: a list node for the
|
* @mcast_want_all_unsnoop_node: a list node for the
|
||||||
* mcast.want_all_unsnoopables list
|
* mcast.want_all_unsnoopables list
|
||||||
|
@ -268,6 +269,8 @@ struct batadv_orig_node {
|
||||||
unsigned long last_seen;
|
unsigned long last_seen;
|
||||||
unsigned long bcast_seqno_reset;
|
unsigned long bcast_seqno_reset;
|
||||||
#ifdef CONFIG_BATMAN_ADV_MCAST
|
#ifdef CONFIG_BATMAN_ADV_MCAST
|
||||||
|
/* synchronizes mcast tvlv specific orig changes */
|
||||||
|
spinlock_t mcast_handler_lock;
|
||||||
uint8_t mcast_flags;
|
uint8_t mcast_flags;
|
||||||
struct hlist_node mcast_want_all_unsnoopables_node;
|
struct hlist_node mcast_want_all_unsnoopables_node;
|
||||||
struct hlist_node mcast_want_all_ipv4_node;
|
struct hlist_node mcast_want_all_ipv4_node;
|
||||||
|
|
Loading…
Reference in a new issue