@@ -168,6 +168,7 @@ int batadv_mesh_init(struct net_device *soft_iface)
spin_lock_init(&bat_priv->tt.commit_lock);
spin_lock_init(&bat_priv->gw.list_lock);
#ifdef CONFIG_BATMAN_ADV_MCAST
+ spin_lock_init(&bat_priv->mcast.mla_lock);
spin_lock_init(&bat_priv->mcast.want_lists_lock);
#endif
spin_lock_init(&bat_priv->tvlv.container_list_lock);
@@ -314,8 +314,6 @@ static void batadv_mcast_mla_list_free(struct hlist_head *mcast_list)
* translation table except the ones listed in the given mcast_list.
*
* If mcast_list is NULL then all are retracted.
- *
- * Do not call outside of the mcast worker! (or cancel mcast worker first)
*/
static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv,
struct hlist_head *mcast_list)
@@ -323,8 +321,6 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv,
struct batadv_hw_addr *mcast_entry;
struct hlist_node *tmp;
- WARN_ON(delayed_work_pending(&bat_priv->mcast.work));
-
hlist_for_each_entry_safe(mcast_entry, tmp, &bat_priv->mcast.mla_list,
list) {
if (mcast_list &&
@@ -348,8 +344,6 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv,
*
* Adds multicast listener announcements from the given mcast_list to the
* translation table if they have not been added yet.
- *
- * Do not call outside of the mcast worker! (or cancel mcast worker first)
*/
static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv,
struct hlist_head *mcast_list)
@@ -357,8 +351,6 @@ static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv,
struct batadv_hw_addr *mcast_entry;
struct hlist_node *tmp;
- WARN_ON(delayed_work_pending(&bat_priv->mcast.work));
-
if (!mcast_list)
return;
@@ -647,7 +639,10 @@ static void batadv_mcast_mla_update(struct work_struct *work)
priv_mcast = container_of(delayed_work, struct batadv_priv_mcast, work);
bat_priv = container_of(priv_mcast, struct batadv_priv, mcast);
+ spin_lock(&bat_priv->mcast.mla_lock);
__batadv_mcast_mla_update(bat_priv);
+ spin_unlock(&bat_priv->mcast.mla_lock);
+
batadv_mcast_start_timer(bat_priv);
}
@@ -1211,6 +1211,11 @@ struct batadv_priv_mcast {
/** @bridged: whether the soft interface has a bridge on top */
unsigned char bridged:1;
+ /**
+ * @mla_lock: a lock protecting mla_list and mla_flags
+ */
+ spinlock_t mla_lock;
+
/**
* @num_want_all_unsnoopables: number of nodes wanting unsnoopable IP
* traffic
Syzbot has reported some issues with the locking assumptions made for the multicast tt/tvlv worker: It was able to trigger the WARN_ON() in batadv_mcast_mla_tt_retract() and batadv_mcast_mla_tt_add(). While hard/not reproduceable for us so far it seems that the delayed_work_pending() we use might not be quite safe from reordering. Therefore this patch adds an explicit, new spinlock to protect the update of the mla_list and flags in bat_priv and then removes the WARN_ON(delayed_work_pending()). Reported-by: syzbot+83f2d54ec6b7e417e13f@syzkaller.appspotmail.com Reported-by: syzbot+050927a651272b145a5d@syzkaller.appspotmail.com Fixes: 40b384052672 ("batman-adv: Use own timer for multicast TT and TVLV updates") Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue> --- https://www.open-mesh.org/issues/369 https://www.open-mesh.org/issues/370 Remark: In the past, before 40b384052672, there things were actually explicitly locked under the tt.commit_lock. --- net/batman-adv/main.c | 1 + net/batman-adv/multicast.c | 11 +++-------- net/batman-adv/types.h | 5 +++++ 3 files changed, 9 insertions(+), 8 deletions(-)