Re: [PATCH] wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

Re: [PATCH] wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

[Ömer Mete Kaya]

Apologies for the noise — I forgot to mark the previous send as v2.
This is v2 of the patch, extending the fix to cover two additional
hung task reports (tun_chr_close and switchdev_deferred_process_work)
that share the same root cause.

Changes in v2:
- Added Reported-by/Closes for tun_chr_close (b0ae8f1a)
- Added Reported-by/Closes for switchdev_deferred_process_work (d6bbe0f5)
- Added linux-kernel@vger.kernel.org to CC
- Updated "three separate" to "five separate" in commit message

In-Reply-To: <20260903111705.472491-1-omermetekaya0@gmail.com>

[PATCH v2] wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

[Ömer Mete Kaya]

reg_check_chans_work() holds rtnl_mutex for the entire duration of
iterating over all registered devices and calling cfg80211_leave() on
each invalid wdev. cfg80211_leave() can be slow (disconnect, stop AP,
leave mesh), causing rtnl_mutex starvation when many wireless interfaces
are present. This results in tasks waiting for rtnl_mutex for longer
than hung_task_timeout_secs:

  INFO: task hung in inet_rtm_newaddr
  INFO: task hung in inet6_rtm_newaddr
  INFO: task hung in nsim_destroy
  INFO: task hung in tun_chr_close
  INFO: task hung in switchdev_deferred_process_work

Fix by walking cfg80211_rdev_list under RCU and acquiring rtnl per-device,
so other rtnl waiters get a chance to run between devices.

I could not add the Fixes: tag because this patch addresses five separate
hung task reports whose cause bisections all failed, making it impossible
to identify a single introducing commit.

Reported-by: syzbot+adeb8550754921fece20@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=adeb8550754921fece20
Reported-by: syzbot+101224300649c3eb8af4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=101224300649c3eb8af4
Reported-by: syzbot+8141dcbd23a8f857798a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8141dcbd23a8f857798a
Reported-by: syzbot+b0ae8f1abf7d891e0426@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b0ae8f1abf7d891e0426
Reported-by: syzbot+d6bbe0f5705cb8a5aa2b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d6bbe0f5705cb8a5aa2b
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
---
 net/wireless/reg.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index a8336baf85dc..a2e0d1cf8317 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2466,12 +2466,21 @@ static void reg_check_chans_work(struct work_struct *work)
 	struct cfg80211_registered_device *rdev;
 
 	pr_debug("Verifying active interfaces after reg change\n");
-	rtnl_lock();
-
-	for_each_rdev(rdev)
+	/*
+	 * Acquire rtnl per-device instead of holding it for the entire loop;
+	 * cfg80211_leave() can be slow and starve other rtnl waiters otherwise.
+	 * wiphy_unregister() holds rtnl across list_del_rcu() + synchronize_rcu(),
+	 * so rdev cannot be freed while we hold rtnl_lock() below.
+	 */
+	rcu_read_lock();
+	list_for_each_entry_rcu(rdev, &cfg80211_rdev_list, list) {
+		rcu_read_unlock();
+		rtnl_lock();
 		reg_leave_invalid_chans(&rdev->wiphy);
-
-	rtnl_unlock();
+		rtnl_unlock();
+		rcu_read_lock();
+	}
+	rcu_read_unlock();
 }
 
 void reg_check_channels(void)
-- 
2.55.0

Re: [PATCH v2] wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/20260903151542.486376-2-omermetekaya0@gmail.com?part=1
---
wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

This commit changes reg_check_chans_work() to iterate over cfg80211_rdev_list
using RCU instead of holding rtnl_mutex for the entire loop. It acquires
rtnl_mutex on a per-device basis to prevent task hangs.

> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index a8336baf85dc6..a2e0d1cf8317c 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -2466,12 +2466,21 @@ static void reg_check_chans_work(struct work_struct *work)
>  	struct cfg80211_registered_device *rdev;
>  
>  	pr_debug("Verifying active interfaces after reg change\n");
> -	rtnl_lock();
> -
> -	for_each_rdev(rdev)
> +	/*
> +	 * Acquire rtnl per-device instead of holding it for the entire loop;
> +	 * cfg80211_leave() can be slow and starve other rtnl waiters otherwise.
> +	 * wiphy_unregister() holds rtnl across list_del_rcu() + synchronize_rcu(),
> +	 * so rdev cannot be freed while we hold rtnl_lock() below.
> +	 */
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(rdev, &cfg80211_rdev_list, list) {
> +		rcu_read_unlock();

[Severity: High]
Does dropping the RCU read lock here remove the lifetime protection of the
current rdev element while waiting for rtnl_lock()?

If a concurrent thread unregisters the device (for example, via
wiphy_unregister()), it could acquire rtnl_lock, remove the device with
list_del_rcu(), and call synchronize_rcu().

Because the RCU read lock was dropped by the work thread here,
synchronize_rcu() could complete and allow the unregister thread to free rdev
before this thread acquires the rtnl lock.

> +		rtnl_lock();
>  		reg_leave_invalid_chans(&rdev->wiphy);

[Severity: High]
Can this regression lead to a use-after-free?

If rdev was freed during the race window while waiting for rtnl_lock(),
accessing rdev->wiphy here would read from freed memory.

> -
> -	rtnl_unlock();
> +		rtnl_unlock();
> +		rcu_read_lock();
> +	}

[Severity: High]
Will the list_for_each_entry_rcu() macro implicitly dereference
rdev->list.next to continue the iteration after the loop body finishes?

If rdev was freed while the lock was dropped, does this result in another
use-after-free when advancing to the next list element?

> +	rcu_read_unlock();
>  }

Re: [PATCH v2] wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

[Ömer Mete Kaya]

Thank you for the review, Simon! You are correct, the previous v2
had a UAF window between rcu_read_unlock() and rtnl_lock().

v3 fixes this by taking a snapshot of cfg80211_rdev_list under RCU
with get_device() holding a reference on each rdev, preventing freeing
between per-device rtnl acquisitions. After acquiring rtnl per-device,
wiphy.registered is checked (set under rtnl in wiphy_unregister()) to
safely skip any unregistered device.

Changes in v3:
- Replace RCU drop/reacquire pattern with get_device() snapshot
- Add wiphy.registered check after per-device rtnl_lock()
- Add Fixes: f7e60032c661 tag
- Update commit message to describe new approach
 
In-Reply-To: <20260903151542.486376-2-omermetekaya0@gmail.com>

[PATCH] wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

[Ömer Mete Kaya]

reg_check_chans_work() holds rtnl_mutex for the entire duration of
iterating over all registered devices and calling cfg80211_leave() on
each invalid wdev. cfg80211_leave() can be slow (disconnect, stop AP,
leave mesh), causing rtnl_mutex starvation when many wireless interfaces
are present. This results in tasks waiting for rtnl_mutex for longer
than hung_task_timeout_secs:

  INFO: task hung in inet_rtm_newaddr
  INFO: task hung in inet6_rtm_newaddr
  INFO: task hung in nsim_destroy
  INFO: task hung in tun_chr_close
  INFO: task hung in switchdev_deferred_process_work

Fix by taking a snapshot of cfg80211_rdev_list under RCU, holding a
device reference (get_device/put_device) to prevent freeing, then
acquiring rtnl per-device so other rtnl waiters get a chance to run
between devices. After acquiring rtnl, wiphy.registered is checked to
skip any device that was unregistered in the meantime.

Fixes: f7e60032c661 ("wifi: cfg80211: fix locking in regulatory disconnect")
Reported-by: syzbot+adeb8550754921fece20@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=adeb8550754921fece20
Reported-by: syzbot+101224300649c3eb8af4@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=101224300649c3eb8af4
Reported-by: syzbot+8141dcbd23a8f857798a@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8141dcbd23a8f857798a
Reported-by: syzbot+b0ae8f1abf7d891e0426@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b0ae8f1abf7d891e0426
Reported-by: syzbot+d6bbe0f5705cb8a5aa2b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d6bbe0f5705cb8a5aa2b
Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>
---
 net/wireless/reg.c | 44 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index a8336baf85dc..7a3f1e470e14 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2463,15 +2463,49 @@ static void reg_leave_invalid_chans(struct wiphy *wiphy)
 
 static void reg_check_chans_work(struct work_struct *work)
 {
-	struct cfg80211_registered_device *rdev;
+	struct cfg80211_registered_device *rdev, **snap;
+	int i, n = 0;
 
 	pr_debug("Verifying active interfaces after reg change\n");
-	rtnl_lock();
 
-	for_each_rdev(rdev)
-		reg_leave_invalid_chans(&rdev->wiphy);
+	/*
+	 * Snapshot rdev pointers under RCU with a device reference so they
+	 * cannot be freed between per-device rtnl acquisitions. Using a
+	 * per-device rtnl_lock() instead of holding it across all devices
+	 * avoids starving other rtnl waiters when cfg80211_leave() is slow.
+	 */
+	rcu_read_lock();
+	list_for_each_entry_rcu(rdev, &cfg80211_rdev_list, list)
+		n++;
+	rcu_read_unlock();
 
-	rtnl_unlock();
+	if (!n)
+		return;
+
+	snap = kmalloc_array(n, sizeof(*snap), GFP_KERNEL);
+	if (!snap)
+		return;
+
+	i = 0;
+	rcu_read_lock();
+	list_for_each_entry_rcu(rdev, &cfg80211_rdev_list, list) {
+		if (i >= n)
+			break;
+		get_device(&rdev->wiphy.dev);
+		snap[i++] = rdev;
+	}
+	rcu_read_unlock();
+	n = i;
+
+	for (i = 0; i < n; i++) {
+		rtnl_lock();
+		if (snap[i]->wiphy.registered)
+			reg_leave_invalid_chans(&snap[i]->wiphy);
+		rtnl_unlock();
+		put_device(&snap[i]->wiphy.dev);
+	}
+
+	kfree(snap);
 }
 
 void reg_check_channels(void)
-- 
2.55.0

Re: [PATCH] wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

[Johannes Berg]

On Sun, 2026-09-06 at 03:25 +0300, Ömer Mete Kaya wrote:
> 
> +	/*
> +	 * Snapshot rdev pointers under RCU with a device reference so they
> +	 * cannot be freed between per-device rtnl acquisitions. Using a
> +	 * per-device rtnl_lock() instead of holding it across all devices
> +	 * avoids starving other rtnl waiters when cfg80211_leave() is slow.
> +	 */
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(rdev, &cfg80211_rdev_list, list)
> +		n++;
> +	rcu_read_unlock();
>  
> -	rtnl_unlock();
> +	if (!n)
> +		return;
> +
> +	snap = kmalloc_array(n, sizeof(*snap), GFP_KERNEL);
> +	if (!snap)
> +		return;

I really don't know _what_ you're doing here, or more realistically what
exactly you're asking your LLM to do here, but it's pointless. Please
just stop, you're not helping in any way.

johannes

Re: [PATCH] wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider:

Critical: 0 · High: 2 · Medium: 1 · Low: 0

- [High] The patch only splits the rtnl critical section at wiphy
  granularity: rtnl_lock() is taken once per snapshot entry and held…
- [High] If a new wiphy device is registered between the two RCU read
  sections, the array population loop will break early and silently skip…
- [Medium] reg_check_chans_work() (net/wireless/reg.c:2464) newly depends
  on a kmalloc_array() allocation and silently abandons the entire…

--

wifi: cfg80211: avoid holding rtnl_mutex across all cfg80211_leave() calls

This changes reg_check_chans_work() to snapshot cfg80211_rdev_list under RCU
with a device reference on each entry, then take rtnl_lock() once per device
instead of once for the whole walk.  A wiphy.registered check is added after
each rtnl acquisition to skip devices that went away in the meantime.

Closes: https://syzkaller.appspot.com/bug?extid=adeb8550754921fece20
Closes: https://syzkaller.appspot.com/bug?extid=101224300649c3eb8af4
Closes: https://syzkaller.appspot.com/bug?extid=8141dcbd23a8f857798a
Closes: https://syzkaller.appspot.com/bug?extid=b0ae8f1abf7d891e0426
Closes: https://syzkaller.appspot.com/bug?extid=d6bbe0f5705cb8a5aa2b

> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index a8336baf85dc6..7a3f1e470e143 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -2463,15 +2463,49 @@ static void reg_leave_invalid_chans(struct wiphy *wiphy)
>  
>  static void reg_check_chans_work(struct work_struct *work)
>  {
> -	struct cfg80211_registered_device *rdev;
> +	struct cfg80211_registered_device *rdev, **snap;
> +	int i, n = 0;
>  
>  	pr_debug("Verifying active interfaces after reg change\n");
> -	rtnl_lock();
>  
> -	for_each_rdev(rdev)
> -		reg_leave_invalid_chans(&rdev->wiphy);
> +	/*
> +	 * Snapshot rdev pointers under RCU with a device reference so they
> +	 * cannot be freed between per-device rtnl acquisitions. Using a
> +	 * per-device rtnl_lock() instead of holding it across all devices
> +	 * avoids starving other rtnl waiters when cfg80211_leave() is slow.
> +	 */
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(rdev, &cfg80211_rdev_list, list)
> +		n++;
> +	rcu_read_unlock();
>  
> -	rtnl_unlock();
> +	if (!n)
> +		return;
> +
> +	snap = kmalloc_array(n, sizeof(*snap), GFP_KERNEL);
> +	if (!snap)
> +		return;

[Severity: Medium]
Does this silently drop the whole regulatory enforcement pass when the
allocation fails?

Before this change reg_check_chans_work() could not fail: it was just
rtnl_lock() + for_each_rdev() + reg_leave_invalid_chans().  Now a transient
kmalloc_array() failure returns without touching any wiphy, so
reg_leave_invalid_chans() never runs for any device in this round.

The only place that queues the work is reg_check_channels():

net/wireless/reg.c:reg_check_channels() {
	mod_delayed_work(system_power_efficient_wq,
			 &reg_check_chans,
			 msecs_to_jiffies(REG_ENFORCE_GRACE_MS));
}

and its callers are all regulatory-change events (set_reg request, country
IE, driver hint, indoor flag).  Nothing re-arms the work on this failure
path, so interfaces that the newly applied regdomain forbids (AP beaconing,
associated STA, joined mesh) would keep running until some unrelated later
regdomain change happens to queue the work again.

Would rescheduling the work on allocation failure, or avoiding the
snapshot array entirely (for example restarting from the list head using
the rdev list generation counter), be preferable here?

> +
> +	i = 0;
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(rdev, &cfg80211_rdev_list, list) {
> +		if (i >= n)
> +			break;
> +		get_device(&rdev->wiphy.dev);
> +		snap[i++] = rdev;
> +	}
> +	rcu_read_unlock();
> +	n = i;
> +
> +	for (i = 0; i < n; i++) {
> +		rtnl_lock();
> +		if (snap[i]->wiphy.registered)
> +			reg_leave_invalid_chans(&snap[i]->wiphy);
> +		rtnl_unlock();
> +		put_device(&snap[i]->wiphy.dev);
> +	}

[Severity: High]
The count pass and the fill pass are two separate RCU read
sections with nothing held in between, so n can be stale by the
time snap[] is populated - and the truncation is silent.

wiphy_register() publishes a new radio with
list_add_rcu(&rdev->list, &cfg80211_rdev_list), i.e. at the head
of the list.  If a device is registered (USB adapter plugged in, a
new radio created) between the two rcu_read_lock() sections, the
second list_for_each_entry_rcu() starts at that brand new rdev,
fills snap[] with the first n entries and then hits the i >= n
break.  The entries that fall off are the ones at the *tail*, i.e.
the oldest radios - exactly the ones most likely to have
associated STAs, beaconing APs or joined meshes that the new
regdomain forbids.  reg_leave_invalid_chans() is never called for
them in this round, and since reg_check_channels() is only armed
from regulatory-change events, nothing retries; those interfaces
keep operating on channels the new regdomain disallows.

Note this is a functional regression against the previous
for_each_rdev() walk under rtnl, which by construction saw a
consistent list.

cfg80211_rdev_list is written under rtnl, so would it be simpler
to build the snapshot in a single pass under rtnl_lock() (counting
and get_device() together, or via cfg80211_rdev_list_generation),
then drop rtnl and re-acquire it per device for the
reg_leave_invalid_chans() calls?  If you want to keep the RCU
two-pass form, what is the plan for the list growing between the
passes - re-count and retry, or over-allocate and re-check?

[Severity: High]
Does this actually bound the rtnl hold time?  The split here is at wiphy
granularity only: rtnl is taken once and held across the complete
reg_leave_invalid_chans() pass for that wiphy.

net/wireless/reg.c:reg_leave_invalid_chans() {
	list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) {
		bool valid;

		scoped_guard(wiphy, wiphy)
			valid = reg_wdev_chan_valid(wiphy, wdev);
		if (!valid)
			cfg80211_leave(rdev, wdev, -1);
	}
}

That inner loop is unchanged and can call cfg80211_leave() an unbounded
number of times, and each call goes into driver ops:

reg_leave_invalid_chans() -> cfg80211_leave() -> cfg80211_leave_locked() ->
cfg80211_disconnect() / cfg80211_stop_ap() / cfg80211_leave_mesh() /
cfg80211_stop_nan()

with cfg80211_leave() itself requiring rtnl:

net/wireless/core.c:cfg80211_leave() {
	ASSERT_RTNL();
	...
	guard(wiphy)(&rdev->wiphy);

	cfg80211_leave_locked(rdev, wdev, link_id);
}

So when the interfaces are concentrated on one wiphy (many wdevs on a single
radio, or one slow leave op), the longest contiguous rtnl section is the same
as before, and rtnl waiters such as inet_rtm_newaddr, tun_chr_close and
nsim_destroy can still exceed hung_task_timeout_secs.  Is that consistent
with the commit message claim that the reported hangs are closed?

Wouldn't a fix need to drop and re-acquire rtnl between wdevs, with
per-wdev re-validation after re-acquiring (re-lookup by wiphy_idx plus a
wdev registered re-check), rather than only between devices?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906002657.620076-2-omermetekaya0%40gmail.com