diff mbox series

[wireless] wifi: mac80211: use monitor sdata with driver only if desired

Message ID 20240725184836.25d334157a8e.I02574086da2c5cf0e18264ce5807db6f14ffd9c0@changeid
State New
Headers show
Series [wireless] wifi: mac80211: use monitor sdata with driver only if desired | expand

Commit Message

Johannes Berg July 25, 2024, 4:48 p.m. UTC
From: Johannes Berg <johannes.berg@intel.com>

In commit 0d9c2beed116 ("wifi: mac80211: fix monitor channel
with chanctx emulation") I changed mac80211 to always have an
internal monitor_sdata to have something to have the chanctx
bound to.

However, if the driver didn't also have the WANT_MONITOR flag
this would cause mac80211 to allocate it without telling the
driver (which was intentional) but also use it for later APIs
to the driver without it ever having known about it which was
_not_ intentional.

Check through the code and only use the monitor_sdata in the
relevant places (TX, MU-MIMO follow settings, TX power, and
interface iteration) when the WANT_MONITOR flag is set.

Cc: stable@vger.kernel.org
Fixes: 0d9c2beed116 ("wifi: mac80211: fix monitor channel with chanctx emulation")
Reported-by: ZeroBeat <ZeroBeat@gmx.de>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219086
Tested-by: Lorenzo Bianconi <lorenzo@kernel.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c  | 7 +++++--
 net/mac80211/tx.c   | 5 +++--
 net/mac80211/util.c | 2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

Comments

ZeroBeat July 25, 2024, 5:42 p.m. UTC | #1
Great work. Thanks.

$ uname -r
6.10.0-1-git-12246-g786c8248dbd3-dirty

$ sudo hcxdumptool -i wlp5s0f4u2 --rcascan=active

^C
24 Packet(s) captured by kernel
0 Packet(s) dropped by kernel
14 PROBERESPONSE(s) captured


Best regards
Michael



Am 25.07.24 um 18:48 schrieb Johannes Berg:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> In commit 0d9c2beed116 ("wifi: mac80211: fix monitor channel
> with chanctx emulation") I changed mac80211 to always have an
> internal monitor_sdata to have something to have the chanctx
> bound to.
> 
> However, if the driver didn't also have the WANT_MONITOR flag
> this would cause mac80211 to allocate it without telling the
> driver (which was intentional) but also use it for later APIs
> to the driver without it ever having known about it which was
> _not_ intentional.
> 
> Check through the code and only use the monitor_sdata in the
> relevant places (TX, MU-MIMO follow settings, TX power, and
> interface iteration) when the WANT_MONITOR flag is set.
> 
> Cc: stable@vger.kernel.org
> Fixes: 0d9c2beed116 ("wifi: mac80211: fix monitor channel with chanctx emulation")
> Reported-by: ZeroBeat <ZeroBeat@gmx.de>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219086
> Tested-by: Lorenzo Bianconi <lorenzo@kernel.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>   net/mac80211/cfg.c  | 7 +++++--
>   net/mac80211/tx.c   | 5 +++--
>   net/mac80211/util.c | 2 +-
>   3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 85cb71de370f..b02b84ce2130 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -114,7 +114,7 @@ static int ieee80211_set_mon_options(struct ieee80211_sub_if_data *sdata,
>   
>   	/* apply all changes now - no failures allowed */
>   
> -	if (monitor_sdata)
> +	if (monitor_sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
>   		ieee80211_set_mu_mimo_follow(monitor_sdata, params);
>   
>   	if (params->flags) {
> @@ -3053,6 +3053,9 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
>   		sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
>   
>   		if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
> +			if (!ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
> +				return -EOPNOTSUPP;
> +
>   			sdata = wiphy_dereference(local->hw.wiphy,
>   						  local->monitor_sdata);
>   			if (!sdata)
> @@ -3115,7 +3118,7 @@ static int ieee80211_set_tx_power(struct wiphy *wiphy,
>   	if (has_monitor) {
>   		sdata = wiphy_dereference(local->hw.wiphy,
>   					  local->monitor_sdata);
> -		if (sdata) {
> +		if (sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) {
>   			sdata->deflink.user_power_level = local->user_power_level;
>   			if (txp_type != sdata->vif.bss_conf.txpower_type)
>   				update_txp_type = true;
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 72a9ba8bc5fd..edba4a31844f 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -1768,7 +1768,7 @@ static bool __ieee80211_tx(struct ieee80211_local *local,
>   			break;
>   		}
>   		sdata = rcu_dereference(local->monitor_sdata);
> -		if (sdata) {
> +		if (sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) {
>   			vif = &sdata->vif;
>   			info->hw_queue =
>   				vif->hw_queue[skb_get_queue_mapping(skb)];
> @@ -3957,7 +3957,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
>   			break;
>   		}
>   		tx.sdata = rcu_dereference(local->monitor_sdata);
> -		if (tx.sdata) {
> +		if (tx.sdata &&
> +		    ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) {
>   			vif = &tx.sdata->vif;
>   			info->hw_queue =
>   				vif->hw_queue[skb_get_queue_mapping(skb)];
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index ced19ce7c51a..c7ad9bc5973a 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -776,7 +776,7 @@ static void __iterate_interfaces(struct ieee80211_local *local,
>   	sdata = rcu_dereference_check(local->monitor_sdata,
>   				      lockdep_is_held(&local->iflist_mtx) ||
>   				      lockdep_is_held(&local->hw.wiphy->mtx));
> -	if (sdata &&
> +	if (sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF) &&
>   	    (iter_flags & IEEE80211_IFACE_ITER_RESUME_ALL || !active_only ||
>   	     sdata->flags & IEEE80211_SDATA_IN_DRIVER))
>   		iterator(data, sdata->vif.addr, &sdata->vif);
diff mbox series

Patch

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 85cb71de370f..b02b84ce2130 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -114,7 +114,7 @@  static int ieee80211_set_mon_options(struct ieee80211_sub_if_data *sdata,
 
 	/* apply all changes now - no failures allowed */
 
-	if (monitor_sdata)
+	if (monitor_sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
 		ieee80211_set_mu_mimo_follow(monitor_sdata, params);
 
 	if (params->flags) {
@@ -3053,6 +3053,9 @@  static int ieee80211_set_tx_power(struct wiphy *wiphy,
 		sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
 
 		if (sdata->vif.type == NL80211_IFTYPE_MONITOR) {
+			if (!ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF))
+				return -EOPNOTSUPP;
+
 			sdata = wiphy_dereference(local->hw.wiphy,
 						  local->monitor_sdata);
 			if (!sdata)
@@ -3115,7 +3118,7 @@  static int ieee80211_set_tx_power(struct wiphy *wiphy,
 	if (has_monitor) {
 		sdata = wiphy_dereference(local->hw.wiphy,
 					  local->monitor_sdata);
-		if (sdata) {
+		if (sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) {
 			sdata->deflink.user_power_level = local->user_power_level;
 			if (txp_type != sdata->vif.bss_conf.txpower_type)
 				update_txp_type = true;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 72a9ba8bc5fd..edba4a31844f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1768,7 +1768,7 @@  static bool __ieee80211_tx(struct ieee80211_local *local,
 			break;
 		}
 		sdata = rcu_dereference(local->monitor_sdata);
-		if (sdata) {
+		if (sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) {
 			vif = &sdata->vif;
 			info->hw_queue =
 				vif->hw_queue[skb_get_queue_mapping(skb)];
@@ -3957,7 +3957,8 @@  struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 			break;
 		}
 		tx.sdata = rcu_dereference(local->monitor_sdata);
-		if (tx.sdata) {
+		if (tx.sdata &&
+		    ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF)) {
 			vif = &tx.sdata->vif;
 			info->hw_queue =
 				vif->hw_queue[skb_get_queue_mapping(skb)];
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index ced19ce7c51a..c7ad9bc5973a 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -776,7 +776,7 @@  static void __iterate_interfaces(struct ieee80211_local *local,
 	sdata = rcu_dereference_check(local->monitor_sdata,
 				      lockdep_is_held(&local->iflist_mtx) ||
 				      lockdep_is_held(&local->hw.wiphy->mtx));
-	if (sdata &&
+	if (sdata && ieee80211_hw_check(&local->hw, WANT_MONITOR_VIF) &&
 	    (iter_flags & IEEE80211_IFACE_ITER_RESUME_ALL || !active_only ||
 	     sdata->flags & IEEE80211_SDATA_IN_DRIVER))
 		iterator(data, sdata->vif.addr, &sdata->vif);