[PATCH] wifi: p54: unwind failed interface additions

[PATCH] wifi: p54: unwind failed interface additions

[Jiale Yao]

p54 stores the vif and changes its operating mode before validating the
interface type and before p54_setup_mac() can fail. Since mac80211 does
not call remove_interface after a failed add, either path can leave the
driver believing that an interface remains active.

Publish the vif only after type validation and restore the monitor state
when MAC setup fails.

Fixes: 1ed32e4fc8cf ("mac80211: remove struct ieee80211_if_init_conf")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/net/wireless/intersil/p54/main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
index 57a62108cbc3..69d85efb9260 100644
--- a/drivers/net/wireless/intersil/p54/main.c
+++ b/drivers/net/wireless/intersil/p54/main.c
@@ -236,8 +236,6 @@ static int p54_add_interface(struct ieee80211_hw *dev,
 		return -EOPNOTSUPP;
 	}
 
-	priv->vif = vif;
-
 	switch (vif->type) {
 	case NL80211_IFTYPE_STATION:
 	case NL80211_IFTYPE_ADHOC:
@@ -250,8 +248,14 @@ static int p54_add_interface(struct ieee80211_hw *dev,
 		return -EOPNOTSUPP;
 	}
 
+	priv->vif = vif;
 	memcpy(priv->mac_addr, vif->addr, ETH_ALEN);
 	err = p54_setup_mac(priv);
+	if (err) {
+		priv->vif = NULL;
+		priv->mode = NL80211_IFTYPE_MONITOR;
+		eth_zero_addr(priv->mac_addr);
+	}
 	mutex_unlock(&priv->conf_mutex);
 	return err;
 }
-- 
2.34.1

Re: [PATCH] wifi: p54: unwind failed interface additions

[Christian Lamparter]

On 10/19/26 3:31 AM, Jiale Yao wrote:
> p54 stores the vif and changes its operating mode before validating the
> interface type and before p54_setup_mac() can fail. Since mac80211 does
> not call remove_interface after a failed add, either path can leave the
> driver believing that an interface remains active.

hmm?

<https://docs.kernel.org/driver-api/80211/mac80211.html>

|stop
|	Called after last netdevice attached to the hardware is disabled.
|	This should turn off the hardware (at least it must turn off frame reception.)
|	>>> May be called right after add_interface if that rejects an interface <<<

so mac80211 should call drv_stop (aka. p54_stop) in ieee80211_do_open() if
p54_add_interface returns with an error code...

and p54_stop does:

priv->mode = NL80211_IFTYPE_UNSPECIFIED;

> Publish the vif only after type validation and restore the monitor state
> when MAC setup fails.
> 
> Fixes: 1ed32e4fc8cf ("mac80211: remove struct ieee80211_if_init_conf")
> Signed-off-by: Jiale Yao <yaojiale02@163.com>

priv->vif and priv->mac_addr remains set. I don't think that having priv->mac_addr
set matters. About priv->vif... Hmm, mac80211 passes it as &sdata->vif. So for the
lifetime of the associated mac80211 device this will always be the same?
(... as p54 only ever has one?)

So, even if beacon_loss traps are injected after a p54_add_interface fails, priv->vif
will be a valid pointer... didn't remember or see any other users...
hmm, it should be the same, but I don't know the mac80211 code well enough.

That said. I don't have a problem with the code. It reduces the "need to know
about mac80211 internals too" load... with that I'm struggeling too.

If you want to respin a v2 with the commit changes, you can have a:

Acked-by: Christian Lamparter <chunkeey@gmail.com>

And if not, I guess it's still fine (but not necessarily 100% correct maybe) as
is either way.

> ---
>   drivers/net/wireless/intersil/p54/main.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/intersil/p54/main.c b/drivers/net/wireless/intersil/p54/main.c
> index 57a62108cbc3..69d85efb9260 100644
> --- a/drivers/net/wireless/intersil/p54/main.c
> +++ b/drivers/net/wireless/intersil/p54/main.c
> @@ -236,8 +236,6 @@ static int p54_add_interface(struct ieee80211_hw *dev,
>   		return -EOPNOTSUPP;
>   	}
>   
> -	priv->vif = vif;
> -
>   	switch (vif->type) {
>   	case NL80211_IFTYPE_STATION:
>   	case NL80211_IFTYPE_ADHOC:
> @@ -250,8 +248,14 @@ static int p54_add_interface(struct ieee80211_hw *dev,
>   		return -EOPNOTSUPP;
>   	}
>   
> +	priv->vif = vif;
>   	memcpy(priv->mac_addr, vif->addr, ETH_ALEN);
>   	err = p54_setup_mac(priv);
> +	if (err) {
> +		priv->vif = NULL;
> +		priv->mode = NL80211_IFTYPE_MONITOR;
> +		eth_zero_addr(priv->mac_addr);
> +	}
>   	mutex_unlock(&priv->conf_mutex);
>   	return err;
>   }