Re: [PATCH wireless-next v2 09/18] wifi: cfg80211: document wiphy mutex for radar/CAC events

[Brian Norris <briannorris@chromium.org>]

On Mon, Oct 05, 2026 at 07:52:11PM +0200, Johannes Berg wrote:
> Woah, thanks for looking through this! :-)

Ha, well sometimes I read stuff with the "mwifiex" keyword in it :)

> On Mon, 2026-10-05 at 10:46 -0700, Brian Norris wrote:
> > > The DFS state of channels and the CAC state of the wdev is
> > > protected by the wiphy mutex, so the radar and CAC events
> > > must be reported by drivers with the wiphy mutex held. In
> > > mac80211 we do this, but some drivers don't yet:
> > >  - mwifiex/nxpwifi have an event handling worker,
> > 
> > FWIW, one of the two contexts that call cfg80211_cac_event() in mwifiex
> > does *not* (by inspection) seem to hold this. 
> 
> Yeah I know - that's why I wrote "some drivers __don't__ yet".
> 
> It's always been broken though, and I kinda just wanted to move on. It's
> racy, but I think mostly wrt. the valid_links warning (which isn't
> relevant here) and the data accesses, nothing worse will happen.

OK.

> > Is this something you'd
> > prefer individual driver users/maintainers resolve?
> 
> I think so. Or we can discuss it should be async in cfg80211, or an
> async version? I guess first we should discuss how to solve it either
> way, and look at all the drivers that still have the issue.

At first, I thought it'd be trivial to just throw in
wiphy_lock()/unlock() in 1 or 2 places, similar to commit 0d7c2194f17c
("wifi: mwifiex: add missing locking for cfg80211 calls"). But I'm not
sure that's actually sound -- it might introduce some locking inversion
problems, where (for example) mwifiex_del_virtual_intf() expects to be
able to flush/destroy these workers, but it's already holding the wiphy
mutex.

(I wonder if commit 0d7c2194f17c is similarly unsound.)

Either I'm missing something (quite possible), or it'll take a little
more thought on what the right solution should be.

Anyway, I'm fine with your approach of document first, fix later. It's
hard to move anything if you have to reason through every crazy /
lightly-maintained driver for every problem.

> > >  void cfg80211_cac_event(struct net_device *netdev,
> > >  			const struct cfg80211_chan_def *chandef,
> > 
> > Should we add an assert to this API?
> > 
> > 	lockdep_assert_wiphy(wiphy);
> 
> Well I figured if I do that now then tools (and perhaps people) will
> start complaining and that'd just put more pressure on everyone to fix
> it ... that's why I didn't yet, but I guess I should've outlined that
> better in the commit message (even after a --- marker).

Ack.

Brian