| From a3c7cd0cdf1107f891aff847ad481e34df727055 Mon Sep 17 00:00:00 2001 |
| From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue> |
| Date: Wed, 24 Apr 2019 03:19:14 +0200 |
| Subject: batman-adv: mcast: fix multicast tt/tvlv worker locking |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=UTF-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Linus Lüssing <linus.luessing@c0d3.blue> |
| |
| commit a3c7cd0cdf1107f891aff847ad481e34df727055 upstream. |
| |
| 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> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| net/batman-adv/main.c | 1 + |
| net/batman-adv/multicast.c | 11 +++-------- |
| net/batman-adv/types.h | 2 ++ |
| 3 files changed, 6 insertions(+), 8 deletions(-) |
| |
| --- a/net/batman-adv/main.c |
| +++ b/net/batman-adv/main.c |
| @@ -153,6 +153,7 @@ int batadv_mesh_init(struct net_device * |
| 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); |
| --- a/net/batman-adv/multicast.c |
| +++ b/net/batman-adv/multicast.c |
| @@ -269,8 +269,6 @@ static void batadv_mcast_mla_list_free(s |
| * 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) |
| @@ -278,8 +276,6 @@ static void batadv_mcast_mla_tt_retract( |
| 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 && |
| @@ -303,8 +299,6 @@ static void batadv_mcast_mla_tt_retract( |
| * |
| * 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) |
| @@ -312,8 +306,6 @@ static void batadv_mcast_mla_tt_add(stru |
| struct batadv_hw_addr *mcast_entry; |
| struct hlist_node *tmp; |
| |
| - WARN_ON(delayed_work_pending(&bat_priv->mcast.work)); |
| - |
| if (!mcast_list) |
| return; |
| |
| @@ -600,7 +592,10 @@ static void batadv_mcast_mla_update(stru |
| 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); |
| } |
| |
| --- a/net/batman-adv/types.h |
| +++ b/net/batman-adv/types.h |
| @@ -798,6 +798,7 @@ struct batadv_mcast_querier_state { |
| * @flags: the flags we have last sent in our mcast tvlv |
| * @enabled: whether the multicast tvlv is currently enabled |
| * @bridged: whether the soft interface has a bridge on top |
| + * @mla_lock: a lock protecting mla_list and mla_flags |
| * @num_disabled: number of nodes that have no mcast tvlv |
| * @num_want_all_unsnoopables: number of nodes wanting unsnoopable IP traffic |
| * @num_want_all_ipv4: counter for items in want_all_ipv4_list |
| @@ -816,6 +817,7 @@ struct batadv_priv_mcast { |
| u8 flags; |
| bool enabled; |
| bool bridged; |
| + spinlock_t mla_lock; |
| atomic_t num_disabled; |
| atomic_t num_want_all_unsnoopables; |
| atomic_t num_want_all_ipv4; |