Re: [PATCH v2] mac80211: reject station addition if AP or MLO link is inactive
Johannes Berg <[email protected]> Mon, 03 Aug 2026 15:15:25 +0200
| Newsgroups | dev.linux.lists.syzbot,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <[email protected]> |
On Mon, 2026-08-03 at 15:09 +0200, Slawomir Stepien wrote: > On lip 31, 2026 14:40, Johannes Berg wrote: > > On Mon, 2026-07-27 at 15:26 +0200, Slawomir Stepien wrote: > > > After taking a closer look at this I wonder how AP_VLAN should be han= dled? Is there a way on > > > cfg80211 level to be able to check AP_VLAN's main device state? > >=20 > > Hmm, yeah that might be an issue? I guess in theory it could track it, > > but it doesn't. > >=20 > > > My code change would look something like this: > > >=20 > > > @@ -9404,6 +9404,12 @@ static int nl80211_new_station(struct sk_buff = *skb, struct genl_info *info) > > > case NL80211_IFTYPE_AP: > > > case NL80211_IFTYPE_AP_VLAN: > > > case NL80211_IFTYPE_P2P_GO: > > > + /* Add new station only after the AP and link has bee= n started */ > > > + int link =3D params.link_sta_params.link_id >=3D 0 ? > > > + params.link_sta_params.link_id : 0; > > > + if (!wdev->links[link].ap.beacon_interval) > > > + return -ENETDOWN; > > > + > > > /* ignore WME attributes if iface/sta is not capable = */ > > > if (!(rdev->wiphy.flags & WIPHY_FLAG_AP_UAPSD) || > > > !(params.sta_flags_set & BIT(NL80211_STA_FLAG_WME= ))) > > >=20 > > > but the NL80211_IFTYPE_AP_VLAN case would not work here, right? Can I= just do the checking only for > > > NL80211_IFTYPE_AP and NL80211_IFTYPE_P2P_GO and skip NL80211_IFTYPE_A= P_VLAN? > >=20 > > Yeah it'd just reject everything, I guess. > >=20 > > I think in practice stations are added at the AP interface first and > > then moved to a VLAN, which would argue for actually rejecting >=20 > But are they moved and then this `case NL80211_IFTYPE_AP_VLAN` should mat= ch? I'm not entirely sure, but I believe they're _added_ to the AP first, and then _modified_ to be in the AP_VLAN via another path (NL80211_CMD_SET_STATION, NL80211_ATTR_VLAN_ID). The question is whether or not we can rely on this, i.e. no userspace ever used the other path. > In my tests this case doesn't match, even after the STA is assoc with vla= n interface (e.g. > wlan0.10). Right. > My test is done using qemu with: I'd probably use UML/hwsim tests, but sure. > This index 4 is assigned to wlan0, so it will never have iftype =3D=3D NL= 80211_IFTYPE_AP_VLAN. > Also the print of wdev->identifier will give 0x1 and not 0x2. >=20 > So, is it even possible that `case NL80211_IFTYPE_AP_VLAN` would match? I= f it will never match, then > do I understand that the change I proposed should be correct for other ca= ses? Well, it's _possible_, right? It just appears to not be used in practice, as I described above (guess your test says I'm right). But then I think we should do two changes: 1) simply refuse adding stations to AP_VLAN, i.e. drop the NL80211_IFTYPE_AP_VLAN case entirely from the function 2) make the change you suggested validating the link Actually only validating the link as you showed would basically achieve (1) with -ENETDOWN (we've been using -ENOLINK btw) rather than with the -EOPNOTSUPP error from the default switch case, but it also seems less explicit... Or maybe just do both in one commit? johannes