* [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure
[not found] <20261112131401.236433-1-skalpesh942@gmail.com>
@ 2026-11-12 13:13 ` Kalpesh Solanki
2026-11-12 21:28 ` Luka Gejak
2026-11-13 6:52 ` Zong-Zhe Yang
2026-11-12 13:14 ` [PATCH v5 2/3] wifi: rtw88: skip already reset station in rtw_sta_remove() Kalpesh Solanki
1 sibling, 2 replies; 7+ messages in thread
From: Kalpesh Solanki @ 2026-11-12 13:13 UTC (permalink / raw)
To: pkshih, luka.gejak
Cc: kvalo, linux-wireless, linux-kernel, Kalpesh Solanki, stable
When mac80211 stops or suspends the device without WoWLAN,
drv_reconfig_complete() is never invoked with
IEEE80211_RECONFIG_TYPE_RESTART. If firmware recovery was in flight
during this transition, RTW_FLAG_RESTARTING remains set permanently,
blocking all subsequent firmware recoveries until the module is reloaded.
Similarly, if rtw_core_start() fails during drv_start() in a recovery
restart, local->started remains false. When mac80211 cleans up via
cfg80211_shutdown_all_interfaces(), drv_stop() returns early without
calling rtw_ops_stop(), leaving RTW_FLAG_RESTARTING set permanently on
hardware that failed to recover.
Clear RTW_FLAG_RESTARTING in rtw_ops_stop(), as well as in
rtw_ops_start() when rtw_core_start() returns an error, ensuring the
driver always exits the restarting state on error or teardown.
Fixes: 5c831644e1f4 ("rtw88: handle and recover when firmware crash")
Cc: stable@vger.kernel.org
Signed-off-by: Kalpesh Solanki <skalpesh942@gmail.com>
---
drivers/net/wireless/realtek/rtw88/mac80211.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 5e40cf06a9e..ffd86f08185 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -57,6 +57,8 @@ static int rtw_ops_start(struct ieee80211_hw *hw)
mutex_lock(&rtwdev->mutex);
ret = rtw_core_start(rtwdev);
+ if (ret)
+ clear_bit(RTW_FLAG_RESTARTING, rtwdev->flags);
mutex_unlock(&rtwdev->mutex);
return ret;
@@ -67,6 +69,7 @@ static void rtw_ops_stop(struct ieee80211_hw *hw, bool suspend)
struct rtw_dev *rtwdev = hw->priv;
mutex_lock(&rtwdev->mutex);
+ clear_bit(RTW_FLAG_RESTARTING, rtwdev->flags);
rtw_core_stop(rtwdev);
mutex_unlock(&rtwdev->mutex);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 2/3] wifi: rtw88: skip already reset station in rtw_sta_remove()
[not found] <20261112131401.236433-1-skalpesh942@gmail.com>
2026-11-12 13:13 ` [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure Kalpesh Solanki
@ 2026-11-12 13:14 ` Kalpesh Solanki
2026-11-12 21:29 ` Luka Gejak
1 sibling, 1 reply; 7+ messages in thread
From: Kalpesh Solanki @ 2026-11-12 13:14 UTC (permalink / raw)
To: pkshih, luka.gejak
Cc: kvalo, linux-wireless, linux-kernel, Kalpesh Solanki, stable
During firmware recovery, __fw_recovery_work() iterates over all
stations and invokes rtw_sta_remove() via rtw_reset_sta_iter().
If recovery is triggered concurrently with station teardown (such as
during ieee80211_set_disassoc() where mac80211 leaves LPS before
removing the AP station), rtw_ops_sta_remove() is subsequently called
for the same station. This causes rtw_sta_remove() to execute twice,
leading to a double free of si->mask and an underflow of rtwdev->sta_cnt.
Check si->vif in rtw_sta_remove() and return early if the station was
already reset, setting si->vif and si->mask to NULL upon removal.
Fixes: 5c831644e1f4 ("rtw88: handle and recover when firmware crash")
Cc: stable@vger.kernel.org
Signed-off-by: Kalpesh Solanki <skalpesh942@gmail.com>
---
drivers/net/wireless/realtek/rtw88/main.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index f7708ad17b6..a74f199a93a 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -384,6 +384,9 @@ void rtw_sta_remove(struct rtw_dev *rtwdev, struct ieee80211_sta *sta,
struct ieee80211_vif *vif = si->vif;
int i;
+ if (!vif)
+ return;
+
cancel_work_sync(&si->rc_work);
if (vif->type != NL80211_IFTYPE_STATION || sta->tdls)
@@ -395,8 +398,10 @@ void rtw_sta_remove(struct rtw_dev *rtwdev, struct ieee80211_sta *sta,
rtw_txq_cleanup(rtwdev, sta->txq[i]);
kfree(si->mask);
+ si->mask = NULL;
rtwdev->sta_cnt--;
+ si->vif = NULL;
rtw_dbg(rtwdev, RTW_DBG_STATE, "sta %pM with macid %d left\n",
sta->addr, si->mac_id);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure
2026-11-12 13:13 ` [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure Kalpesh Solanki
@ 2026-11-12 21:28 ` Luka Gejak
2026-11-13 6:52 ` Zong-Zhe Yang
1 sibling, 0 replies; 7+ messages in thread
From: Luka Gejak @ 2026-11-12 21:28 UTC (permalink / raw)
To: Kalpesh Solanki, pkshih; +Cc: kvalo, linux-wireless, linux-kernel, stable
On November 12, 2026 2:13:59 PM GMT+01:00, Kalpesh Solanki
<skalpesh942@gmail.com> wrote:
>When mac80211 stops or suspends the device without WoWLAN,
>drv_reconfig_complete() is never invoked with
>IEEE80211_RECONFIG_TYPE_RESTART. If firmware recovery was in flight
>during this transition, RTW_FLAG_RESTARTING remains set permanently,
>blocking all subsequent firmware recoveries until the module is reloaded.
>
>Similarly, if rtw_core_start() fails during drv_start() in a recovery
>restart, local->started remains false. When mac80211 cleans up via
>cfg80211_shutdown_all_interfaces(), drv_stop() returns early without
>calling rtw_ops_stop(), leaving RTW_FLAG_RESTARTING set permanently on
>hardware that failed to recover.
>
>Clear RTW_FLAG_RESTARTING in rtw_ops_stop(), as well as in
>rtw_ops_start() when rtw_core_start() returns an error, ensuring the
>driver always exits the restarting state on error or teardown.
>
>Fixes: 5c831644e1f4 ("rtw88: handle and recover when firmware crash")
>Cc: stable@vger.kernel.org
>Signed-off-by: Kalpesh Solanki <skalpesh942@gmail.com>
>---
You seem to have missed it in v4 so please pick up this tag in v6.
Reviewed-by: Luka Gejak <luka.gejak@linux.dev>
Best regards,
Luka Gejak
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/3] wifi: rtw88: skip already reset station in rtw_sta_remove()
2026-11-12 13:14 ` [PATCH v5 2/3] wifi: rtw88: skip already reset station in rtw_sta_remove() Kalpesh Solanki
@ 2026-11-12 21:29 ` Luka Gejak
0 siblings, 0 replies; 7+ messages in thread
From: Luka Gejak @ 2026-11-12 21:29 UTC (permalink / raw)
To: Kalpesh Solanki, pkshih; +Cc: kvalo, linux-wireless, linux-kernel, stable
Hi Kalpesh,
Thanks for the new version. Two comments below.
On November 12, 2026 2:14:00 PM GMT+01:00, Kalpesh Solanki
<skalpesh942@gmail.com> wrote:
> diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
> index f7708ad17b6f..a74f199a93ab 100644
> --- a/drivers/net/wireless/realtek/rtw88/main.c
> +++ b/drivers/net/wireless/realtek/rtw88/main.c
> @@ -384,6 +384,9 @@ void rtw_sta_remove(struct rtw_dev *rtwdev, struct ieee80211_sta *sta,
> struct ieee80211_vif *vif = si->vif;
> int i;
>
> + if (!vif)
> + return;
> +
> cancel_work_sync(&si->rc_work);
I think my comment on v4 about this got missed:
https://lore.kernel.org/all/83E3139E-BE19-4EAD-9BCB-6DABBF8D1942@linux.dev/
rtw_ops_sta_rc_update() can still queue rc_work after the recovery has
reset the station. If the station is then removed before the restart
adds it back, this return skips the only cancel_work_sync() and
rtw_sta_rc_work() could run on a freed station. Moving the check below
cancel_work_sync() should be enough.
[ ... ]
> kfree(si->mask);
> + si->mask = NULL;
As far as I can see si->use_cfg_mask stays set here. mac80211 keeps the
same station across the restart, and when it adds it back, rtw_sta_add()
calls rtw_update_sta_info(), which gets to this:
rtw_rate_mask_cfg()
if (!si->use_cfg_mask)
return ra_mask;
...
cfg_mask = mask->control[band].legacy;
So if I read it right, for a station that got a bitrate mask, a recovery
now ends in a NULL pointer dereference, where before it read the freed
mask. Maybe clear si->use_cfg_mask together with si->mask? The mask is
then dropped after a recovery, since mac80211 doesn't pass it down again
on restart, but I think that is fine.
Best regards,
Luka Gejak
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure
2026-11-12 13:13 ` [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure Kalpesh Solanki
2026-11-12 21:28 ` Luka Gejak
@ 2026-11-13 6:52 ` Zong-Zhe Yang
2026-11-13 7:19 ` Luka Gejak
1 sibling, 1 reply; 7+ messages in thread
From: Zong-Zhe Yang @ 2026-11-13 6:52 UTC (permalink / raw)
To: Kalpesh Solanki, Ping-Ke Shih, luka.gejak@linux.dev
Cc: kvalo@kernel.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Kalpesh Solanki <skalpesh942@gmail.com> wrote:
>
> When mac80211 stops or suspends the device without WoWLAN,
> drv_reconfig_complete() is never invoked with
> IEEE80211_RECONFIG_TYPE_RESTART. If firmware recovery was in flight
> during this transition, RTW_FLAG_RESTARTING remains set permanently,
> blocking all subsequent firmware recoveries until the module is reloaded.
>
In this case, I saw you clear RTW_FLAG_RESTARTING in rtw_ops_stop.
However, the reconfig flow might not have even run at that moment.
How about clearing RTW_FLAG_RESTARTING when driver gets
IEEE80211_RECONFIG_TYPE_SUSPEND?
>
> Similarly, if rtw_core_start() fails during drv_start() in a recovery
> restart, local->started remains false. When mac80211 cleans up via
> cfg80211_shutdown_all_interfaces(), drv_stop() returns early without
> calling rtw_ops_stop(), leaving RTW_FLAG_RESTARTING set permanently on
> hardware that failed to recover.
>
> Clear RTW_FLAG_RESTARTING in rtw_ops_stop(), as well as in
> rtw_ops_start() when rtw_core_start() returns an error, ensuring the
> driver always exits the restarting state on error or teardown.
>
> Fixes: 5c831644e1f4 ("rtw88: handle and recover when firmware crash")
> Cc: stable@vger.kernel.org
> Signed-off-by: Kalpesh Solanki <skalpesh942@gmail.com>
> ---
> drivers/net/wireless/realtek/rtw88/mac80211.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c
> b/drivers/net/wireless/realtek/rtw88/mac80211.c
> index 5e40cf06a9e..ffd86f08185 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
> @@ -57,6 +57,8 @@ static int rtw_ops_start(struct ieee80211_hw *hw)
>
> mutex_lock(&rtwdev->mutex);
> ret = rtw_core_start(rtwdev);
> + if (ret)
> + clear_bit(RTW_FLAG_RESTARTING, rtwdev->flags);
> mutex_unlock(&rtwdev->mutex);
>
> return ret;
> @@ -67,6 +69,7 @@ static void rtw_ops_stop(struct ieee80211_hw *hw, bool
> suspend)
> struct rtw_dev *rtwdev = hw->priv;
>
> mutex_lock(&rtwdev->mutex);
> + clear_bit(RTW_FLAG_RESTARTING, rtwdev->flags);
> rtw_core_stop(rtwdev);
> mutex_unlock(&rtwdev->mutex);
> }
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure
2026-11-13 6:52 ` Zong-Zhe Yang
@ 2026-11-13 7:19 ` Luka Gejak
2026-11-13 8:22 ` Zong-Zhe Yang
0 siblings, 1 reply; 7+ messages in thread
From: Luka Gejak @ 2026-11-13 7:19 UTC (permalink / raw)
To: Zong-Zhe Yang, Kalpesh Solanki, Ping-Ke Shih
Cc: kvalo@kernel.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Hi Zong-Zhe,
Thanks for taking a look.
On November 13, 2026 7:52:23 AM GMT+01:00, Zong-Zhe Yang
<kevin_yang@realtek.com> wrote:
> In this case, I saw you clear RTW_FLAG_RESTARTING in rtw_ops_stop.
> However, the reconfig flow might not have even run at that moment.
>
> How about clearing RTW_FLAG_RESTARTING when driver gets
> IEEE80211_RECONFIG_TYPE_SUSPEND?
As far as I can see that misses the stop case. If the last interface
goes down during a recovery, the restart work finds open_count at 0 and
mac80211 sends neither RESTART nor SUSPEND, so the flag would stay set.
Suspend without WoWLAN also goes through drv_stop(), so rtw_ops_stop()
covers both.
Best regards,
Luka Gejak
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure
2026-11-13 7:19 ` Luka Gejak
@ 2026-11-13 8:22 ` Zong-Zhe Yang
0 siblings, 0 replies; 7+ messages in thread
From: Zong-Zhe Yang @ 2026-11-13 8:22 UTC (permalink / raw)
To: Luka Gejak, Kalpesh Solanki, Ping-Ke Shih
Cc: kvalo@kernel.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
Luka Gejak <luka.gejak@linux.dev> wrote:
>
> On November 13, 2026 7:52:23 AM GMT+01:00, Zong-Zhe Yang
> <kevin_yang@realtek.com> wrote:
> > In this case, I saw you clear RTW_FLAG_RESTARTING in rtw_ops_stop.
> > However, the reconfig flow might not have even run at that moment.
> >
> > How about clearing RTW_FLAG_RESTARTING when driver gets
> > IEEE80211_RECONFIG_TYPE_SUSPEND?
>
> As far as I can see that misses the stop case. If the last interface goes down
> during a recovery, the restart work finds open_count at 0 and
> mac80211 sends neither RESTART nor SUSPEND, so the flag would stay set.
> Suspend without WoWLAN also goes through drv_stop(), so rtw_ops_stop()
> covers both.
I missed to consider this corner case. My previous point was I don't want
RTW_FLAG_RESTARTING to be cleared too early (assumed open_count != 0).
I don't seem to have other way for this case except by clearing it in drv_stop().
So, keep the code. Thanks.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-11-13 8:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261112131401.236433-1-skalpesh942@gmail.com>
2026-11-12 13:13 ` [PATCH v5 1/3] wifi: rtw88: clear RTW_FLAG_RESTARTING on stop and start failure Kalpesh Solanki
2026-11-12 21:28 ` Luka Gejak
2026-11-13 6:52 ` Zong-Zhe Yang
2026-11-13 7:19 ` Luka Gejak
2026-11-13 8:22 ` Zong-Zhe Yang
2026-11-12 13:14 ` [PATCH v5 2/3] wifi: rtw88: skip already reset station in rtw_sta_remove() Kalpesh Solanki
2026-11-12 21:29 ` Luka Gejak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox