[PATCH 1/3] wifi: cfg80211: do not support direct add of station to AP_VLAN interfaces

[PATCH 1/3] wifi: cfg80211: do not support direct add of station to AP_VLAN interfaces

[Slawomir Stepien]

Prevent userspace from adding stations directly to AP_VLAN type
interfaces. Userspace should first add the station to the base interface
(AP type) and then can use CMD_SET_STATION to move it to AP_VLAN.

Without this path, we cannot check if the AP has been started before
adding the station - wdev for AP_VLAN does not store information about
the base AP interface.

Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 net/wireless/nl80211.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5adcb6bd0fc5..0c4e6bd6a44c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -9402,7 +9402,6 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 
 	switch (wdev->iftype) {
 	case NL80211_IFTYPE_AP:
-	case NL80211_IFTYPE_AP_VLAN:
 	case NL80211_IFTYPE_P2P_GO:
 		/* ignore WME attributes if iface/sta is not capable */
 		if (!(rdev->wiphy.flags & WIPHY_FLAG_AP_UAPSD) ||
-- 
2.55.0

[PATCH 2/3] wifi: cfg80211: move link_id validation earlier in nl80211_new_station()

[Slawomir Stepien]

I do not see a reason why this check is so low in the function. Move it
up right next to param fetch.

This new position is more beneficial for AP/Link state check that will
be added in upcoming commit.

Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 net/wireless/nl80211.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 0c4e6bd6a44c..ebde52655904 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -9222,6 +9222,16 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	params.link_sta_params.link_id =
 		nl80211_link_id_or_invalid(info->attrs);
 
+	if (wdev->valid_links) {
+		if (params.link_sta_params.link_id < 0)
+			return -EINVAL;
+		if (!(wdev->valid_links & BIT(params.link_sta_params.link_id)))
+			return -ENOLINK;
+	} else {
+		if (params.link_sta_params.link_id >= 0)
+			return -EINVAL;
+	}
+
 	if (info->attrs[NL80211_ATTR_MLD_ADDR]) {
 		mac_addr = nla_data(info->attrs[NL80211_ATTR_MLD_ADDR]);
 		params.link_sta_params.mld_mac = mac_addr;
@@ -9494,27 +9504,10 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 
 	/* be aware of params.vlan when changing code here */
 
-	if (wdev->valid_links) {
-		if (params.link_sta_params.link_id < 0) {
-			err = -EINVAL;
-			goto out;
-		}
-		if (!(wdev->valid_links & BIT(params.link_sta_params.link_id))) {
-			err = -ENOLINK;
-			goto out;
-		}
-	} else {
-		if (params.link_sta_params.link_id >= 0) {
-			err = -EINVAL;
-			goto out;
-		}
-	}
-
 	params.epp_peer =
 		nla_get_flag(info->attrs[NL80211_ATTR_EPP_PEER]);
 
 	err = rdev_add_station(rdev, wdev, mac_addr, &params);
-out:
 	dev_put(params.vlan);
 	return err;
 }
-- 
2.55.0

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

[Slawomir Stepien]

Adding a new station to AP makes only sense when the AP has been started
beforehand (nl80211_start_ap()). Check if AP is up and beaconing on the
link when adding new station. Return error if this isn't the case.

Reported-by: syzbot+9bdc0c5998ab45b05030@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9bdc0c5998ab45b05030
Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 net/wireless/nl80211.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ebde52655904..75bbd78f91ba 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -9174,7 +9174,7 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
 static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
-	int err;
+	int err, link_id;
 	struct wireless_dev *wdev = info->user_ptr[1];
 	struct net_device *dev = wdev->netdev;
 	struct station_parameters params;
@@ -9413,6 +9413,11 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
 	switch (wdev->iftype) {
 	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_P2P_GO:
+		/* Add a new station only after the AP and link has been started */
+		link_id = wdev->valid_links ? params.link_sta_params.link_id : 0;
+		if (!wdev->links[link_id].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)))
-- 
2.55.0

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

[Johannes Berg]

On Thu, 2026-08-13 at 11:04 +0200, Slawomir Stepien wrote:
> 
>  	case NL80211_IFTYPE_P2P_GO:
> +		/* Add a new station only after the AP and link has been started */
> +		link_id = wdev->valid_links ? params.link_sta_params.link_id : 0;

max() maybe?

> +		if (!wdev->links[link_id].ap.beacon_interval)
> +			return -ENETDOWN;

I think the same issue might exist also for mesh, maybe address that as
well?

And I think you could have a fourth patch to clean up the mac80211 code
to no longer handle the "STA creation on AP_VLAN" case?

johannes

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

[Slawomir Stepien]

On wrz 04, 2026 11:25, Johannes Berg wrote:
> On Thu, 2026-08-13 at 11:04 +0200, Slawomir Stepien wrote:
> > 
> >  	case NL80211_IFTYPE_P2P_GO:
> > +		/* Add a new station only after the AP and link has been started */
> > +		link_id = wdev->valid_links ? params.link_sta_params.link_id : 0;
> 
> max() maybe?

But wouldn't that blur the intent here? It's not about a bigger value. It's about MLO or not MLO
device handling.

> > +		if (!wdev->links[link_id].ap.beacon_interval)
> > +			return -ENETDOWN;
> 
> I think the same issue might exist also for mesh, maybe address that as
> well?

Sure, I will take a look at this.

> And I think you could have a fourth patch to clean up the mac80211 code
> to no longer handle the "STA creation on AP_VLAN" case?

Sure, will take a look on that too.

Thanks for review!

-- 
Slawomir Stepien

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

[Johannes Berg]

On Mon, 2026-09-07 at 11:46 +0200, Slawomir Stepien wrote:
> On wrz 04, 2026 11:25, Johannes Berg wrote:
> > On Thu, 2026-08-13 at 11:04 +0200, Slawomir Stepien wrote:
> > > 
> > >  	case NL80211_IFTYPE_P2P_GO:
> > > +		/* Add a new station only after the AP and link has been started */
> > > +		link_id = wdev->valid_links ? params.link_sta_params.link_id : 0;
> > 
> > max() maybe?
> 
> But wouldn't that blur the intent here? It's not about a bigger value. It's about MLO or not MLO
> device handling.

Yeah, true, was thinking more of the pattern than the semantics.

johannes

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

[Slawomir Stepien]

On wrz 04, 2026 11:25, Johannes Berg wrote:
> On Thu, 2026-08-13 at 11:04 +0200, Slawomir Stepien wrote:
> > 
> >  	case NL80211_IFTYPE_P2P_GO:
> > +		/* Add a new station only after the AP and link has been started */
> > +		link_id = wdev->valid_links ? params.link_sta_params.link_id : 0;
> 
> max() maybe?
> 
> > +		if (!wdev->links[link_id].ap.beacon_interval)
> > +			return -ENETDOWN;
> 
> I think the same issue might exist also for mesh, maybe address that as
> well?
> 
> And I think you could have a fourth patch to clean up the mac80211 code
> to no longer handle the "STA creation on AP_VLAN" case?

But is there such case?
The only part I think is related is the handling code when params.vlan is set. However, this code is
still OK. I do not see any other code that would be a good candidate for removal/change.

Am I missing something here?

Also, during this check, I've came to a conclusion that I will reword a bit the commit message for
PATCH 1/3, since userspace can still add STA to AP_VLAN in one go, using
attrs[NL80211_ATTR_STA_VLAN] and that's fine, since the wdev would be the base AP interface, so all
check will happen.

-- 
Slawomir Stepien

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

[Johannes Berg]

On Tue, 2026-09-08 at 11:44 +0200, Slawomir Stepien wrote:
> On wrz 04, 2026 11:25, Johannes Berg wrote:
> > On Thu, 2026-08-13 at 11:04 +0200, Slawomir Stepien wrote:
> > > 
> > >  	case NL80211_IFTYPE_P2P_GO:
> > > +		/* Add a new station only after the AP and link has been started */
> > > +		link_id = wdev->valid_links ? params.link_sta_params.link_id : 0;
> > 
> > max() maybe?
> > 
> > > +		if (!wdev->links[link_id].ap.beacon_interval)
> > > +			return -ENETDOWN;
> > 
> > I think the same issue might exist also for mesh, maybe address that as
> > well?
> > 
> > And I think you could have a fourth patch to clean up the mac80211 code
> > to no longer handle the "STA creation on AP_VLAN" case?
> 
> But is there such case?
> The only part I think is related is the handling code when params.vlan is set. However, this code is
> still OK. I do not see any other code that would be a good candidate for removal/change.
> 
> Am I missing something here?

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?

> Also, during this check, I've came to a conclusion that I will reword a bit the commit message for
> PATCH 1/3, since userspace can still add STA to AP_VLAN in one go, using
> attrs[NL80211_ATTR_STA_VLAN] and that's fine, since the wdev would be the base AP interface, so all
> check will happen.

Sure :)

johannes

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

[Slawomir Stepien]

On wrz 08, 2026 11:46, Johannes Berg wrote:
> On Tue, 2026-09-08 at 11:44 +0200, Slawomir Stepien wrote:
> > On wrz 04, 2026 11:25, Johannes Berg wrote:
> > > On Thu, 2026-08-13 at 11:04 +0200, Slawomir Stepien wrote:
> > > > 
> > > >  	case NL80211_IFTYPE_P2P_GO:
> > > > +		/* Add a new station only after the AP and link has been started */
> > > > +		link_id = wdev->valid_links ? params.link_sta_params.link_id : 0;
> > > 
> > > max() maybe?
> > > 
> > > > +		if (!wdev->links[link_id].ap.beacon_interval)
> > > > +			return -ENETDOWN;
> > > 
> > > I think the same issue might exist also for mesh, maybe address that as
> > > well?
> > > 
> > > And I think you could have a fourth patch to clean up the mac80211 code
> > > to no longer handle the "STA creation on AP_VLAN" case?
> > 
> > But is there such case?
> > The only part I think is related is the handling code when params.vlan is set. However, this code is
> > still OK. I do not see any other code that would be a good candidate for removal/change.
> > 
> > Am I missing something here?
> 
> 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?

> > Also, during this check, I've came to a conclusion that I will reword a bit the commit message for
> > PATCH 1/3, since userspace can still add STA to AP_VLAN in one go, using
> > attrs[NL80211_ATTR_STA_VLAN] and that's fine, since the wdev would be the base AP interface, so all
> > check will happen.
> 
> Sure :)

-- 
Slawomir Stepien

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

[Johannes Berg]

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.

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.)

johannes

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

[Slawomir Stepien]

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

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

[Johannes Berg]

On Tue, 2026-09-08 at 15:35 +0200, Slawomir Stepien wrote:
> > 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) {

Sure, but that just means that A and B have to be on the same wiphy.

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

Don't think so? You can have A, B, A' and B' all on the same wiphy.

johannes