On 03/22/2022, Jeff Johnson wrote:
> On 3/21/2022 1:07 PM, Johannes Berg wrote:
> [..snip..]
>
> > > I'm not an networking expert. So my main question is if I'm allowed to take
> > > the RTNL lock inside the nl80211_vendor_cmd callbacks?
> >
> > Evidently, you're not. It's interesting though, it used to be that we
> > called these with the RTNL held, now we don't, and the driver you're
> > using somehow "got fixed" to take it, but whoever fixed it didn't take
> > into account that this is not possible?
>
> On this point I just want to remind that prior to the locking change that a
> driver would specify on a per-vendor command basis whether or not it wanted
> the rtnl_lock to be held via NL80211_FLAG_NEED_RTNL. I'm guessing for the
> command in question the driver did not set this flag since the driver wanted
> to explicitly take the lock itself, otherwise it would have deadlocked on
> itself with the 5.10 kernel.
>
> /jeff
On the 5.10 kernel, the core kernel sets NL80211_FLAG_NEED_RTNL as part of
the internal_flags for NL80211_CMD_VENDOR:
net/wireless/nl80211.c:
{
.cmd = NL80211_CMD_VENDOR,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = nl80211_vendor_cmd,
.dumpit = nl80211_vendor_cmd_dump,
.flags = GENL_UNS_ADMIN_PERM,
.internal_flags = NL80211_FLAG_NEED_WIPHY |
NL80211_FLAG_NEED_RTNL |
NL80211_FLAG_CLEAR_SKB,
},
So the 5.10 version of this driver doesn't need to directly call rtnl_lock()
within the vendor command doit() functions since pre_doit() handles the RTNL
locking.
It would be nice if nl80211_vendor_cmd() could support taking the RTNL lock if
requested via the vendor flags. That would require moving the wiphy lock to
nl80211_vendor_cmds() so that it could take the RTNL and wiphy lock in the
correct order. Is that something you'd be open to Johannes?
--Will
