diff options
author | Linus Lüssing <linus.luessing@c0d3.blue> | 2019-04-23 21:19:14 -0400 |
---|---|---|
committer | Simon Wunderlich <sw@simonwunderlich.de> | 2019-05-06 05:40:46 -0400 |
commit | a3c7cd0cdf1107f891aff847ad481e34df727055 (patch) | |
tree | 7e22ab7d0e0ec6a22b1095bd2af1b61bf79ad9a2 | |
parent | bdc76fd299600736e832f1525f4f23dd210b97cb (diff) |
batman-adv: mcast: fix multicast tt/tvlv worker locking
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
Reported-by: syzbot+979ffc89b87309b1b94b@syzkaller.appspotmail.com
Reported-by: syzbot+f9f3f388440283da2965@syzkaller.appspotmail.com
Fixes: cbebd363b2e9 ("batman-adv: Use own timer for multicast TT and TVLV updates")
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
-rw-r--r-- | net/batman-adv/main.c | 1 | ||||
-rw-r--r-- | net/batman-adv/multicast.c | 11 | ||||
-rw-r--r-- | net/batman-adv/types.h | 5 |
3 files changed, 9 insertions, 8 deletions
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index 75750870cf04..f8725786b596 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c | |||
@@ -161,6 +161,7 @@ int batadv_mesh_init(struct net_device *soft_iface) | |||
161 | spin_lock_init(&bat_priv->tt.commit_lock); | 161 | spin_lock_init(&bat_priv->tt.commit_lock); |
162 | spin_lock_init(&bat_priv->gw.list_lock); | 162 | spin_lock_init(&bat_priv->gw.list_lock); |
163 | #ifdef CONFIG_BATMAN_ADV_MCAST | 163 | #ifdef CONFIG_BATMAN_ADV_MCAST |
164 | spin_lock_init(&bat_priv->mcast.mla_lock); | ||
164 | spin_lock_init(&bat_priv->mcast.want_lists_lock); | 165 | spin_lock_init(&bat_priv->mcast.want_lists_lock); |
165 | #endif | 166 | #endif |
166 | spin_lock_init(&bat_priv->tvlv.container_list_lock); | 167 | spin_lock_init(&bat_priv->tvlv.container_list_lock); |
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c index f91b1b6265cf..1b985ab89c08 100644 --- a/net/batman-adv/multicast.c +++ b/net/batman-adv/multicast.c | |||
@@ -325,8 +325,6 @@ static void batadv_mcast_mla_list_free(struct hlist_head *mcast_list) | |||
325 | * translation table except the ones listed in the given mcast_list. | 325 | * translation table except the ones listed in the given mcast_list. |
326 | * | 326 | * |
327 | * If mcast_list is NULL then all are retracted. | 327 | * If mcast_list is NULL then all are retracted. |
328 | * | ||
329 | * Do not call outside of the mcast worker! (or cancel mcast worker first) | ||
330 | */ | 328 | */ |
331 | static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv, | 329 | static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv, |
332 | struct hlist_head *mcast_list) | 330 | struct hlist_head *mcast_list) |
@@ -334,8 +332,6 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv, | |||
334 | struct batadv_hw_addr *mcast_entry; | 332 | struct batadv_hw_addr *mcast_entry; |
335 | struct hlist_node *tmp; | 333 | struct hlist_node *tmp; |
336 | 334 | ||
337 | WARN_ON(delayed_work_pending(&bat_priv->mcast.work)); | ||
338 | |||
339 | hlist_for_each_entry_safe(mcast_entry, tmp, &bat_priv->mcast.mla_list, | 335 | hlist_for_each_entry_safe(mcast_entry, tmp, &bat_priv->mcast.mla_list, |
340 | list) { | 336 | list) { |
341 | if (mcast_list && | 337 | if (mcast_list && |
@@ -359,8 +355,6 @@ static void batadv_mcast_mla_tt_retract(struct batadv_priv *bat_priv, | |||
359 | * | 355 | * |
360 | * Adds multicast listener announcements from the given mcast_list to the | 356 | * Adds multicast listener announcements from the given mcast_list to the |
361 | * translation table if they have not been added yet. | 357 | * translation table if they have not been added yet. |
362 | * | ||
363 | * Do not call outside of the mcast worker! (or cancel mcast worker first) | ||
364 | */ | 358 | */ |
365 | static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv, | 359 | static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv, |
366 | struct hlist_head *mcast_list) | 360 | struct hlist_head *mcast_list) |
@@ -368,8 +362,6 @@ static void batadv_mcast_mla_tt_add(struct batadv_priv *bat_priv, | |||
368 | struct batadv_hw_addr *mcast_entry; | 362 | struct batadv_hw_addr *mcast_entry; |
369 | struct hlist_node *tmp; | 363 | struct hlist_node *tmp; |
370 | 364 | ||
371 | WARN_ON(delayed_work_pending(&bat_priv->mcast.work)); | ||
372 | |||
373 | if (!mcast_list) | 365 | if (!mcast_list) |
374 | return; | 366 | return; |
375 | 367 | ||
@@ -658,7 +650,10 @@ static void batadv_mcast_mla_update(struct work_struct *work) | |||
658 | priv_mcast = container_of(delayed_work, struct batadv_priv_mcast, work); | 650 | priv_mcast = container_of(delayed_work, struct batadv_priv_mcast, work); |
659 | bat_priv = container_of(priv_mcast, struct batadv_priv, mcast); | 651 | bat_priv = container_of(priv_mcast, struct batadv_priv, mcast); |
660 | 652 | ||
653 | spin_lock(&bat_priv->mcast.mla_lock); | ||
661 | __batadv_mcast_mla_update(bat_priv); | 654 | __batadv_mcast_mla_update(bat_priv); |
655 | spin_unlock(&bat_priv->mcast.mla_lock); | ||
656 | |||
662 | batadv_mcast_start_timer(bat_priv); | 657 | batadv_mcast_start_timer(bat_priv); |
663 | } | 658 | } |
664 | 659 | ||
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h index a21b34ed6548..ed0f6a519de5 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h | |||
@@ -1224,6 +1224,11 @@ struct batadv_priv_mcast { | |||
1224 | unsigned char bridged:1; | 1224 | unsigned char bridged:1; |
1225 | 1225 | ||
1226 | /** | 1226 | /** |
1227 | * @mla_lock: a lock protecting mla_list and mla_flags | ||
1228 | */ | ||
1229 | spinlock_t mla_lock; | ||
1230 | |||
1231 | /** | ||
1227 | * @num_want_all_unsnoopables: number of nodes wanting unsnoopable IP | 1232 | * @num_want_all_unsnoopables: number of nodes wanting unsnoopable IP |
1228 | * traffic | 1233 | * traffic |
1229 | */ | 1234 | */ |