| From 03531e27350ebc6472975aa9699a1313f8c3b15a Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 29 Nov 2021 16:47:34 -0800 |
| Subject: mwifiex: Fix possible ABBA deadlock |
| |
| From: Brian Norris <briannorris@chromium.org> |
| |
| [ Upstream commit 1b8bb8919ef81bfc8873d223b9361f1685f2106d ] |
| |
| Quoting Jia-Ju Bai <baijiaju1990@gmail.com>: |
| |
| mwifiex_dequeue_tx_packet() |
| spin_lock_bh(&priv->wmm.ra_list_spinlock); --> Line 1432 (Lock A) |
| mwifiex_send_addba() |
| spin_lock_bh(&priv->sta_list_spinlock); --> Line 608 (Lock B) |
| |
| mwifiex_process_sta_tx_pause() |
| spin_lock_bh(&priv->sta_list_spinlock); --> Line 398 (Lock B) |
| mwifiex_update_ralist_tx_pause() |
| spin_lock_bh(&priv->wmm.ra_list_spinlock); --> Line 941 (Lock A) |
| |
| Similar report for mwifiex_process_uap_tx_pause(). |
| |
| While the locking expectations in this driver are a bit unclear, the |
| Fixed commit only intended to protect the sta_ptr, so we can drop the |
| lock as soon as we're done with it. |
| |
| IIUC, this deadlock cannot actually happen, because command event |
| processing (which calls mwifiex_process_sta_tx_pause()) is |
| sequentialized with TX packet processing (e.g., |
| mwifiex_dequeue_tx_packet()) via the main loop (mwifiex_main_process()). |
| But it's good not to leave this potential issue lurking. |
| |
| Fixes: f0f7c2275fb9 ("mwifiex: minor cleanups w/ sta_list_spinlock in cfg80211.c") |
| Cc: Douglas Anderson <dianders@chromium.org> |
| Reported-by: TOTE Robot <oslab@tsinghua.edu.cn> |
| Link: https://lore.kernel.org/linux-wireless/0e495b14-efbb-e0da-37bd-af6bd677ee2c@gmail.com/ |
| Signed-off-by: Brian Norris <briannorris@chromium.org> |
| Reviewed-by: Douglas Anderson <dianders@chromium.org> |
| Signed-off-by: Kalle Valo <kvalo@kernel.org> |
| Link: https://lore.kernel.org/r/YaV0pllJ5p/EuUat@google.com |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/net/wireless/marvell/mwifiex/sta_event.c | 8 ++++++-- |
| 1 file changed, 6 insertions(+), 2 deletions(-) |
| |
| diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c b/drivers/net/wireless/marvell/mwifiex/sta_event.c |
| index 5fdffb114913d..fd12093863801 100644 |
| --- a/drivers/net/wireless/marvell/mwifiex/sta_event.c |
| +++ b/drivers/net/wireless/marvell/mwifiex/sta_event.c |
| @@ -364,10 +364,12 @@ static void mwifiex_process_uap_tx_pause(struct mwifiex_private *priv, |
| sta_ptr = mwifiex_get_sta_entry(priv, tp->peermac); |
| if (sta_ptr && sta_ptr->tx_pause != tp->tx_pause) { |
| sta_ptr->tx_pause = tp->tx_pause; |
| + spin_unlock_bh(&priv->sta_list_spinlock); |
| mwifiex_update_ralist_tx_pause(priv, tp->peermac, |
| tp->tx_pause); |
| + } else { |
| + spin_unlock_bh(&priv->sta_list_spinlock); |
| } |
| - spin_unlock_bh(&priv->sta_list_spinlock); |
| } |
| } |
| |
| @@ -399,11 +401,13 @@ static void mwifiex_process_sta_tx_pause(struct mwifiex_private *priv, |
| sta_ptr = mwifiex_get_sta_entry(priv, tp->peermac); |
| if (sta_ptr && sta_ptr->tx_pause != tp->tx_pause) { |
| sta_ptr->tx_pause = tp->tx_pause; |
| + spin_unlock_bh(&priv->sta_list_spinlock); |
| mwifiex_update_ralist_tx_pause(priv, |
| tp->peermac, |
| tp->tx_pause); |
| + } else { |
| + spin_unlock_bh(&priv->sta_list_spinlock); |
| } |
| - spin_unlock_bh(&priv->sta_list_spinlock); |
| } |
| } |
| } |
| -- |
| 2.34.1 |
| |