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@xxxxxxxxx?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();
>  }