* [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain
@ 2026-07-07 13:49 Cen Zhang
2026-07-07 14:38 ` Ben Greear
2026-07-07 17:31 ` Johannes Berg
0 siblings, 2 replies; 6+ messages in thread
From: Cen Zhang @ 2026-07-07 13:49 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, baijiaju1990, zzzccc427
cfg80211_process_wiphy_works() has a runaway guard for the process-all
case. When the guard fires, it drops the remaining queued work by
reinitializing rdev->wiphy_work_list.
That only resets the list head. The queued struct wiphy_work entries are
embedded in their owners, and their entry fields still point at the old
list neighbors. Later queue or cancel checks can then see a work item as
listed even though the rdev list has been cleared.
Drain the remaining list with list_del_init() under wiphy_work_lock
instead of reinitializing only the head. This keeps the existing WARN and
drop behavior while leaving each work item in the same state as normal
wiphy work removal.
Fixes: a3ee4dc84c4e ("wifi: cfg80211: add a work abstraction with special semantics")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
net/wireless/core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 3dcf63b04c41..a7e011ed455d 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1278,8 +1278,14 @@ void cfg80211_process_wiphy_works(struct cfg80211_registered_device *rdev,
if (wk == end)
break;
- if (WARN_ON(--runaway_limit == 0))
- INIT_LIST_HEAD(&rdev->wiphy_work_list);
+ if (WARN_ON(--runaway_limit == 0)) {
+ while (!list_empty(&rdev->wiphy_work_list)) {
+ wk = list_first_entry(&rdev->wiphy_work_list,
+ struct wiphy_work,
+ entry);
+ list_del_init(&wk->entry);
+ }
+ }
}
spin_unlock_irqrestore(&rdev->wiphy_work_lock, flags);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain
2026-07-07 13:49 [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain Cen Zhang
@ 2026-07-07 14:38 ` Ben Greear
2026-07-07 15:47 ` Cen Zhang
2026-07-07 17:31 ` Johannes Berg
1 sibling, 1 reply; 6+ messages in thread
From: Ben Greear @ 2026-07-07 14:38 UTC (permalink / raw)
To: Cen Zhang, Johannes Berg; +Cc: linux-wireless, baijiaju1990
On 7/7/26 06:49, Cen Zhang wrote:
> cfg80211_process_wiphy_works() has a runaway guard for the process-all
> case. When the guard fires, it drops the remaining queued work by
> reinitializing rdev->wiphy_work_list.
Even this likely leaves the system in a bad state since requested work
items would be skipped. Probably should also increase runaway_limit quite
a bit as well (that has worked OK for me), maybe remove it entirely.
Thanks,
Ben
>
> That only resets the list head. The queued struct wiphy_work entries are
> embedded in their owners, and their entry fields still point at the old
> list neighbors. Later queue or cancel checks can then see a work item as
> listed even though the rdev list has been cleared.
>
> Drain the remaining list with list_del_init() under wiphy_work_lock
> instead of reinitializing only the head. This keeps the existing WARN and
> drop behavior while leaving each work item in the same state as normal
> wiphy work removal.
>
> Fixes: a3ee4dc84c4e ("wifi: cfg80211: add a work abstraction with special semantics")
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> net/wireless/core.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/wireless/core.c b/net/wireless/core.c
> index 3dcf63b04c41..a7e011ed455d 100644
> --- a/net/wireless/core.c
> +++ b/net/wireless/core.c
> @@ -1278,8 +1278,14 @@ void cfg80211_process_wiphy_works(struct cfg80211_registered_device *rdev,
> if (wk == end)
> break;
>
> - if (WARN_ON(--runaway_limit == 0))
> - INIT_LIST_HEAD(&rdev->wiphy_work_list);
> + if (WARN_ON(--runaway_limit == 0)) {
> + while (!list_empty(&rdev->wiphy_work_list)) {
> + wk = list_first_entry(&rdev->wiphy_work_list,
> + struct wiphy_work,
> + entry);
> + list_del_init(&wk->entry);
> + }
> + }
> }
> spin_unlock_irqrestore(&rdev->wiphy_work_lock, flags);
> }
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain
2026-07-07 14:38 ` Ben Greear
@ 2026-07-07 15:47 ` Cen Zhang
2026-07-07 16:03 ` Ben Greear
0 siblings, 1 reply; 6+ messages in thread
From: Cen Zhang @ 2026-07-07 15:47 UTC (permalink / raw)
To: Ben Greear; +Cc: Johannes Berg, linux-wireless, baijiaju1990
Hi Ben,
Thanks for your review and comments.
> Even this likely leaves the system in a bad state since requested work
> items would be skipped. Probably should also increase runaway_limit quite
> a bit as well (that has worked OK for me), maybe remove it entirely.
Fair. Hitting the guard still means pending work gets skipped; this patch only
avoids leaving those work entries with stale list links after the
existing drop path.
I was hesitant to remove the guard in the same change, because the
flush-all paths run under the wiphy mutex, and a work item that keeps
requeueing itself could otherwise keep flush, suspend, or unregister
stuck there. Raising the limit sounds less risky, but I would want a
concrete reason for the new value rather than picking one arbitrarily..
Best Regards,
Cen Zhang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain
2026-07-07 15:47 ` Cen Zhang
@ 2026-07-07 16:03 ` Ben Greear
0 siblings, 0 replies; 6+ messages in thread
From: Ben Greear @ 2026-07-07 16:03 UTC (permalink / raw)
To: Cen Zhang; +Cc: Johannes Berg, linux-wireless, baijiaju1990
On 7/7/26 08:47, Cen Zhang wrote:
> Hi Ben,
>
> Thanks for your review and comments.
>
>> Even this likely leaves the system in a bad state since requested work
>> items would be skipped. Probably should also increase runaway_limit quite
>> a bit as well (that has worked OK for me), maybe remove it entirely.
>
> Fair. Hitting the guard still means pending work gets skipped; this patch only
> avoids leaving those work entries with stale list links after the
> existing drop path.
>
> I was hesitant to remove the guard in the same change, because the
> flush-all paths run under the wiphy mutex, and a work item that keeps
> requeueing itself could otherwise keep flush, suspend, or unregister
> stuck there. Raising the limit sounds less risky, but I would want a
> concrete reason for the new value rather than picking one arbitrarily..
Yeah, you have a good point. I'm not sure of a proper solution. I set my limit to 200
and was no longer able to reproduce warning splats.
Your patch may certainly be an improvement.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain
2026-07-07 13:49 [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain Cen Zhang
2026-07-07 14:38 ` Ben Greear
@ 2026-07-07 17:31 ` Johannes Berg
2026-07-08 3:04 ` Cen Zhang
1 sibling, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2026-07-07 17:31 UTC (permalink / raw)
To: Cen Zhang; +Cc: linux-wireless, baijiaju1990
On Tue, 2026-07-07 at 21:49 +0800, Cen Zhang wrote:
> cfg80211_process_wiphy_works() has a runaway guard for the process-all
> case. When the guard fires, it drops the remaining queued work by
> reinitializing rdev->wiphy_work_list.
>
> That only resets the list head. The queued struct wiphy_work entries are
> embedded in their owners, and their entry fields still point at the old
> list neighbors. Later queue or cancel checks can then see a work item as
> listed even though the rdev list has been cleared.
Yeah ... that's not ideal, but also maybe part of the point. IIRC we've
seen cases where works are accidentally re-initialised and we get into
infinite loops iterating the list, or so.
So if everything else goes according to plan, your fix is certainly
correct. If we get some kind of list corruption in the first place, then
it's potentially going to make it worse.
Maybe at this point it's just worth bumping it to a few tens of thousand
iterations and doing BUG()? I don't think loop detection is simple here.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain
2026-07-07 17:31 ` Johannes Berg
@ 2026-07-08 3:04 ` Cen Zhang
0 siblings, 0 replies; 6+ messages in thread
From: Cen Zhang @ 2026-07-08 3:04 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, baijiaju1990
Hi Johannes,
Thanks for your review and explanation.
> Yeah ... that's not ideal, but also maybe part of the point. IIRC we've
> seen cases where works are accidentally re-initialised and we get into
> infinite loops iterating the list, or so.
>
> So if everything else goes according to plan, your fix is certainly
> correct. If we get some kind of list corruption in the first place, then
> it's potentially going to make it worse.
Fair enough, thanks.
> Maybe at this point it's just worth bumping it to a few tens of thousand
> iterations and doing BUG()? I don't think loop detection is simple here.
I'll respin v2 with a much higher limit and BUG() if it is still hit.
Best regards,
Cen Zhang
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-08 3:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 13:49 [PATCH] wifi: cfg80211: reinit wiphy work entries on runaway drain Cen Zhang
2026-07-07 14:38 ` Ben Greear
2026-07-07 15:47 ` Cen Zhang
2026-07-07 16:03 ` Ben Greear
2026-07-07 17:31 ` Johannes Berg
2026-07-08 3:04 ` Cen Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox