All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2
@ 2020-03-26 13:09 Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 01/12] mac80211: implement Operating Mode Notification extended NSS support Luca Coelho
                   ` (11 more replies)
  0 siblings, 12 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Luca Coelho <luciano.coelho@intel.com>

Hi,

A bunch of patches with mac80211 and cfg80211 changes from our
internal tree.

Please review, though you have already reviewed most if not all of
them ;)

In v2 I replaced the Change-Ids with Message-Ids as it should be.

Cheers,
Luca.


Andrei Otcheretianski (1):
  mac80211: Don't destroy auth data in case of anti-clogging

Ilan Peer (4):
  cfg80211: Parse HE membership selector
  mac80211: Skip entries with HE membership selector
  mac80211: Fail association when AP has no legacy rates
  cfg80211: Do not warn on same channel at the end of CSA

Johannes Berg (4):
  mac80211: implement Operating Mode Notification extended NSS support
  mac80211: minstrel_ht_assign_best_tp_rates: remove redundant test
  mac80211_hwsim: indicate in IBSS that we have transmitted beacons
  mac80211: drop data frames without key on encrypted links

Mordechay Goodstein (2):
  mac80211: agg-tx: refactor sending addba
  mac80211: agg-tx: add an option to defer ADDBA transmit

Shaul Triebitz (1):
  mac80211: add twt_protected flag to the bss_conf structure

 drivers/net/wireless/intel/iwlwifi/mvm/rs.c |  6 +-
 drivers/net/wireless/mac80211_hwsim.c       |  6 ++
 include/linux/ieee80211.h                   | 13 +++-
 include/net/cfg80211.h                      |  3 +-
 include/net/mac80211.h                      |  8 ++-
 net/mac80211/agg-tx.c                       | 79 +++++++++++++--------
 net/mac80211/debugfs_sta.c                  |  3 +-
 net/mac80211/key.c                          | 20 +++---
 net/mac80211/mlme.c                         | 33 +++++++--
 net/mac80211/rc80211_minstrel_ht.c          |  3 +-
 net/mac80211/sta_info.h                     |  3 +
 net/mac80211/tx.c                           | 12 +++-
 net/mac80211/vht.c                          | 10 ++-
 net/wireless/nl80211.c                      |  2 +
 net/wireless/scan.c                         |  6 +-
 net/wireless/util.c                         | 26 +++----
 16 files changed, 163 insertions(+), 70 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 01/12] mac80211: implement Operating Mode Notification extended NSS support
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 02/12] mac80211: add twt_protected flag to the bss_conf structure Luca Coelho
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

Somehow we missed this for a long time, but similar to the extended
NSS support in VHT capabilities, we need to have this in Operating
Mode notification.

Implement it by
 * parsing the 160/80+80 bit there and setting the bandwidth
   appropriately
 * having callers of ieee80211_get_vht_max_nss() pass in the current
   max NSS value as received in the operating mode notification in
   order to modify it appropriately depending on the extended NSS
   bits.

This updates all drivers that use it, i.e. only iwlwifi/mvm.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rs.c |  6 ++---
 include/linux/ieee80211.h                   | 12 +++++++---
 net/mac80211/vht.c                          | 10 ++++++--
 net/wireless/util.c                         | 26 +++++++++++----------
 4 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index 1a990ed9c3ca..af1baebe11a7 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -1,10 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /******************************************************************************
  *
- * Copyright(c) 2005 - 2014 Intel Corporation. All rights reserved.
+ * Copyright(c) 2005 - 2014, 2018 - 2020 Intel Corporation. All rights reserved.
  * Copyright(c) 2013 - 2015 Intel Mobile Communications GmbH
  * Copyright(c) 2016 - 2017 Intel Deutschland GmbH
- * Copyright(c) 2018 - 2019 Intel Corporation
  *
  * Contact Information:
  *  Intel Linux Wireless <linuxwifi@intel.com>
@@ -1430,7 +1429,8 @@ static u32 rs_bw_from_sta_bw(struct ieee80211_sta *sta)
 		 */
 		if (ieee80211_get_vht_max_nss(&vht_cap,
 					      IEEE80211_VHT_CHANWIDTH_160MHZ,
-					      0, true) < sta->rx_nss)
+					      0, true,
+					      sta->rx_nss) < sta->rx_nss)
 			return RATE_MCS_CHAN_WIDTH_80;
 		return RATE_MCS_CHAN_WIDTH_160;
 	case IEEE80211_STA_RX_BW_80:
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 33d907eec0b6..bdcc3165c440 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -9,7 +9,7 @@
  * Copyright (c) 2006, Michael Wu <flamingice@sourmilk.net>
  * Copyright (c) 2013 - 2014 Intel Mobile Communications GmbH
  * Copyright (c) 2016 - 2017 Intel Deutschland GmbH
- * Copyright (c) 2018 - 2019 Intel Corporation
+ * Copyright (c) 2018 - 2020 Intel Corporation
  */
 
 #ifndef LINUX_IEEE80211_H
@@ -859,6 +859,7 @@ enum ieee80211_ht_chanwidth_values {
  * @IEEE80211_OPMODE_NOTIF_CHANWIDTH_40MHZ: 40 MHz channel width
  * @IEEE80211_OPMODE_NOTIF_CHANWIDTH_80MHZ: 80 MHz channel width
  * @IEEE80211_OPMODE_NOTIF_CHANWIDTH_160MHZ: 160 MHz or 80+80 MHz channel width
+ * @IEEE80211_OPMODE_NOTIF_BW_160_80P80: 160 / 80+80 MHz indicator flag
  * @IEEE80211_OPMODE_NOTIF_RX_NSS_MASK: number of spatial streams mask
  *	(the NSS value is the value of this field + 1)
  * @IEEE80211_OPMODE_NOTIF_RX_NSS_SHIFT: number of spatial streams shift
@@ -866,11 +867,12 @@ enum ieee80211_ht_chanwidth_values {
  *	using a beamforming steering matrix
  */
 enum ieee80211_vht_opmode_bits {
-	IEEE80211_OPMODE_NOTIF_CHANWIDTH_MASK	= 3,
+	IEEE80211_OPMODE_NOTIF_CHANWIDTH_MASK	= 0x03,
 	IEEE80211_OPMODE_NOTIF_CHANWIDTH_20MHZ	= 0,
 	IEEE80211_OPMODE_NOTIF_CHANWIDTH_40MHZ	= 1,
 	IEEE80211_OPMODE_NOTIF_CHANWIDTH_80MHZ	= 2,
 	IEEE80211_OPMODE_NOTIF_CHANWIDTH_160MHZ	= 3,
+	IEEE80211_OPMODE_NOTIF_BW_160_80P80	= 0x04,
 	IEEE80211_OPMODE_NOTIF_RX_NSS_MASK	= 0x70,
 	IEEE80211_OPMODE_NOTIF_RX_NSS_SHIFT	= 4,
 	IEEE80211_OPMODE_NOTIF_RX_NSS_TYPE_BF	= 0x80,
@@ -1731,6 +1733,9 @@ struct ieee80211_mu_edca_param_set {
  * @ext_nss_bw_capable: indicates whether or not the local transmitter
  *	(rate scaling algorithm) can deal with the new logic
  *	(dot11VHTExtendedNSSBWCapable)
+ * @max_vht_nss: current maximum NSS as advertised by the STA in
+ *	operating mode notification, can be 0 in which case the
+ *	capability data will be used to derive this (from MCS support)
  *
  * Due to the VHT Extended NSS Bandwidth Support, the maximum NSS can
  * vary for a given BW/MCS. This function parses the data.
@@ -1739,7 +1744,8 @@ struct ieee80211_mu_edca_param_set {
  */
 int ieee80211_get_vht_max_nss(struct ieee80211_vht_cap *cap,
 			      enum ieee80211_vht_chanwidth bw,
-			      int mcs, bool ext_nss_bw_capable);
+			      int mcs, bool ext_nss_bw_capable,
+			      unsigned int max_vht_nss);
 
 /* 802.11ax HE MAC capabilities */
 #define IEEE80211_HE_MAC_CAP0_HTC_HE				0x01
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index 632f07401850..9c6045f9c24d 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -4,7 +4,7 @@
  *
  * Portions of this file
  * Copyright(c) 2015 - 2016 Intel Deutschland GmbH
- * Copyright (C) 2018 - 2019 Intel Corporation
+ * Copyright (C) 2018 - 2020 Intel Corporation
  */
 
 #include <linux/ieee80211.h>
@@ -575,15 +575,21 @@ u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
 
 	switch (opmode & IEEE80211_OPMODE_NOTIF_CHANWIDTH_MASK) {
 	case IEEE80211_OPMODE_NOTIF_CHANWIDTH_20MHZ:
+		/* ignore IEEE80211_OPMODE_NOTIF_BW_160_80P80 must not be set */
 		sta->cur_max_bandwidth = IEEE80211_STA_RX_BW_20;
 		break;
 	case IEEE80211_OPMODE_NOTIF_CHANWIDTH_40MHZ:
+		/* ignore IEEE80211_OPMODE_NOTIF_BW_160_80P80 must not be set */
 		sta->cur_max_bandwidth = IEEE80211_STA_RX_BW_40;
 		break;
 	case IEEE80211_OPMODE_NOTIF_CHANWIDTH_80MHZ:
-		sta->cur_max_bandwidth = IEEE80211_STA_RX_BW_80;
+		if (opmode & IEEE80211_OPMODE_NOTIF_BW_160_80P80)
+			sta->cur_max_bandwidth = IEEE80211_STA_RX_BW_160;
+		else
+			sta->cur_max_bandwidth = IEEE80211_STA_RX_BW_80;
 		break;
 	case IEEE80211_OPMODE_NOTIF_CHANWIDTH_160MHZ:
+		/* legacy only, no longer used by newer spec */
 		sta->cur_max_bandwidth = IEEE80211_STA_RX_BW_160;
 		break;
 	}
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 6590efbbcbb9..123d6ce79b8e 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -5,7 +5,7 @@
  * Copyright 2007-2009	Johannes Berg <johannes@sipsolutions.net>
  * Copyright 2013-2014  Intel Mobile Communications GmbH
  * Copyright 2017	Intel Deutschland GmbH
- * Copyright (C) 2018-2019 Intel Corporation
+ * Copyright (C) 2018-2020 Intel Corporation
  */
 #include <linux/export.h>
 #include <linux/bitops.h>
@@ -2030,10 +2030,10 @@ EXPORT_SYMBOL(cfg80211_send_layer2_update);
 
 int ieee80211_get_vht_max_nss(struct ieee80211_vht_cap *cap,
 			      enum ieee80211_vht_chanwidth bw,
-			      int mcs, bool ext_nss_bw_capable)
+			      int mcs, bool ext_nss_bw_capable,
+			      unsigned int max_vht_nss)
 {
 	u16 map = le16_to_cpu(cap->supp_mcs.rx_mcs_map);
-	int max_vht_nss = 0;
 	int ext_nss_bw;
 	int supp_width;
 	int i, mcs_encoding;
@@ -2041,7 +2041,7 @@ int ieee80211_get_vht_max_nss(struct ieee80211_vht_cap *cap,
 	if (map == 0xffff)
 		return 0;
 
-	if (WARN_ON(mcs > 9))
+	if (WARN_ON(mcs > 9 || max_vht_nss > 8))
 		return 0;
 	if (mcs <= 7)
 		mcs_encoding = 0;
@@ -2050,16 +2050,18 @@ int ieee80211_get_vht_max_nss(struct ieee80211_vht_cap *cap,
 	else
 		mcs_encoding = 2;
 
-	/* find max_vht_nss for the given MCS */
-	for (i = 7; i >= 0; i--) {
-		int supp = (map >> (2 * i)) & 3;
+	if (!max_vht_nss) {
+		/* find max_vht_nss for the given MCS */
+		for (i = 7; i >= 0; i--) {
+			int supp = (map >> (2 * i)) & 3;
 
-		if (supp == 3)
-			continue;
+			if (supp == 3)
+				continue;
 
-		if (supp >= mcs_encoding) {
-			max_vht_nss = i + 1;
-			break;
+			if (supp >= mcs_encoding) {
+				max_vht_nss = i + 1;
+				break;
+			}
 		}
 	}
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 02/12] mac80211: add twt_protected flag to the bss_conf structure
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 01/12] mac80211: implement Operating Mode Notification extended NSS support Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 03/12] mac80211: Don't destroy auth data in case of anti-clogging Luca Coelho
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Shaul Triebitz <shaul.triebitz@intel.com>

Add a flag to the BSS conf whether the BSS and STA support protected TWT.

Signed-off-by: Shaul Triebitz <shaul.triebitz@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/mac80211.h | 2 ++
 net/mac80211/mlme.c    | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index b6b4de0e4b5e..f2b0a7795d0a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -508,6 +508,7 @@ struct ieee80211_ftm_responder_params {
  *	mode only, set if the AP advertises TWT responder role)
  * @twt_responder: does this BSS support TWT requester (relevant for managed
  *	mode only, set if the AP advertises TWT responder role)
+ * @twt_protected: does this BSS support protected TWT frames
  * @assoc: association status
  * @ibss_joined: indicates whether this station is part of an IBSS
  *	or not
@@ -618,6 +619,7 @@ struct ieee80211_bss_conf {
 	bool he_support;
 	bool twt_requester;
 	bool twt_responder;
+	bool twt_protected;
 	/* association related data */
 	bool assoc, ibss_joined;
 	bool ibss_creator;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 16d75da0996a..f1f518790c12 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3384,10 +3384,19 @@ static bool ieee80211_assoc_success(struct ieee80211_sub_if_data *sdata,
 						  sta);
 
 		bss_conf->he_support = sta->sta.he_cap.has_he;
+		if (elems->rsnx && elems->rsnx_len &&
+		    (elems->rsnx[0] & WLAN_RSNX_CAPA_PROTECTED_TWT) &&
+		    wiphy_ext_feature_isset(local->hw.wiphy,
+					    NL80211_EXT_FEATURE_PROTECTED_TWT))
+			bss_conf->twt_protected = true;
+		else
+			bss_conf->twt_protected = false;
+
 		changed |= ieee80211_recalc_twt_req(sdata, sta, elems);
 	} else {
 		bss_conf->he_support = false;
 		bss_conf->twt_requester = false;
+		bss_conf->twt_protected = false;
 	}
 
 	if (bss_conf->he_support) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 03/12] mac80211: Don't destroy auth data in case of anti-clogging
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 01/12] mac80211: implement Operating Mode Notification extended NSS support Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 02/12] mac80211: add twt_protected flag to the bss_conf structure Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 04/12] cfg80211: Parse HE membership selector Luca Coelho
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>

SAE AP may reject authentication with WLAN_STATUS_ANTI_CLOG_REQUIRED.
As the user space will immediately continue the authentication flow,
there is no need to destroy the authentication data in this case.
This saves unneeded station removal and releasing the channel.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/mlme.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index f1f518790c12..a0de21a51f65 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2948,10 +2948,15 @@ static void ieee80211_rx_mgmt_auth(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (status_code != WLAN_STATUS_SUCCESS) {
+		cfg80211_rx_mlme_mgmt(sdata->dev, (u8 *)mgmt, len);
+
+		if (auth_alg == WLAN_AUTH_SAE &&
+		    status_code == WLAN_STATUS_ANTI_CLOG_REQUIRED)
+			return;
+
 		sdata_info(sdata, "%pM denied authentication (status %d)\n",
 			   mgmt->sa, status_code);
 		ieee80211_destroy_auth_data(sdata, false);
-		cfg80211_rx_mlme_mgmt(sdata->dev, (u8 *)mgmt, len);
 		event.u.mlme.status = MLME_DENIED;
 		event.u.mlme.reason = status_code;
 		drv_event_callback(sdata->local, sdata, &event);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 04/12] cfg80211: Parse HE membership selector
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (2 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 03/12] mac80211: Don't destroy auth data in case of anti-clogging Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 05/12] mac80211: Skip entries with " Luca Coelho
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Ilan Peer <ilan.peer@intel.com>

This extends the support for drivers that rebuilds IEs in the
FW (same as with HT/VHT).

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/linux/ieee80211.h | 1 +
 include/net/cfg80211.h    | 3 ++-
 net/wireless/nl80211.c    | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index bdcc3165c440..7c7b974c41b1 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1067,6 +1067,7 @@ struct ieee80211_mgmt {
 /* Supported rates membership selectors */
 #define BSS_MEMBERSHIP_SELECTOR_HT_PHY	127
 #define BSS_MEMBERSHIP_SELECTOR_VHT_PHY	126
+#define BSS_MEMBERSHIP_SELECTOR_HE_PHY	122
 
 /* mgmt header + 1 byte category code */
 #define IEEE80211_MIN_ACTION_SIZE offsetof(struct ieee80211_mgmt, u.action.u)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index c78bd4ff9e33..18e17496a904 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1052,6 +1052,7 @@ enum cfg80211_ap_settings_flags {
  * @ht_required: stations must support HT
  * @vht_required: stations must support VHT
  * @twt_responder: Enable Target Wait Time
+ * @he_required: stations must support HE
  * @flags: flags, as defined in enum cfg80211_ap_settings_flags
  * @he_obss_pd: OBSS Packet Detection settings
  * @he_bss_color: BSS Color settings
@@ -1081,7 +1082,7 @@ struct cfg80211_ap_settings {
 	const struct ieee80211_vht_cap *vht_cap;
 	const struct ieee80211_he_cap_elem *he_cap;
 	const struct ieee80211_he_operation *he_oper;
-	bool ht_required, vht_required;
+	bool ht_required, vht_required, he_required;
 	bool twt_responder;
 	u32 flags;
 	struct ieee80211_he_obss_pd he_obss_pd;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ad87e9db9a91..0586093420e8 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4728,6 +4728,8 @@ static void nl80211_check_ap_rate_selectors(struct cfg80211_ap_settings *params,
 			params->ht_required = true;
 		if (rates[2 + i] == BSS_MEMBERSHIP_SELECTOR_VHT_PHY)
 			params->vht_required = true;
+		if (rates[2 + i] == BSS_MEMBERSHIP_SELECTOR_HE_PHY)
+			params->he_required = true;
 	}
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 05/12] mac80211: Skip entries with HE membership selector
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (3 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 04/12] cfg80211: Parse HE membership selector Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 06/12] mac80211: agg-tx: refactor sending addba Luca Coelho
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Ilan Peer <ilan.peer@intel.com>

When parsing supported rates IE.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/mlme.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index a0de21a51f65..04ed883339c7 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3154,15 +3154,16 @@ static void ieee80211_get_rates(struct ieee80211_supported_band *sband,
 			*have_higher_than_11mbit = true;
 
 		/*
-		 * Skip HT and VHT BSS membership selectors since they're not
-		 * rates.
+		 * Skip HT, VHT and HE BSS membership selectors since they're
+		 * not rates.
 		 *
 		 * Note: Even though the membership selector and the basic
 		 *	 rate flag share the same bit, they are not exactly
 		 *	 the same.
 		 */
 		if (supp_rates[i] == (0x80 | BSS_MEMBERSHIP_SELECTOR_HT_PHY) ||
-		    supp_rates[i] == (0x80 | BSS_MEMBERSHIP_SELECTOR_VHT_PHY))
+		    supp_rates[i] == (0x80 | BSS_MEMBERSHIP_SELECTOR_VHT_PHY) ||
+		    supp_rates[i] == (0x80 | BSS_MEMBERSHIP_SELECTOR_HE_PHY))
 			continue;
 
 		for (j = 0; j < sband->n_bitrates; j++) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 06/12] mac80211: agg-tx: refactor sending addba
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (4 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 05/12] mac80211: Skip entries with " Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 07/12] mac80211: agg-tx: add an option to defer ADDBA transmit Luca Coelho
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Mordechay Goodstein <mordechay.goodstein@intel.com>

We move the actual arming the timer and sending ADDBA to a function
for the use in different places calling the same logic.

Signed-off-by: Mordechay Goodstein <mordechay.goodstein@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/agg-tx.c | 67 +++++++++++++++++++++++++------------------
 1 file changed, 39 insertions(+), 28 deletions(-)

diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 33da6f738c99..32f40c4f3120 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -9,7 +9,7 @@
  * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
  * Copyright 2007-2010, Intel Corporation
  * Copyright(c) 2015-2017 Intel Deutschland GmbH
- * Copyright (C) 2018 - 2019 Intel Corporation
+ * Copyright (C) 2018 - 2020 Intel Corporation
  */
 
 #include <linux/ieee80211.h>
@@ -448,6 +448,43 @@ static void sta_addba_resp_timer_expired(struct timer_list *t)
 	ieee80211_stop_tx_ba_session(&sta->sta, tid);
 }
 
+static void ieee80211_send_addba_with_timeout(struct sta_info *sta,
+					      struct tid_ampdu_tx *tid_tx)
+{
+	struct ieee80211_sub_if_data *sdata = sta->sdata;
+	struct ieee80211_local *local = sta->local;
+	u8 tid = tid_tx->tid;
+	u16 buf_size;
+
+	/* activate the timer for the recipient's addBA response */
+	mod_timer(&tid_tx->addba_resp_timer, jiffies + ADDBA_RESP_INTERVAL);
+	ht_dbg(sdata, "activated addBA response timer on %pM tid %d\n",
+	       sta->sta.addr, tid);
+
+	spin_lock_bh(&sta->lock);
+	sta->ampdu_mlme.last_addba_req_time[tid] = jiffies;
+	sta->ampdu_mlme.addba_req_num[tid]++;
+	spin_unlock_bh(&sta->lock);
+
+	if (sta->sta.he_cap.has_he) {
+		buf_size = local->hw.max_tx_aggregation_subframes;
+	} else {
+		/*
+		 * We really should use what the driver told us it will
+		 * transmit as the maximum, but certain APs (e.g. the
+		 * LinkSys WRT120N with FW v1.0.07 build 002 Jun 18 2012)
+		 * will crash when we use a lower number.
+		 */
+		buf_size = IEEE80211_MAX_AMPDU_BUF_HT;
+	}
+
+	/* send AddBA request */
+	ieee80211_send_addba_request(sdata, sta->sta.addr, tid,
+				     tid_tx->dialog_token,
+				     sta->tid_seq[tid] >> 4,
+				     buf_size, tid_tx->timeout);
+}
+
 void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
 {
 	struct tid_ampdu_tx *tid_tx;
@@ -462,7 +499,6 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
 		.timeout = 0,
 	};
 	int ret;
-	u16 buf_size;
 
 	tid_tx = rcu_dereference_protected_tid_tx(sta, tid);
 
@@ -508,32 +544,7 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
 		return;
 	}
 
-	/* activate the timer for the recipient's addBA response */
-	mod_timer(&tid_tx->addba_resp_timer, jiffies + ADDBA_RESP_INTERVAL);
-	ht_dbg(sdata, "activated addBA response timer on %pM tid %d\n",
-	       sta->sta.addr, tid);
-
-	spin_lock_bh(&sta->lock);
-	sta->ampdu_mlme.last_addba_req_time[tid] = jiffies;
-	sta->ampdu_mlme.addba_req_num[tid]++;
-	spin_unlock_bh(&sta->lock);
-
-	if (sta->sta.he_cap.has_he) {
-		buf_size = local->hw.max_tx_aggregation_subframes;
-	} else {
-		/*
-		 * We really should use what the driver told us it will
-		 * transmit as the maximum, but certain APs (e.g. the
-		 * LinkSys WRT120N with FW v1.0.07 build 002 Jun 18 2012)
-		 * will crash when we use a lower number.
-		 */
-		buf_size = IEEE80211_MAX_AMPDU_BUF_HT;
-	}
-
-	/* send AddBA request */
-	ieee80211_send_addba_request(sdata, sta->sta.addr, tid,
-				     tid_tx->dialog_token, params.ssn,
-				     buf_size, tid_tx->timeout);
+	ieee80211_send_addba_with_timeout(sta, tid_tx);
 }
 
 /*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 07/12] mac80211: agg-tx: add an option to defer ADDBA transmit
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (5 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 06/12] mac80211: agg-tx: refactor sending addba Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 08/12] mac80211: Fail association when AP has no legacy rates Luca Coelho
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Mordechay Goodstein <mordechay.goodstein@intel.com>

Driver tells mac80211 to sends ADDBA with SSN (starting sequence number)
from the head of the queue, while the transmission of all the frames in the
queue may take a while, which causes the peer to time out. In order to
fix this scenario, add an option to defer ADDBA transmit until queue
is drained.

Signed-off-by: Mordechay Goodstein <mordechay.goodstein@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/mac80211.h  |  6 +++++-
 net/mac80211/agg-tx.c   | 12 +++++++++++-
 net/mac80211/sta_info.h |  2 ++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f2b0a7795d0a..bcf706798e52 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -3119,7 +3119,10 @@ enum ieee80211_filter_flags {
  * @IEEE80211_AMPDU_RX_START: start RX aggregation
  * @IEEE80211_AMPDU_RX_STOP: stop RX aggregation
  * @IEEE80211_AMPDU_TX_START: start TX aggregation, the driver must either
- *	call ieee80211_start_tx_ba_cb_irqsafe() or return the special
+ *	call ieee80211_start_tx_ba_cb_irqsafe() or
+ *	call ieee80211_start_tx_ba_cb_irqsafe() with status
+ *	%IEEE80211_AMPDU_TX_START_DELAY_ADDBA to delay addba after
+ *	ieee80211_start_tx_ba_cb_irqsafe is called, or just return the special
  *	status %IEEE80211_AMPDU_TX_START_IMMEDIATE.
  * @IEEE80211_AMPDU_TX_OPERATIONAL: TX aggregation has become operational
  * @IEEE80211_AMPDU_TX_STOP_CONT: stop TX aggregation but continue transmitting
@@ -3145,6 +3148,7 @@ enum ieee80211_ampdu_mlme_action {
 };
 
 #define IEEE80211_AMPDU_TX_START_IMMEDIATE 1
+#define IEEE80211_AMPDU_TX_START_DELAY_ADDBA 2
 
 /**
  * struct ieee80211_ampdu_params - AMPDU action parameters
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 32f40c4f3120..c2d5f512526d 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -483,6 +483,8 @@ static void ieee80211_send_addba_with_timeout(struct sta_info *sta,
 				     tid_tx->dialog_token,
 				     sta->tid_seq[tid] >> 4,
 				     buf_size, tid_tx->timeout);
+
+	WARN_ON(test_and_set_bit(HT_AGG_STATE_SENT_ADDBA, &tid_tx->state));
 }
 
 void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
@@ -521,7 +523,9 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
 
 	params.ssn = sta->tid_seq[tid] >> 4;
 	ret = drv_ampdu_action(local, sdata, &params);
-	if (ret == IEEE80211_AMPDU_TX_START_IMMEDIATE) {
+	if (ret == IEEE80211_AMPDU_TX_START_DELAY_ADDBA) {
+		return;
+	} else if (ret == IEEE80211_AMPDU_TX_START_IMMEDIATE) {
 		/*
 		 * We didn't send the request yet, so don't need to check
 		 * here if we already got a response, just mark as driver
@@ -765,6 +769,12 @@ void ieee80211_start_tx_ba_cb(struct sta_info *sta, int tid,
 	if (WARN_ON(test_and_set_bit(HT_AGG_STATE_DRV_READY, &tid_tx->state)))
 		return;
 
+	if (!test_bit(HT_AGG_STATE_SENT_ADDBA, &tid_tx->state)) {
+		ieee80211_send_addba_with_timeout(sta, tid_tx);
+		/* RESPONSE_RECEIVED state whould trigger the flow again */
+		return;
+	}
+
 	if (test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state))
 		ieee80211_agg_tx_operational(local, sta, tid);
 }
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 364a35414d05..78ab18eee1ec 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -3,6 +3,7 @@
  * Copyright 2002-2005, Devicescape Software, Inc.
  * Copyright 2013-2014  Intel Mobile Communications GmbH
  * Copyright(c) 2015-2017 Intel Deutschland GmbH
+ * Copyright(c) 2020 Intel Corporation
  */
 
 #ifndef STA_INFO_H
@@ -115,6 +116,7 @@ enum ieee80211_sta_info_flags {
 #define HT_AGG_STATE_WANT_STOP		5
 #define HT_AGG_STATE_START_CB		6
 #define HT_AGG_STATE_STOP_CB		7
+#define HT_AGG_STATE_SENT_ADDBA		8
 
 DECLARE_EWMA(avg_signal, 10, 8)
 enum ieee80211_agg_stop_reason {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 08/12] mac80211: Fail association when AP has no legacy rates
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (6 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 07/12] mac80211: agg-tx: add an option to defer ADDBA transmit Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 09/12] mac80211: minstrel_ht_assign_best_tp_rates: remove redundant test Luca Coelho
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Ilan Peer <ilan.peer@intel.com>

The MLME logic had a workaround that allowed to continue an
association with an AP even if the AP did not provide any basic
rates in its supported rates in the association response, assuming
that the first (non basic) legacy rate could be used as a basic rate.
However, this did not consider the case where the AP (which is
obviously buggy) did not provide any legacy rate.

Fix this by failing the association, as this can result in
an unexpected failure in the low level driver and FW, e.g., in
rate scale logic etc.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/mlme.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 04ed883339c7..2b57c53ab070 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5037,8 +5037,16 @@ static int ieee80211_prep_connection(struct ieee80211_sub_if_data *sdata,
 		 * doesn't happen any more, but keep the workaround so
 		 * in case some *other* APs are buggy in different ways
 		 * we can connect -- with a warning.
+		 * Allow this workaround only in case the AP provided at least
+		 * one rate.
 		 */
-		if (!basic_rates && min_rate_index >= 0) {
+		if (min_rate_index < 0) {
+			sdata_info(sdata,
+				   "No legacy rates in association response\n");
+
+			sta_info_free(local, new_sta);
+			return -EINVAL;
+		} else if (!basic_rates) {
 			sdata_info(sdata,
 				   "No basic rates, using min rate instead\n");
 			basic_rates = BIT(min_rate_index);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 09/12] mac80211: minstrel_ht_assign_best_tp_rates: remove redundant test
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (7 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 08/12] mac80211: Fail association when AP has no legacy rates Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 10/12] mac80211_hwsim: indicate in IBSS that we have transmitted beacons Luca Coelho
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

We know this pointer isn't NULL and in fact dereferenced it before,
remove the redundant test.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/rc80211_minstrel_ht.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 694a31978a04..5547111d22bf 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (C) 2010-2013 Felix Fietkau <nbd@openwrt.org>
+ * Copyright (C) 2019-2020 Intel Corporation
  */
 #include <linux/netdevice.h>
 #include <linux/types.h>
@@ -490,7 +491,7 @@ minstrel_ht_assign_best_tp_rates(struct minstrel_ht_sta *mi,
 	tmp_prob = mi->groups[tmp_group].rates[tmp_idx].prob_avg;
 	tmp_mcs_tp = minstrel_ht_get_tp_avg(mi, tmp_group, tmp_idx, tmp_prob);
 
-	if (tmp_cck_tp_rate && tmp_cck_tp > tmp_mcs_tp) {
+	if (tmp_cck_tp > tmp_mcs_tp) {
 		for(i = 0; i < MAX_THR_RATES; i++) {
 			minstrel_ht_sort_best_tp_rates(mi, tmp_cck_tp_rate[i],
 						       tmp_mcs_tp_rate);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 10/12] mac80211_hwsim: indicate in IBSS that we have transmitted beacons
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (8 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 09/12] mac80211: minstrel_ht_assign_best_tp_rates: remove redundant test Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links Luca Coelho
  2020-03-26 13:09 ` [PATCH v2 12/12] cfg80211: Do not warn on same channel at the end of CSA Luca Coelho
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

This is actually true because there's no functional beacon distribution
and lets us get active scanning working - without it, mac80211 doesn't
respond to probe requests.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 4d7141d06027..21148c6d24af 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2464,6 +2464,11 @@ static void mac80211_hwsim_get_et_stats(struct ieee80211_hw *hw,
 	WARN_ON(i != MAC80211_HWSIM_SSTATS_LEN);
 }
 
+static int mac80211_hwsim_tx_last_beacon(struct ieee80211_hw *hw)
+{
+	return 1;
+}
+
 #define HWSIM_COMMON_OPS					\
 	.tx = mac80211_hwsim_tx,				\
 	.start = mac80211_hwsim_start,				\
@@ -2474,6 +2479,7 @@ static void mac80211_hwsim_get_et_stats(struct ieee80211_hw *hw,
 	.config = mac80211_hwsim_config,			\
 	.configure_filter = mac80211_hwsim_configure_filter,	\
 	.bss_info_changed = mac80211_hwsim_bss_info_changed,	\
+	.tx_last_beacon = mac80211_hwsim_tx_last_beacon,	\
 	.sta_add = mac80211_hwsim_sta_add,			\
 	.sta_remove = mac80211_hwsim_sta_remove,		\
 	.sta_notify = mac80211_hwsim_sta_notify,		\
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (9 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 10/12] mac80211_hwsim: indicate in IBSS that we have transmitted beacons Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  2020-03-27 15:03   ` Sasha Levin
  2020-03-26 13:09 ` [PATCH v2 12/12] cfg80211: Do not warn on same channel at the end of CSA Luca Coelho
  11 siblings, 1 reply; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

If we know that we have an encrypted link (based on having had
a key configured for TX in the past) then drop all data frames
in the key selection handler if there's no key anymore.

This fixes an issue with mac80211 internal TXQs - there we can
buffer frames for an encrypted link, but then if the key is no
longer there when they're dequeued, the frames are sent without
encryption. This happens if a station is disconnected while the
frames are still on the TXQ.

Detecting that a link should be encrypted based on a first key
having been configured for TX is fine as there are no use cases
for a connection going from with encryption to no encryption.
With extended key IDs, however, there is a case of having a key
configured for only decryption, so we can't just trigger this
behaviour on a key being configured.

Cc: stable@vger.kernel.org
Reported-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/debugfs_sta.c |  3 ++-
 net/mac80211/key.c         | 20 ++++++++++++--------
 net/mac80211/sta_info.h    |  1 +
 net/mac80211/tx.c          | 12 +++++++++---
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 266d63819415..829dcad69c2c 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -5,7 +5,7 @@
  * Copyright 2007	Johannes Berg <johannes@sipsolutions.net>
  * Copyright 2013-2014  Intel Mobile Communications GmbH
  * Copyright(c) 2016 Intel Deutschland GmbH
- * Copyright (C) 2018 - 2019 Intel Corporation
+ * Copyright (C) 2018 - 2020 Intel Corporation
  */
 
 #include <linux/debugfs.h>
@@ -78,6 +78,7 @@ static const char * const sta_flag_names[] = {
 	FLAG(MPSP_OWNER),
 	FLAG(MPSP_RECIPIENT),
 	FLAG(PS_DELIVER),
+	FLAG(USES_ENCRYPTION),
 #undef FLAG
 };
 
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 6354491c5a09..8f403c1bb908 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -6,7 +6,7 @@
  * Copyright 2007-2008	Johannes Berg <johannes@sipsolutions.net>
  * Copyright 2013-2014  Intel Mobile Communications GmbH
  * Copyright 2015-2017	Intel Deutschland GmbH
- * Copyright 2018-2019  Intel Corporation
+ * Copyright 2018-2020  Intel Corporation
  */
 
 #include <linux/if_ether.h>
@@ -277,22 +277,29 @@ static void ieee80211_key_disable_hw_accel(struct ieee80211_key *key)
 			  sta ? sta->sta.addr : bcast_addr, ret);
 }
 
-int ieee80211_set_tx_key(struct ieee80211_key *key)
+static int _ieee80211_set_tx_key(struct ieee80211_key *key, bool force)
 {
 	struct sta_info *sta = key->sta;
 	struct ieee80211_local *local = key->local;
 
 	assert_key_lock(local);
 
+	set_sta_flag(sta, WLAN_STA_USES_ENCRYPTION);
+
 	sta->ptk_idx = key->conf.keyidx;
 
-	if (!ieee80211_hw_check(&local->hw, AMPDU_KEYBORDER_SUPPORT))
+	if (force || !ieee80211_hw_check(&local->hw, AMPDU_KEYBORDER_SUPPORT))
 		clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
 	ieee80211_check_fast_xmit(sta);
 
 	return 0;
 }
 
+int ieee80211_set_tx_key(struct ieee80211_key *key)
+{
+	return _ieee80211_set_tx_key(key, false);
+}
+
 static void ieee80211_pairwise_rekey(struct ieee80211_key *old,
 				     struct ieee80211_key *new)
 {
@@ -481,11 +488,8 @@ static int ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
 		if (pairwise) {
 			rcu_assign_pointer(sta->ptk[idx], new);
 			if (new &&
-			    !(new->conf.flags & IEEE80211_KEY_FLAG_NO_AUTO_TX)) {
-				sta->ptk_idx = idx;
-				clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
-				ieee80211_check_fast_xmit(sta);
-			}
+			    !(new->conf.flags & IEEE80211_KEY_FLAG_NO_AUTO_TX))
+				_ieee80211_set_tx_key(new, true);
 		} else {
 			rcu_assign_pointer(sta->gtk[idx], new);
 		}
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 78ab18eee1ec..a5de3aa6ea42 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -99,6 +99,7 @@ enum ieee80211_sta_info_flags {
 	WLAN_STA_MPSP_OWNER,
 	WLAN_STA_MPSP_RECIPIENT,
 	WLAN_STA_PS_DELIVER,
+	WLAN_STA_USES_ENCRYPTION,
 
 	NUM_WLAN_STA_FLAGS,
 };
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 49d35936cc9d..637c22a4256f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -590,10 +590,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
 
-	if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT))
+	if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) {
 		tx->key = NULL;
-	else if (tx->sta &&
-		 (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
+		return TX_CONTINUE;
+	}
+
+	if (tx->sta &&
+	    (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
 		tx->key = key;
 	else if (ieee80211_is_group_privacy_action(tx->skb) &&
 		(key = rcu_dereference(tx->sdata->default_multicast_key)))
@@ -654,6 +657,9 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 		if (!skip_hw && tx->key &&
 		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
 			info->control.hw_key = &tx->key->conf;
+	} else if (!ieee80211_is_mgmt(hdr->frame_control) && tx->sta &&
+		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
+		return TX_DROP;
 	}
 
 	return TX_CONTINUE;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 12/12] cfg80211: Do not warn on same channel at the end of CSA
  2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
                   ` (10 preceding siblings ...)
  2020-03-26 13:09 ` [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links Luca Coelho
@ 2020-03-26 13:09 ` Luca Coelho
  11 siblings, 0 replies; 22+ messages in thread
From: Luca Coelho @ 2020-03-26 13:09 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

From: Ilan Peer <ilan.peer@intel.com>

When cfg80211_update_assoc_bss_entry() is called, there is a
verification that the BSS channel actually changed. As some APs use
CSA also for bandwidth changes, this would result with a kernel
warning.

Fix this by removing the WARN_ON().

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/scan.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index dd41e41f9d26..4000382aef48 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -2019,7 +2019,11 @@ void cfg80211_update_assoc_bss_entry(struct wireless_dev *wdev,
 
 	spin_lock_bh(&rdev->bss_lock);
 
-	if (WARN_ON(cbss->pub.channel == chan))
+	/*
+	 * Some APs use CSA also for bandwidth changes, i.e., without actually
+	 * changing the control channel, so no need to update in such a case.
+	 */
+	if (cbss->pub.channel == chan)
 		goto done;
 
 	/* use transmitting bss */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links
  2020-03-26 13:09 ` [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links Luca Coelho
@ 2020-03-27 15:03   ` Sasha Levin
  2021-06-11 10:10     ` Pali Rohár
  2021-08-16 13:44     ` [PATCH] " Pali Rohár
  0 siblings, 2 replies; 22+ messages in thread
From: Sasha Levin @ 2020-03-27 15:03 UTC (permalink / raw)
  To: Sasha Levin, Luca Coelho, Johannes Berg, johannes
  Cc: linux-wireless, stable, stable

Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.5.11, v5.4.27, v4.19.112, v4.14.174, v4.9.217, v4.4.217.

v5.5.11: Build OK!
v5.4.27: Build OK!
v4.19.112: Failed to apply! Possible dependencies:
    03512ceb60ae ("ieee80211: remove redundant leading zeroes")
    09b4a4faf9d0 ("mac80211: introduce capability flags for VHT EXT NSS support")
    0eeb2b674f05 ("mac80211: add an option for station management TXQ")
    1c9559734eca ("mac80211: remove unnecessary key condition")
    57a3a454f303 ("iwlwifi: split HE capabilities between AP and STA")
    62872a9b9a10 ("mac80211: Fix PTK rekey freezes and clear text leak")
    77f7ffdc335d ("mac80211: minstrel_ht: add flag to indicate missing/inaccurate tx A-MPDU length")
    77ff2c6b4984 ("mac80211: update HE IEs to D3.3")
    80aaa9c16415 ("mac80211: Add he_capa debugfs entry")
    96fc6efb9ad9 ("mac80211: IEEE 802.11 Extended Key ID support")
    add7453ad62f ("wireless: align to draft 11ax D3.0")
    adf8ed01e4fd ("mac80211: add an optional TXQ for other PS-buffered frames")
    caf56338c22f ("mac80211: indicate support for multiple BSSID")
    daa5b83513a7 ("mac80211: update HE operation fields to D3.0")

v4.14.174: Failed to apply! Possible dependencies:
    110b32f065f3 ("iwlwifi: mvm: rs: add basic implementation of the new RS API handlers")
    1c73acf58bd6 ("iwlwifi: acpi: move ACPI method definitions to acpi.h")
    28e9c00fe1f0 ("iwlwifi: remove upper case letters in sku_capa_band_*_enable")
    4ae80f6c8d86 ("iwlwifi: support api ver2 of NVM_GET_INFO resp")
    4b82455ca51e ("iwlwifi: use flags to denote modifiers for the channel maps")
    4c625c564ba2 ("iwlwifi: get rid of fw/nvm.c")
    514c30696fbc ("iwlwifi: add support for IEEE802.11ax")
    57a3a454f303 ("iwlwifi: split HE capabilities between AP and STA")
    77ff2c6b4984 ("mac80211: update HE IEs to D3.3")
    813df5cef3bb ("iwlwifi: acpi: add common code to read from ACPI")
    8a6171a7b601 ("iwlwifi: fw: add FW APIs for HE")
    9c4f7d512740 ("iwlwifi: move all NVM parsing code to the common files")
    9f66a397c877 ("iwlwifi: mvm: rs: add ops for the new rate scaling in the FW")
    e7a3b8d87910 ("iwlwifi: acpi: move ACPI-related definitions to acpi.h")

v4.9.217: Failed to apply! Possible dependencies:
    01796ff2fa6e ("iwlwifi: mvm: always free inactive queue when moving ownership")
    0aaece81114e ("iwlwifi: split firmware API from iwl-trans.h")
    1ea423b0e047 ("iwlwifi: remove unnecessary dev_cmd_headroom parameter")
    310181ec34e2 ("iwlwifi: move to TVQM mode")
    5594d80e9bf4 ("iwlwifi: support two phys for a000 devices")
    623e7766be90 ("iwlwifi: pcie: introduce split point to a000 devices")
    65e254821cee ("iwlwifi: mvm: use firmware station PM notification for AP_LINK_PS")
    6b35ff91572f ("iwlwifi: pcie: introduce a000 TX queues management")
    727c02dfb848 ("iwlwifi: pcie: cleanup rfkill checks")
    77ff2c6b4984 ("mac80211: update HE IEs to D3.3")
    8236f7db2724 ("iwlwifi: mvm: assign cab queue to the correct station")
    87d0e1af9db3 ("iwlwifi: mvm: separate queue mapping from queue enablement")
    bb49701b41de ("iwlwifi: mvm: support a000 SCD queue configuration")
    cf90da352a32 ("iwlwifi: mvm: use mvm_disable_queue instead of sharing logic")
    d172a5eff629 ("iwlwifi: reorganize firmware API")
    df88c08d5c7e ("iwlwifi: mvm: release static queues on bcast release")
    eda50cde58de ("iwlwifi: pcie: add context information support")

v4.4.217: Failed to apply! Possible dependencies:
    0aaece81114e ("iwlwifi: split firmware API from iwl-trans.h")
    13555e8ba2f4 ("iwlwifi: mvm: add 9000-series RX API")
    1a616dd2f171 ("iwlwifi: dump prph registers in a common place for all transports")
    1e0bbebaae66 ("mac80211: enable starting BA session with custom timeout")
    2f89a5d7d377 ("iwlwifi: mvm: move fw-dbg code to separate file")
    39bdb17ebb5b ("iwlwifi: update host command messages to new format")
    41837ca962ec ("iwlwifi: pcie: allow to pretend to have Tx CSUM for debug")
    4707fde5cdef ("iwlwifi: mvm: use build-time assertion for fw trigger ID")
    5b88792cd850 ("iwlwifi: move to wide ID for all commands")
    6c4fbcbc1c95 ("iwlwifi: add support for 12K Receive Buffers")
    77ff2c6b4984 ("mac80211: update HE IEs to D3.3")
    92fe83430b89 ("iwlwifi: uninline iwl_trans_send_cmd")
    d172a5eff629 ("iwlwifi: reorganize firmware API")
    dcbb4746286a ("iwlwifi: trans: support a callback for ASYNC commands")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links
  2020-03-27 15:03   ` Sasha Levin
@ 2021-06-11 10:10     ` Pali Rohár
  2021-06-22 23:15       ` Pali Rohár
  2021-06-23 12:16       ` Johannes Berg
  2021-08-16 13:44     ` [PATCH] " Pali Rohár
  1 sibling, 2 replies; 22+ messages in thread
From: Pali Rohár @ 2021-06-11 10:10 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Luca Coelho, Johannes Berg, johannes, linux-wireless, stable

On Friday 27 March 2020 15:03:41 Sasha Levin wrote:
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
> 
> The bot has tested the following trees: v5.5.11, v5.4.27, v4.19.112, v4.14.174, v4.9.217, v4.4.217.
> 
> v5.5.11: Build OK!
> v5.4.27: Build OK!
> v4.19.112: Failed to apply! Possible dependencies:
...
> v4.14.174: Failed to apply! Possible dependencies:
...
> v4.9.217: Failed to apply! Possible dependencies:
...
> v4.4.217: Failed to apply! Possible dependencies:
...
> 
> How should we proceed with this patch?

Hello! I have looked at this patch and backported it into 4.19 and older
versions. But as this patch is security related and backporting needed
some code changes, it is required to review this patch prior including
it into any stable branch. Patch is below.

The main change in backported patch is in ieee80211_key_replace()
function.

So could you please review this patch if it is correct and if it is
suitable for backporting it into stable kernels 4.19 in this (or other)
form?

=======================================================================

From 189da5743e28d0c5d211b70b4cb06ce3aff77d86 Mon Sep 17 00:00:00 2001
From: Johannes Berg <johannes.berg@intel.com>
Date: Thu, 26 Mar 2020 15:09:42 +0200
Subject: [PATCH] mac80211: drop data frames without key on encrypted links
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

commit a0761a301746ec2d92d7fcb82af69c0a6a4339aa upstream.

If we know that we have an encrypted link (based on having had
a key configured for TX in the past) then drop all data frames
in the key selection handler if there's no key anymore.

This fixes an issue with mac80211 internal TXQs - there we can
buffer frames for an encrypted link, but then if the key is no
longer there when they're dequeued, the frames are sent without
encryption. This happens if a station is disconnected while the
frames are still on the TXQ.

Detecting that a link should be encrypted based on a first key
having been configured for TX is fine as there are no use cases
for a connection going from with encryption to no encryption.
With extended key IDs, however, there is a case of having a key
configured for only decryption, so we can't just trigger this
behaviour on a key being configured.

Cc: stable@vger.kernel.org
Reported-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Link: https://lore.kernel.org/r/iwlwifi.20200326150855.6865c7f28a14.I9fb1d911b064262d33e33dfba730cdeef83926ca@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
[pali: Backported to 4.19 and older versions]
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 net/mac80211/debugfs_sta.c |  1 +
 net/mac80211/key.c         |  7 +++++--
 net/mac80211/sta_info.h    |  1 +
 net/mac80211/tx.c          | 12 +++++++++---
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 4105081dc1df..6f390c2e4c8e 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -80,6 +80,7 @@ static const char * const sta_flag_names[] = {
 	FLAG(MPSP_OWNER),
 	FLAG(MPSP_RECIPIENT),
 	FLAG(PS_DELIVER),
+	FLAG(USES_ENCRYPTION),
 #undef FLAG
 };
 
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index f20bb39f492d..217db25a1afa 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -341,8 +341,11 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
 	if (sta) {
 		if (pairwise) {
 			rcu_assign_pointer(sta->ptk[idx], new);
-			sta->ptk_idx = idx;
-			ieee80211_check_fast_xmit(sta);
+			if (new) {
+				set_sta_flag(new->sta, WLAN_STA_USES_ENCRYPTION);
+				new->sta->ptk_idx = new->conf.keyidx;
+				ieee80211_check_fast_xmit(new->sta);
+			}
 		} else {
 			rcu_assign_pointer(sta->gtk[idx], new);
 		}
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 9a04327d71d1..075609c4571d 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -101,6 +101,7 @@ enum ieee80211_sta_info_flags {
 	WLAN_STA_MPSP_OWNER,
 	WLAN_STA_MPSP_RECIPIENT,
 	WLAN_STA_PS_DELIVER,
+	WLAN_STA_USES_ENCRYPTION,
 
 	NUM_WLAN_STA_FLAGS,
 };
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 98d048630ad2..3530d1a5fc98 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -593,10 +593,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
 
-	if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT))
+	if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) {
 		tx->key = NULL;
-	else if (tx->sta &&
-		 (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
+		return TX_CONTINUE;
+	}
+
+	if (tx->sta &&
+	    (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
 		tx->key = key;
 	else if (ieee80211_is_group_privacy_action(tx->skb) &&
 		(key = rcu_dereference(tx->sdata->default_multicast_key)))
@@ -657,6 +660,9 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 		if (!skip_hw && tx->key &&
 		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
 			info->control.hw_key = &tx->key->conf;
+	} else if (!ieee80211_is_mgmt(hdr->frame_control) && tx->sta &&
+		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
+		return TX_DROP;
 	}
 
 	return TX_CONTINUE;
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links
  2021-06-11 10:10     ` Pali Rohár
@ 2021-06-22 23:15       ` Pali Rohár
  2021-06-23 14:55         ` Greg KH
  2021-06-23 12:16       ` Johannes Berg
  1 sibling, 1 reply; 22+ messages in thread
From: Pali Rohár @ 2021-06-22 23:15 UTC (permalink / raw)
  To: Sasha Levin, Greg KH, Johannes Berg
  Cc: Luca Coelho, johannes, linux-wireless, stable

On Friday 11 June 2021 12:10:46 Pali Rohár wrote:
> On Friday 27 March 2020 15:03:41 Sasha Levin wrote:
> > This commit has been processed because it contains a -stable tag.
> > The stable tag indicates that it's relevant for the following trees: all
> > 
> > The bot has tested the following trees: v5.5.11, v5.4.27, v4.19.112, v4.14.174, v4.9.217, v4.4.217.
> > 
> > v5.5.11: Build OK!
> > v5.4.27: Build OK!
> > v4.19.112: Failed to apply! Possible dependencies:
> ...
> > v4.14.174: Failed to apply! Possible dependencies:
> ...
> > v4.9.217: Failed to apply! Possible dependencies:
> ...
> > v4.4.217: Failed to apply! Possible dependencies:
> ...
> > 
> > How should we proceed with this patch?
> 
> Hello! I have looked at this patch and backported it into 4.19 and older
> versions. But as this patch is security related and backporting needed
> some code changes, it is required to review this patch prior including
> it into any stable branch. Patch is below.

Hello Sasha and Greg!

Do you have any opinion how do you want to process this patch? I would
like to know if something else is needed from my side.

> The main change in backported patch is in ieee80211_key_replace()
> function.
> 
> So could you please review this patch if it is correct and if it is
> suitable for backporting it into stable kernels 4.19 in this (or other)
> form?

Johannes, you are the original author of this patch, what do you think,
would you be able to find some time and review at this backported patch?

> =======================================================================
> 
> From 189da5743e28d0c5d211b70b4cb06ce3aff77d86 Mon Sep 17 00:00:00 2001
> From: Johannes Berg <johannes.berg@intel.com>
> Date: Thu, 26 Mar 2020 15:09:42 +0200
> Subject: [PATCH] mac80211: drop data frames without key on encrypted links
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> commit a0761a301746ec2d92d7fcb82af69c0a6a4339aa upstream.
> 
> If we know that we have an encrypted link (based on having had
> a key configured for TX in the past) then drop all data frames
> in the key selection handler if there's no key anymore.
> 
> This fixes an issue with mac80211 internal TXQs - there we can
> buffer frames for an encrypted link, but then if the key is no
> longer there when they're dequeued, the frames are sent without
> encryption. This happens if a station is disconnected while the
> frames are still on the TXQ.
> 
> Detecting that a link should be encrypted based on a first key
> having been configured for TX is fine as there are no use cases
> for a connection going from with encryption to no encryption.
> With extended key IDs, however, there is a case of having a key
> configured for only decryption, so we can't just trigger this
> behaviour on a key being configured.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Jouni Malinen <j@w1.fi>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> Link: https://lore.kernel.org/r/iwlwifi.20200326150855.6865c7f28a14.I9fb1d911b064262d33e33dfba730cdeef83926ca@changeid
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> [pali: Backported to 4.19 and older versions]
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  net/mac80211/debugfs_sta.c |  1 +
>  net/mac80211/key.c         |  7 +++++--
>  net/mac80211/sta_info.h    |  1 +
>  net/mac80211/tx.c          | 12 +++++++++---
>  4 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
> index 4105081dc1df..6f390c2e4c8e 100644
> --- a/net/mac80211/debugfs_sta.c
> +++ b/net/mac80211/debugfs_sta.c
> @@ -80,6 +80,7 @@ static const char * const sta_flag_names[] = {
>  	FLAG(MPSP_OWNER),
>  	FLAG(MPSP_RECIPIENT),
>  	FLAG(PS_DELIVER),
> +	FLAG(USES_ENCRYPTION),
>  #undef FLAG
>  };
>  
> diff --git a/net/mac80211/key.c b/net/mac80211/key.c
> index f20bb39f492d..217db25a1afa 100644
> --- a/net/mac80211/key.c
> +++ b/net/mac80211/key.c
> @@ -341,8 +341,11 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
>  	if (sta) {
>  		if (pairwise) {
>  			rcu_assign_pointer(sta->ptk[idx], new);
> -			sta->ptk_idx = idx;
> -			ieee80211_check_fast_xmit(sta);
> +			if (new) {
> +				set_sta_flag(new->sta, WLAN_STA_USES_ENCRYPTION);
> +				new->sta->ptk_idx = new->conf.keyidx;
> +				ieee80211_check_fast_xmit(new->sta);
> +			}
>  		} else {
>  			rcu_assign_pointer(sta->gtk[idx], new);
>  		}
> diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
> index 9a04327d71d1..075609c4571d 100644
> --- a/net/mac80211/sta_info.h
> +++ b/net/mac80211/sta_info.h
> @@ -101,6 +101,7 @@ enum ieee80211_sta_info_flags {
>  	WLAN_STA_MPSP_OWNER,
>  	WLAN_STA_MPSP_RECIPIENT,
>  	WLAN_STA_PS_DELIVER,
> +	WLAN_STA_USES_ENCRYPTION,
>  
>  	NUM_WLAN_STA_FLAGS,
>  };
> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> index 98d048630ad2..3530d1a5fc98 100644
> --- a/net/mac80211/tx.c
> +++ b/net/mac80211/tx.c
> @@ -593,10 +593,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
>  	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
>  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
>  
> -	if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT))
> +	if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) {
>  		tx->key = NULL;
> -	else if (tx->sta &&
> -		 (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
> +		return TX_CONTINUE;
> +	}
> +
> +	if (tx->sta &&
> +	    (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
>  		tx->key = key;
>  	else if (ieee80211_is_group_privacy_action(tx->skb) &&
>  		(key = rcu_dereference(tx->sdata->default_multicast_key)))
> @@ -657,6 +660,9 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
>  		if (!skip_hw && tx->key &&
>  		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
>  			info->control.hw_key = &tx->key->conf;
> +	} else if (!ieee80211_is_mgmt(hdr->frame_control) && tx->sta &&
> +		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
> +		return TX_DROP;
>  	}
>  
>  	return TX_CONTINUE;
> -- 
> 2.20.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links
  2021-06-11 10:10     ` Pali Rohár
  2021-06-22 23:15       ` Pali Rohár
@ 2021-06-23 12:16       ` Johannes Berg
  2021-06-29 21:32         ` Pali Rohár
  1 sibling, 1 reply; 22+ messages in thread
From: Johannes Berg @ 2021-06-23 12:16 UTC (permalink / raw)
  To: Pali Rohár, Sasha Levin; +Cc: Luca Coelho, linux-wireless, stable

On Fri, 2021-06-11 at 12:10 +0200, Pali Rohár wrote:
> 
> @@ -341,8 +341,11 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
>  	if (sta) {
>  		if (pairwise) {
>  			rcu_assign_pointer(sta->ptk[idx], new);
> -			sta->ptk_idx = idx;
> -			ieee80211_check_fast_xmit(sta);
> +			if (new) {
> +				set_sta_flag(new->sta, WLAN_STA_USES_ENCRYPTION);
> +				new->sta->ptk_idx = new->conf.keyidx;

I'm not entirely sure moving that assignment under the guard is correct.

> +				ieee80211_check_fast_xmit(new->sta);

and I'm pretty sure that moving call under the guard is incorrect,
although in the end it probably doesn't even matter if we will drop all
frames anyway (due to this patch).

So all you need under the assignment is the flag, but also only
theoretically, because the function cannot be called with old==NULL &&
new==NULL, the first time around it's called we must have old==NULL (no
key was ever installed), and so the first time it's called it must be
old==NULL && new!=NULL, and then the flag gets set and we never want to
clear it again, so I believe you don't need the "if (new)" condition at
all.

In the code as it was in (and before) my patch the condition is
necessary because we use 'new' to obtain the 'sta' and 'local' pointers,
but otherwise we don't really need it even in the current version.

johannes




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links
  2021-06-22 23:15       ` Pali Rohár
@ 2021-06-23 14:55         ` Greg KH
  0 siblings, 0 replies; 22+ messages in thread
From: Greg KH @ 2021-06-23 14:55 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Sasha Levin, Johannes Berg, Luca Coelho, johannes,
	linux-wireless, stable

On Wed, Jun 23, 2021 at 01:15:11AM +0200, Pali Rohár wrote:
> On Friday 11 June 2021 12:10:46 Pali Rohár wrote:
> > On Friday 27 March 2020 15:03:41 Sasha Levin wrote:
> > > This commit has been processed because it contains a -stable tag.
> > > The stable tag indicates that it's relevant for the following trees: all
> > > 
> > > The bot has tested the following trees: v5.5.11, v5.4.27, v4.19.112, v4.14.174, v4.9.217, v4.4.217.
> > > 
> > > v5.5.11: Build OK!
> > > v5.4.27: Build OK!
> > > v4.19.112: Failed to apply! Possible dependencies:
> > ...
> > > v4.14.174: Failed to apply! Possible dependencies:
> > ...
> > > v4.9.217: Failed to apply! Possible dependencies:
> > ...
> > > v4.4.217: Failed to apply! Possible dependencies:
> > ...
> > > 
> > > How should we proceed with this patch?
> > 
> > Hello! I have looked at this patch and backported it into 4.19 and older
> > versions. But as this patch is security related and backporting needed
> > some code changes, it is required to review this patch prior including
> > it into any stable branch. Patch is below.
> 
> Hello Sasha and Greg!
> 
> Do you have any opinion how do you want to process this patch? I would
> like to know if something else is needed from my side.

If it is not applied, please resend it, it's not in any queue that I can
see...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links
  2021-06-23 12:16       ` Johannes Berg
@ 2021-06-29 21:32         ` Pali Rohár
  2021-06-30  6:49           ` Johannes Berg
  0 siblings, 1 reply; 22+ messages in thread
From: Pali Rohár @ 2021-06-29 21:32 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Sasha Levin, Luca Coelho, linux-wireless, stable

On Wednesday 23 June 2021 14:16:12 Johannes Berg wrote:
> On Fri, 2021-06-11 at 12:10 +0200, Pali Rohár wrote:
> > 
> > @@ -341,8 +341,11 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
> >  	if (sta) {
> >  		if (pairwise) {
> >  			rcu_assign_pointer(sta->ptk[idx], new);
> > -			sta->ptk_idx = idx;
> > -			ieee80211_check_fast_xmit(sta);
> > +			if (new) {
> > +				set_sta_flag(new->sta, WLAN_STA_USES_ENCRYPTION);
> > +				new->sta->ptk_idx = new->conf.keyidx;
> 
> I'm not entirely sure moving that assignment under the guard is correct.
> 
> > +				ieee80211_check_fast_xmit(new->sta);
> 
> and I'm pretty sure that moving call under the guard is incorrect,
> although in the end it probably doesn't even matter if we will drop all
> frames anyway (due to this patch).
> 
> So all you need under the assignment is the flag, but also only
> theoretically, because the function cannot be called with old==NULL &&
> new==NULL, the first time around it's called we must have old==NULL (no
> key was ever installed), and so the first time it's called it must be
> old==NULL && new!=NULL, and then the flag gets set and we never want to
> clear it again, so I believe you don't need the "if (new)" condition at
> all.
> 
> In the code as it was in (and before) my patch the condition is
> necessary because we use 'new' to obtain the 'sta' and 'local' pointers,
> but otherwise we don't really need it even in the current version.
> 
> johannes

Now I see, thank you for explanation. So the code should be like this:

 		if (pairwise) {
 			rcu_assign_pointer(sta->ptk[idx], new);
+			set_sta_flag(sta, WLAN_STA_USES_ENCRYPTION);
 			sta->ptk_idx = idx;
 			ieee80211_check_fast_xmit(sta);
 		} else {

Right?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links
  2021-06-29 21:32         ` Pali Rohár
@ 2021-06-30  6:49           ` Johannes Berg
  0 siblings, 0 replies; 22+ messages in thread
From: Johannes Berg @ 2021-06-30  6:49 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Sasha Levin, Luca Coelho, linux-wireless, stable

On Tue, 2021-06-29 at 23:32 +0200, Pali Rohár wrote:
> On Wednesday 23 June 2021 14:16:12 Johannes Berg wrote:
> > On Fri, 2021-06-11 at 12:10 +0200, Pali Rohár wrote:
> > > 
> > > @@ -341,8 +341,11 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
> > >  	if (sta) {
> > >  		if (pairwise) {
> > >  			rcu_assign_pointer(sta->ptk[idx], new);
> > > -			sta->ptk_idx = idx;
> > > -			ieee80211_check_fast_xmit(sta);
> > > +			if (new) {
> > > +				set_sta_flag(new->sta, WLAN_STA_USES_ENCRYPTION);
> > > +				new->sta->ptk_idx = new->conf.keyidx;
> > 
> > I'm not entirely sure moving that assignment under the guard is correct.
> > 
> > > +				ieee80211_check_fast_xmit(new->sta);
> > 
> > and I'm pretty sure that moving call under the guard is incorrect,
> > although in the end it probably doesn't even matter if we will drop all
> > frames anyway (due to this patch).
> > 
> > So all you need under the assignment is the flag, but also only
> > theoretically, because the function cannot be called with old==NULL &&
> > new==NULL, the first time around it's called we must have old==NULL (no
> > key was ever installed), and so the first time it's called it must be
> > old==NULL && new!=NULL, and then the flag gets set and we never want to
> > clear it again, so I believe you don't need the "if (new)" condition at
> > all.
> > 
> > In the code as it was in (and before) my patch the condition is
> > necessary because we use 'new' to obtain the 'sta' and 'local' pointers,
> > but otherwise we don't really need it even in the current version.
> > 
> > johannes
> 
> Now I see, thank you for explanation. So the code should be like this:
> 
>  		if (pairwise) {
>  			rcu_assign_pointer(sta->ptk[idx], new);
> +			set_sta_flag(sta, WLAN_STA_USES_ENCRYPTION);
>  			sta->ptk_idx = idx;
>  			ieee80211_check_fast_xmit(sta);
>  		} else {
> 
> Right?

Yes, I think so.

johannes


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH] mac80211: drop data frames without key on encrypted links
  2020-03-27 15:03   ` Sasha Levin
  2021-06-11 10:10     ` Pali Rohár
@ 2021-08-16 13:44     ` Pali Rohár
  2021-08-16 13:54       ` Greg KH
  1 sibling, 1 reply; 22+ messages in thread
From: Pali Rohár @ 2021-08-16 13:44 UTC (permalink / raw)
  To: stable; +Cc: Johannes Berg, Sasha Levin, Luca Coelho, linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

commit a0761a301746ec2d92d7fcb82af69c0a6a4339aa upstream.

If we know that we have an encrypted link (based on having had
a key configured for TX in the past) then drop all data frames
in the key selection handler if there's no key anymore.

This fixes an issue with mac80211 internal TXQs - there we can
buffer frames for an encrypted link, but then if the key is no
longer there when they're dequeued, the frames are sent without
encryption. This happens if a station is disconnected while the
frames are still on the TXQ.

Detecting that a link should be encrypted based on a first key
having been configured for TX is fine as there are no use cases
for a connection going from with encryption to no encryption.
With extended key IDs, however, there is a case of having a key
configured for only decryption, so we can't just trigger this
behaviour on a key being configured.

Cc: stable@vger.kernel.org
Reported-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Link: https://lore.kernel.org/r/iwlwifi.20200326150855.6865c7f28a14.I9fb1d911b064262d33e33dfba730cdeef83926ca@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
[pali: Backported to 4.19 and older versions]
Signed-off-by: Pali Rohár <pali@kernel.org>
---
 net/mac80211/debugfs_sta.c |  1 +
 net/mac80211/key.c         |  1 +
 net/mac80211/sta_info.h    |  1 +
 net/mac80211/tx.c          | 12 +++++++++---
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 4105081dc1df..6f390c2e4c8e 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -80,6 +80,7 @@ static const char * const sta_flag_names[] = {
 	FLAG(MPSP_OWNER),
 	FLAG(MPSP_RECIPIENT),
 	FLAG(PS_DELIVER),
+	FLAG(USES_ENCRYPTION),
 #undef FLAG
 };
 
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 6775d6cb7d3d..7fc55177db84 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -341,6 +341,7 @@ static void ieee80211_key_replace(struct ieee80211_sub_if_data *sdata,
 	if (sta) {
 		if (pairwise) {
 			rcu_assign_pointer(sta->ptk[idx], new);
+			set_sta_flag(sta, WLAN_STA_USES_ENCRYPTION);
 			sta->ptk_idx = idx;
 			ieee80211_check_fast_xmit(sta);
 		} else {
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index c33bc5fc0f2d..75d982ff7f3d 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -102,6 +102,7 @@ enum ieee80211_sta_info_flags {
 	WLAN_STA_MPSP_OWNER,
 	WLAN_STA_MPSP_RECIPIENT,
 	WLAN_STA_PS_DELIVER,
+	WLAN_STA_USES_ENCRYPTION,
 
 	NUM_WLAN_STA_FLAGS,
 };
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 98d048630ad2..3530d1a5fc98 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -593,10 +593,13 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
 
-	if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT))
+	if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) {
 		tx->key = NULL;
-	else if (tx->sta &&
-		 (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
+		return TX_CONTINUE;
+	}
+
+	if (tx->sta &&
+	    (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
 		tx->key = key;
 	else if (ieee80211_is_group_privacy_action(tx->skb) &&
 		(key = rcu_dereference(tx->sdata->default_multicast_key)))
@@ -657,6 +660,9 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
 		if (!skip_hw && tx->key &&
 		    tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
 			info->control.hw_key = &tx->key->conf;
+	} else if (!ieee80211_is_mgmt(hdr->frame_control) && tx->sta &&
+		   test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
+		return TX_DROP;
 	}
 
 	return TX_CONTINUE;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH] mac80211: drop data frames without key on encrypted links
  2021-08-16 13:44     ` [PATCH] " Pali Rohár
@ 2021-08-16 13:54       ` Greg KH
  0 siblings, 0 replies; 22+ messages in thread
From: Greg KH @ 2021-08-16 13:54 UTC (permalink / raw)
  To: Pali Rohár
  Cc: stable, Johannes Berg, Sasha Levin, Luca Coelho, linux-wireless

On Mon, Aug 16, 2021 at 03:44:24PM +0200, Pali Rohár wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> commit a0761a301746ec2d92d7fcb82af69c0a6a4339aa upstream.
> 
> If we know that we have an encrypted link (based on having had
> a key configured for TX in the past) then drop all data frames
> in the key selection handler if there's no key anymore.
> 
> This fixes an issue with mac80211 internal TXQs - there we can
> buffer frames for an encrypted link, but then if the key is no
> longer there when they're dequeued, the frames are sent without
> encryption. This happens if a station is disconnected while the
> frames are still on the TXQ.
> 
> Detecting that a link should be encrypted based on a first key
> having been configured for TX is fine as there are no use cases
> for a connection going from with encryption to no encryption.
> With extended key IDs, however, there is a case of having a key
> configured for only decryption, so we can't just trigger this
> behaviour on a key being configured.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Jouni Malinen <j@w1.fi>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> Link: https://lore.kernel.org/r/iwlwifi.20200326150855.6865c7f28a14.I9fb1d911b064262d33e33dfba730cdeef83926ca@changeid
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> [pali: Backported to 4.19 and older versions]
> Signed-off-by: Pali Rohár <pali@kernel.org>

Now queued up, thanks!

Did not apply to 4.4.y, don't know if you want it there or not...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-08-16 13:54 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 13:09 [PATCH v2 00/12] cfg80211/mac80211 patches from our internal tree 2020-03-26-2 Luca Coelho
2020-03-26 13:09 ` [PATCH v2 01/12] mac80211: implement Operating Mode Notification extended NSS support Luca Coelho
2020-03-26 13:09 ` [PATCH v2 02/12] mac80211: add twt_protected flag to the bss_conf structure Luca Coelho
2020-03-26 13:09 ` [PATCH v2 03/12] mac80211: Don't destroy auth data in case of anti-clogging Luca Coelho
2020-03-26 13:09 ` [PATCH v2 04/12] cfg80211: Parse HE membership selector Luca Coelho
2020-03-26 13:09 ` [PATCH v2 05/12] mac80211: Skip entries with " Luca Coelho
2020-03-26 13:09 ` [PATCH v2 06/12] mac80211: agg-tx: refactor sending addba Luca Coelho
2020-03-26 13:09 ` [PATCH v2 07/12] mac80211: agg-tx: add an option to defer ADDBA transmit Luca Coelho
2020-03-26 13:09 ` [PATCH v2 08/12] mac80211: Fail association when AP has no legacy rates Luca Coelho
2020-03-26 13:09 ` [PATCH v2 09/12] mac80211: minstrel_ht_assign_best_tp_rates: remove redundant test Luca Coelho
2020-03-26 13:09 ` [PATCH v2 10/12] mac80211_hwsim: indicate in IBSS that we have transmitted beacons Luca Coelho
2020-03-26 13:09 ` [PATCH v2 11/12] mac80211: drop data frames without key on encrypted links Luca Coelho
2020-03-27 15:03   ` Sasha Levin
2021-06-11 10:10     ` Pali Rohár
2021-06-22 23:15       ` Pali Rohár
2021-06-23 14:55         ` Greg KH
2021-06-23 12:16       ` Johannes Berg
2021-06-29 21:32         ` Pali Rohár
2021-06-30  6:49           ` Johannes Berg
2021-08-16 13:44     ` [PATCH] " Pali Rohár
2021-08-16 13:54       ` Greg KH
2020-03-26 13:09 ` [PATCH v2 12/12] cfg80211: Do not warn on same channel at the end of CSA Luca Coelho

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.