Re: [PATCH 3/3] wifi: cfg80211: check if AP has been started before adding new station

[Slawomir Stepien <sst@poczta.fm>]

On wrz 08, 2026 14:25, Johannes Berg wrote:
> On Tue, 2026-09-08 at 13:59 +0200, Slawomir Stepien wrote:
> > > 
> > > Don't think you're missing anything, there's just the block handling
> > > param->vlan in add_station, it's dead code now. But why not kill it?
> > 
> > For the reason explained below (in my previous message).
> > There can still be a command from userspace with AP base interface (as wdev) with
> > attrs[NL80211_ATTR_STA_VLAN] set and this block will be executed in such case. Right?
> 
> D'oh. Going too fast, I somehow managed to not connect the two things.
> Sorry!
> 
> But it's another messy code path - if you have AP A and B, and VLANs A'
> and B' belonging to A and B respectively, then you can
> 
> 	add_station(A, vlan=B')
> 
> and it gets added on B/B', and B must be IFF_UP (because B' must be
> IFF_UP) but doesn't need to be beaconing and we're back to the bug, it
> seems? Maybe I'm missing something.

You can't add a vlan interface that dosn't have correct "parent" wiphy, get_vlan() checks that
already:

	v = dev_get_by_index(genl_info_net(info), nla_get_u32(vlanattr));
	if (!v)
		return ERR_PTR(-ENODEV);

	if (!v->ieee80211_ptr || v->ieee80211_ptr->wiphy != &rdev->wiphy) {
		ret = -EINVAL;
		goto error;
	}

> Since VLANs have to have the same local address as their AP (i.e. we
> need addr(B') == addr(B)), I guess get_vlan() could check this though,
> if it's indeed a problem, and that would avoid it through the chain of
> other invariants (IFF_UP, beaconing, etc.)

This check I've pasted should be good enough, right?

-- 
Slawomir Stepien