linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15
@ 2018-12-15  9:03 Luca Coelho
  2018-12-15  9:03 ` [PATCH 01/24] mac80211: suspicious RCU usage fix Luca Coelho
                   ` (24 more replies)
  0 siblings, 25 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luca Coelho

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

Hi,

Some patches with mac80211 and cfg80211 changes from our internal
tree.

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

Cheers,
Luca.


Andrei Otcheretianski (1):
  cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH

Emmanuel Grumbach (3):
  ieee80211: add bits for TWT in Extended Capabilities IE
  mac80211: propagate the support for TWT to the driver
  mac80211: ignore NullFunc frames in the duplicate detection

Ilan Peer (2):
  mac80211: Properly handle SKB with radiotap only
  mac80211: Properly access radiotap vendor data

Johannes Berg (9):
  mac80211: document RCU requirements for ieee80211_tx_dequeue()
  mac80211: remove superfluous NULL check
  mac80211: never pass NULL params to ieee80211_if_add()
  mac80211: fix radiotap vendor presence bitmap handling
  cfg80211: pmsr: fix MAC address setting
  cfg80211: fix ieee80211_get_vht_max_nss()
  nl80211: fix memory leak if validate_pae_over_nl80211() fails
  cfg80211: clarify LCI/civic location documentation
  mac80211: ftm responder: remove pointless defensive coding

Lior Cohen (1):
  mac80211: suspicious RCU usage fix

Luca Coelho (1):
  cfg80211: add some missing fall through annotations

Sara Sharon (3):
  mac80211: free skb fraglist before freeing the skb
  mac80211: don't build AMSDU from GSO packets
  mac80211: fix a kernel panic when TXing after TXQ teardown

Shaul Triebitz (4):
  mac80211: update HE operation fields to D3.0
  mac80211: update driver when MU EDCA params change
  mac80211: set STA flag DISABLE_HE if HE is not supported
  mac80211: do not advertise HE cap IE if HE disabled

 include/linux/ieee80211.h    | 30 +++++++++++---------
 include/net/cfg80211.h       | 12 ++++++--
 include/net/mac80211.h       | 11 ++++++++
 include/uapi/linux/nl80211.h | 20 ++++++++++----
 net/mac80211/cfg.c           |  4 +--
 net/mac80211/ieee80211_i.h   |  1 +
 net/mac80211/iface.c         | 13 +++++----
 net/mac80211/main.c          |  6 ++--
 net/mac80211/mlme.c          | 53 ++++++++++++++++++++++++++----------
 net/mac80211/rx.c            | 42 ++++++++++++++++++----------
 net/mac80211/status.c        |  5 ++++
 net/mac80211/tx.c            |  7 +++--
 net/mac80211/util.c          |  2 ++
 net/wireless/chan.c          |  3 ++
 net/wireless/core.c          |  2 ++
 net/wireless/nl80211.c       | 25 ++++++++++++++++-
 net/wireless/pmsr.c          | 25 +++++++++++------
 net/wireless/scan.c          |  2 +-
 net/wireless/util.c          | 15 +++++-----
 19 files changed, 198 insertions(+), 80 deletions(-)

-- 
2.19.2


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

* [PATCH 01/24] mac80211: suspicious RCU usage fix
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 02/24] ieee80211: add bits for TWT in Extended Capabilities IE Luca Coelho
                   ` (23 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Lior Cohen, Luca Coelho

From: Lior Cohen <lior2.cohen@intel.com>

Suspicious RCU usage observed while calling the
cfg80211_sta_opmode_change_notify() during rx_handlers path.
The issue occurred due to illegal blocking while in RCU read-side
critical section. The blocking caused by an attempt to alloc skb with the
GFP_KERNEL flag, which eventually call the might_sleep().

Signed-off-by: Lior Cohen <lior2.cohen@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/rx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 3bd3b5769797..a69ecfb212ed 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3063,7 +3063,7 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
 			cfg80211_sta_opmode_change_notify(sdata->dev,
 							  rx->sta->addr,
 							  &sta_opmode,
-							  GFP_KERNEL);
+							  GFP_ATOMIC);
 			goto handled;
 		}
 		case WLAN_HT_ACTION_NOTIFY_CHANWIDTH: {
@@ -3100,7 +3100,7 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
 			cfg80211_sta_opmode_change_notify(sdata->dev,
 							  rx->sta->addr,
 							  &sta_opmode,
-							  GFP_KERNEL);
+							  GFP_ATOMIC);
 			goto handled;
 		}
 		default:
-- 
2.19.2


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

* [PATCH 02/24] ieee80211: add bits for TWT in Extended Capabilities IE
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
  2018-12-15  9:03 ` [PATCH 01/24] mac80211: suspicious RCU usage fix Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 03/24] mac80211: propagate the support for TWT to the driver Luca Coelho
                   ` (22 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Emmanuel Grumbach, Luca Coelho

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

These bits are defined in ieee802.11ax to advertise support
for TWT in addition to the bits in the HE IE.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/linux/ieee80211.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 407d6fd66fa9..a9484b3e898d 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -2687,6 +2687,10 @@ enum ieee80211_tdls_actioncode {
  */
 #define WLAN_EXT_CAPA9_FTM_INITIATOR	BIT(7)
 
+/* Defines support for TWT Requester and TWT Responder */
+#define WLAN_EXT_CAPA10_TWT_REQUESTER_SUPPORT	BIT(5)
+#define WLAN_EXT_CAPA10_TWT_RESPONDER_SUPPORT	BIT(6)
+
 /* TDLS specific payload type in the LLC/SNAP header */
 #define WLAN_TDLS_SNAP_RFTYPE	0x2
 
-- 
2.19.2


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

* [PATCH 03/24] mac80211: propagate the support for TWT to the driver
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
  2018-12-15  9:03 ` [PATCH 01/24] mac80211: suspicious RCU usage fix Luca Coelho
  2018-12-15  9:03 ` [PATCH 02/24] ieee80211: add bits for TWT in Extended Capabilities IE Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 04/24] mac80211: update HE operation fields to D3.0 Luca Coelho
                   ` (21 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Emmanuel Grumbach, Luca Coelho

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

TWT is a feature that was added in 11ah and enhanced in
11ax. There are two bits that need to be set if we want
to use the feature in 11ax: one in the HE Capability IE
and one in the Extended Capability IE. This is because
of backward compatibility between 11ah and 11ax.

In order to simplify the flow for the low level driver
in managed mode, aggregate the two bits and add a boolean
that tells whether TWT is supported or not, but only if
11ax is supported.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/mac80211.h |  3 +++
 net/mac80211/mlme.c    | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 9386cf9fe714..1553152b77e0 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -496,6 +496,8 @@ struct ieee80211_ftm_responder_params {
  * @uora_ocw_range: UORA element's OCW Range field
  * @frame_time_rts_th: HE duration RTS threshold, in units of 32us
  * @he_support: does this BSS support HE
+ * @twt_requester: does this BSS support TWT requester (relevant for managed
+ *	mode only, set if the AP advertises TWT responder role)
  * @assoc: association status
  * @ibss_joined: indicates whether this station is part of an IBSS
  *	or not
@@ -594,6 +596,7 @@ struct ieee80211_bss_conf {
 	u8 uora_ocw_range;
 	u16 frame_time_rts_th;
 	bool he_support;
+	bool twt_requester;
 	/* association related data */
 	bool assoc, ibss_joined;
 	bool ibss_creator;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index d2bc8d57c87e..3d1334a4a264 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3058,6 +3058,19 @@ static void ieee80211_get_rates(struct ieee80211_supported_band *sband,
 	}
 }
 
+static bool ieee80211_twt_req_supported(const struct sta_info *sta,
+					const struct ieee802_11_elems *elems)
+{
+	if (elems->ext_capab_len < 10)
+		return false;
+
+	if (!(elems->ext_capab[9] & WLAN_EXT_CAPA10_TWT_RESPONDER_SUPPORT))
+		return false;
+
+	return sta->sta.he_cap.he_cap_elem.mac_cap_info[0] &
+		IEEE80211_HE_MAC_CAP0_TWT_RES;
+}
+
 static bool ieee80211_assoc_success(struct ieee80211_sub_if_data *sdata,
 				    struct cfg80211_bss *cbss,
 				    struct ieee80211_mgmt *mgmt, size_t len)
@@ -3247,8 +3260,11 @@ static bool ieee80211_assoc_success(struct ieee80211_sub_if_data *sdata,
 						  sta);
 
 		bss_conf->he_support = sta->sta.he_cap.has_he;
+		bss_conf->twt_requester =
+			ieee80211_twt_req_supported(sta, &elems);
 	} else {
 		bss_conf->he_support = false;
+		bss_conf->twt_requester = false;
 	}
 
 	if (bss_conf->he_support) {
-- 
2.19.2


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

* [PATCH 04/24] mac80211: update HE operation fields to D3.0
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (2 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 03/24] mac80211: propagate the support for TWT to the driver Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 05/24] mac80211: free skb fraglist before freeing the skb Luca Coelho
                   ` (20 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Shaul Triebitz, Luca Coelho

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

HE Operation element has changed in 11ax D3.0.  Update the fields
accordingly.

Signed-off-by: Shaul Triebitz <shaul.triebitz@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/linux/ieee80211.h | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index a9484b3e898d..3b04e72315e1 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1619,7 +1619,7 @@ struct ieee80211_he_mcs_nss_supp {
  * struct ieee80211_he_operation - HE capabilities element
  *
  * This structure is the "HE operation element" fields as
- * described in P802.11ax_D2.0 section 9.4.2.238
+ * described in P802.11ax_D3.0 section 9.4.2.238
  */
 struct ieee80211_he_operation {
 	__le32 he_oper_params;
@@ -2011,17 +2011,17 @@ ieee80211_he_ppe_size(u8 ppe_thres_hdr, const u8 *phy_cap_info)
 }
 
 /* HE Operation defines */
-#define IEEE80211_HE_OPERATION_BSS_COLOR_MASK			0x0000003f
-#define IEEE80211_HE_OPERATION_DFLT_PE_DURATION_MASK		0x000001c0
-#define IEEE80211_HE_OPERATION_DFLT_PE_DURATION_OFFSET		6
-#define IEEE80211_HE_OPERATION_TWT_REQUIRED			0x00000200
-#define IEEE80211_HE_OPERATION_RTS_THRESHOLD_MASK		0x000ffc00
-#define IEEE80211_HE_OPERATION_RTS_THRESHOLD_OFFSET		10
-#define IEEE80211_HE_OPERATION_PARTIAL_BSS_COLOR		0x00100000
-#define IEEE80211_HE_OPERATION_VHT_OPER_INFO			0x00200000
-#define IEEE80211_HE_OPERATION_MULTI_BSSID_AP			0x10000000
-#define IEEE80211_HE_OPERATION_TX_BSSID_INDICATOR		0x20000000
-#define IEEE80211_HE_OPERATION_BSS_COLOR_DISABLED		0x40000000
+#define IEEE80211_HE_OPERATION_DFLT_PE_DURATION_MASK		0x00000003
+#define IEEE80211_HE_OPERATION_TWT_REQUIRED			0x00000008
+#define IEEE80211_HE_OPERATION_RTS_THRESHOLD_MASK		0x00003ff0
+#define IEEE80211_HE_OPERATION_RTS_THRESHOLD_OFFSET		4
+#define IEEE80211_HE_OPERATION_VHT_OPER_INFO			0x00004000
+#define IEEE80211_HE_OPERATION_CO_LOCATED_BSS			0x00008000
+#define IEEE80211_HE_OPERATION_ER_SU_DISABLE			0x00010000
+#define IEEE80211_HE_OPERATION_BSS_COLOR_MASK			0x3f000000
+#define IEEE80211_HE_OPERATION_BSS_COLOR_OFFSET		24
+#define IEEE80211_HE_OPERATION_PARTIAL_BSS_COLOR		0x40000000
+#define IEEE80211_HE_OPERATION_BSS_COLOR_DISABLED		0x80000000
 
 /*
  * ieee80211_he_oper_size - calculate 802.11ax HE Operations IE size
@@ -2046,7 +2046,7 @@ ieee80211_he_oper_size(const u8 *he_oper_ie)
 	he_oper_params = le32_to_cpu(he_oper->he_oper_params);
 	if (he_oper_params & IEEE80211_HE_OPERATION_VHT_OPER_INFO)
 		oper_len += 3;
-	if (he_oper_params & IEEE80211_HE_OPERATION_MULTI_BSSID_AP)
+	if (he_oper_params & IEEE80211_HE_OPERATION_CO_LOCATED_BSS)
 		oper_len++;
 
 	/* Add the first byte (extension ID) to the total length */
-- 
2.19.2


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

* [PATCH 05/24] mac80211: free skb fraglist before freeing the skb
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (3 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 04/24] mac80211: update HE operation fields to D3.0 Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 06/24] mac80211: don't build AMSDU from GSO packets Luca Coelho
                   ` (19 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Sara Sharon, Luca Coelho

From: Sara Sharon <sara.sharon@intel.com>

mac80211 uses the frag list to build AMSDU. When freeing
the skb, it may not be really freed, since someone is still
holding a reference to it.
In that case, when TCP skb is being retransmitted, the
pointer to the frag list is being reused, while the data
in there is no longer valid.
Since we will never get frag list from the network stack,
as mac80211 doesn't advertise the capability, we can safely
free and nullify it before releasing the SKB.

Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/status.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index aa4afbf0abaf..267236297b2a 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -556,6 +556,11 @@ static void ieee80211_report_used_skb(struct ieee80211_local *local,
 	}
 
 	ieee80211_led_tx(local);
+
+	if (skb_has_frag_list(skb)) {
+		kfree_skb_list(skb_shinfo(skb)->frag_list);
+		skb_shinfo(skb)->frag_list = NULL;
+	}
 }
 
 /*
-- 
2.19.2


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

* [PATCH 06/24] mac80211: don't build AMSDU from GSO packets
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (4 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 05/24] mac80211: free skb fraglist before freeing the skb Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 07/24] mac80211: document RCU requirements for ieee80211_tx_dequeue() Luca Coelho
                   ` (18 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Sara Sharon, Luca Coelho

From: Sara Sharon <sara.sharon@intel.com>

If we build AMSDU from GSO packets, it can lead to
bad results if anyone tries to call skb_gso_segment
on the packets.

Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/tx.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 582b3d49f891..565fe7291c7d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3218,6 +3218,9 @@ static bool ieee80211_amsdu_aggregate(struct ieee80211_sub_if_data *sdata,
 	if (!ieee80211_hw_check(&local->hw, TX_AMSDU))
 		return false;
 
+	if (skb_is_gso(skb))
+		return false;
+
 	if (!txq)
 		return false;
 
@@ -3242,7 +3245,7 @@ static bool ieee80211_amsdu_aggregate(struct ieee80211_sub_if_data *sdata,
 	tin = &txqi->tin;
 	flow = fq_flow_classify(fq, tin, skb, fq_flow_get_default_func);
 	head = skb_peek_tail(&flow->queue);
-	if (!head)
+	if (!head || skb_is_gso(head))
 		goto out;
 
 	orig_len = head->len;
-- 
2.19.2


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

* [PATCH 07/24] mac80211: document RCU requirements for ieee80211_tx_dequeue()
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (5 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 06/24] mac80211: don't build AMSDU from GSO packets Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 08/24] mac80211: remove superfluous NULL check Luca Coelho
                   ` (17 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg, Luca Coelho

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

In the iwlwifi conversion, we sometimes call this from outside
of the wake_tx_queue() method, and in those cases must be in an
RCU critical section. Document this requirement.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/mac80211.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 1553152b77e0..40ad390f0f6a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6106,6 +6106,14 @@ void ieee80211_unreserve_tid(struct ieee80211_sta *sta, u8 tid);
  * @txq: pointer obtained from station or virtual interface
  *
  * Returns the skb if successful, %NULL if no frame was available.
+ *
+ * Note that this must be called in an rcu_read_lock() critical section,
+ * which can only be released after the SKB was handled. Some pointers in
+ * skb->cb, e.g. the key pointer, are protected by by RCU and thus the
+ * critical section must persist not just for the duration of this call
+ * but for the duration of the frame handling.
+ * However, also note that while in the wake_tx_queue() method,
+ * rcu_read_lock() is already held.
  */
 struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 				     struct ieee80211_txq *txq);
-- 
2.19.2


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

* [PATCH 08/24] mac80211: remove superfluous NULL check
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (6 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 07/24] mac80211: document RCU requirements for ieee80211_tx_dequeue() Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 09/24] mac80211: fix a kernel panic when TXing after TXQ teardown Luca Coelho
                   ` (16 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

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

At the place where this code lives now, the skb can never be
NULL, so we can remove the pointless NULL check.

It seems to exist because this code was moved around a few times
and originally came from a place where it could in fact be NULL.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 565fe7291c7d..4f348b116549 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3586,7 +3586,7 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 			skb_queue_splice_tail(&tx.skbs, &txqi->frags);
 	}
 
-	if (skb && skb_has_frag_list(skb) &&
+	if (skb_has_frag_list(skb) &&
 	    !ieee80211_hw_check(&local->hw, TX_FRAG_LIST)) {
 		if (skb_linearize(skb)) {
 			ieee80211_free_txskb(&local->hw, skb);
-- 
2.19.2


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

* [PATCH 09/24] mac80211: fix a kernel panic when TXing after TXQ teardown
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (7 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 08/24] mac80211: remove superfluous NULL check Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 10/24] mac80211: never pass NULL params to ieee80211_if_add() Luca Coelho
                   ` (15 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Sara Sharon, Luca Coelho

From: Sara Sharon <sara.sharon@intel.com>

Recently TXQ teardown was moved earlier in ieee80211_unregister_hw(),
to avoid a use-after-free of the netdev data. However, interfaces
aren't fully removed at the point, and cfg80211_shutdown_all_interfaces
can for example, TX a deauth frame. Move the TXQ teardown to the
point between cfg80211_shutdown_all_interfaces and the free of
netdev queues, so we can be sure they are torn down before netdev
is freed, but after there is no ongoing TX.

Fixes: 77cfaf52eca5 ("mac80211: Run TXQ teardown code before de-registering interfaces")
Signed-off-by: Sara Sharon <sara.sharon@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/iface.c | 3 +++
 net/mac80211/main.c  | 2 --
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 5836ddeac9e3..90ffdd9f756f 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -7,6 +7,7 @@
  * Copyright 2008, Johannes Berg <johannes@sipsolutions.net>
  * Copyright 2013-2014  Intel Mobile Communications GmbH
  * Copyright (c) 2016        Intel Deutschland GmbH
+ * Copyright (C) 2018 Intel Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -1949,6 +1950,8 @@ void ieee80211_remove_interfaces(struct ieee80211_local *local)
 	WARN(local->open_count, "%s: open count remains %d\n",
 	     wiphy_name(local->hw.wiphy), local->open_count);
 
+	ieee80211_txq_teardown_flows(local);
+
 	mutex_lock(&local->iflist_mtx);
 	list_for_each_entry_safe(sdata, tmp, &local->interfaces, list) {
 		list_del(&sdata->list);
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 83e71e6b2ebe..7b8320d4a8e4 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1262,7 +1262,6 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 	rtnl_unlock();
 	ieee80211_led_exit(local);
 	ieee80211_wep_free(local);
-	ieee80211_txq_teardown_flows(local);
  fail_flows:
 	destroy_workqueue(local->workqueue);
  fail_workqueue:
@@ -1288,7 +1287,6 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
 #if IS_ENABLED(CONFIG_IPV6)
 	unregister_inet6addr_notifier(&local->ifa6_notifier);
 #endif
-	ieee80211_txq_teardown_flows(local);
 
 	rtnl_lock();
 
-- 
2.19.2


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

* [PATCH 10/24] mac80211: never pass NULL params to ieee80211_if_add()
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (8 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 09/24] mac80211: fix a kernel panic when TXing after TXQ teardown Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 11/24] mac80211: fix radiotap vendor presence bitmap handling Luca Coelho
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg, Luca Coelho

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

This isn't really a problem now, but it means that the function
has a few NULL checks that are only relevant when coming from
the initial interface added in mac80211, and that's confusing.
Just pass non-NULL (but equivalently empty) in that case and
remove all the NULL checks.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/iface.c | 10 ++++------
 net/mac80211/main.c  |  4 +++-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 90ffdd9f756f..a9e08e200780 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1800,7 +1800,7 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 		}
 
 		ieee80211_assign_perm_addr(local, ndev->perm_addr, type);
-		if (params && is_valid_ether_addr(params->macaddr))
+		if (is_valid_ether_addr(params->macaddr))
 			memcpy(ndev->dev_addr, params->macaddr, ETH_ALEN);
 		else
 			memcpy(ndev->dev_addr, ndev->perm_addr, ETH_ALEN);
@@ -1869,11 +1869,9 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 	ieee80211_setup_sdata(sdata, type);
 
 	if (ndev) {
-		if (params) {
-			ndev->ieee80211_ptr->use_4addr = params->use_4addr;
-			if (type == NL80211_IFTYPE_STATION)
-				sdata->u.mgd.use_4addr = params->use_4addr;
-		}
+		ndev->ieee80211_ptr->use_4addr = params->use_4addr;
+		if (type == NL80211_IFTYPE_STATION)
+			sdata->u.mgd.use_4addr = params->use_4addr;
 
 		ndev->features |= local->hw.netdev_features;
 
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 7b8320d4a8e4..87a729926734 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1221,8 +1221,10 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 	/* add one default STA interface if supported */
 	if (local->hw.wiphy->interface_modes & BIT(NL80211_IFTYPE_STATION) &&
 	    !ieee80211_hw_check(hw, NO_AUTO_VIF)) {
+		struct vif_params params = {0};
+
 		result = ieee80211_if_add(local, "wlan%d", NET_NAME_ENUM, NULL,
-					  NL80211_IFTYPE_STATION, NULL);
+					  NL80211_IFTYPE_STATION, &params);
 		if (result)
 			wiphy_warn(local->hw.wiphy,
 				   "Failed to add default virtual iface\n");
-- 
2.19.2


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

* [PATCH 11/24] mac80211: fix radiotap vendor presence bitmap handling
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (9 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 10/24] mac80211: never pass NULL params to ieee80211_if_add() Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection Luca Coelho
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg, Luca Coelho

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

Due to the alignment handling, it actually matters where in the code
we add the 4 bytes for the presence bitmap to the length; the first
field is the timestamp with 8 byte alignment so we need to add the
space for the extra vendor namespace presence bitmap *before* we do
any alignment for the fields.

Move the presence bitmap length accounting to the right place to fix
the alignment for the data properly.

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

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index a69ecfb212ed..2394008f82b9 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -143,6 +143,9 @@ ieee80211_rx_radiotap_hdrlen(struct ieee80211_local *local,
 	/* allocate extra bitmaps */
 	if (status->chains)
 		len += 4 * hweight8(status->chains);
+	/* vendor presence bitmap */
+	if (status->flag & RX_FLAG_RADIOTAP_VENDOR_DATA)
+		len += 4;
 
 	if (ieee80211_have_rx_timestamp(status)) {
 		len = ALIGN(len, 8);
@@ -207,8 +210,6 @@ ieee80211_rx_radiotap_hdrlen(struct ieee80211_local *local,
 	if (status->flag & RX_FLAG_RADIOTAP_VENDOR_DATA) {
 		struct ieee80211_vendor_radiotap *rtap = (void *)skb->data;
 
-		/* vendor presence bitmap */
-		len += 4;
 		/* alignment for fixed 6-byte vendor data header */
 		len = ALIGN(len, 2);
 		/* vendor data header */
-- 
2.19.2


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

* [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (10 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 11/24] mac80211: fix radiotap vendor presence bitmap handling Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15 17:31   ` Emmanuel Grumbach
  2018-12-15  9:03 ` [PATCH 13/24] cfg80211: pmsr: fix MAC address setting Luca Coelho
                   ` (12 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Emmanuel Grumbach, stable, Luca Coelho

From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>

NullFunc packets should never be duplicate just like
QoS-NullFunc packets.

We saw a client that enters / exits power save with
NullFunc frames (and not with QoS-NullFunc) despite the
fact that the association supports HT.
This specific client also re-uses a non-zero sequence number
for different NullFunc frames.
At some point, the client had to send a retransmission of
the NullFunc frame and we dropped it, leading to a
misalignment in the power save state.
Fix this by never consider a NullFunc frame as duplicate,
just like we do for QoS NullFunc frames.

This fixes https://bugzilla.kernel.org/show_bug.cgi?id=201449

CC: <stable@vger.kernel.org>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/rx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 2394008f82b9..60d179bf2585 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1404,6 +1404,7 @@ ieee80211_rx_h_check_dup(struct ieee80211_rx_data *rx)
 		return RX_CONTINUE;
 
 	if (ieee80211_is_ctl(hdr->frame_control) ||
+	    ieee80211_is_nullfunc(hdr->frame_control) ||
 	    ieee80211_is_qos_nullfunc(hdr->frame_control) ||
 	    is_multicast_ether_addr(hdr->addr1))
 		return RX_CONTINUE;
-- 
2.19.2


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

* [PATCH 13/24] cfg80211: pmsr: fix MAC address setting
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (11 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-18 12:17   ` Johannes Berg
  2018-12-15  9:03 ` [PATCH 14/24] mac80211: update driver when MU EDCA params change Luca Coelho
                   ` (11 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

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

When we *don't* have a MAC address attribute, we shouldn't
try to use this - this was intended to copy the local MAC
address instead, so fix it.

Fixes: 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM initiator API")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/core.c |  2 ++
 net/wireless/pmsr.c | 25 ++++++++++++++++---------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 623dfe5e211c..b36ad8efb5e5 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1068,6 +1068,8 @@ static void __cfg80211_unregister_wdev(struct wireless_dev *wdev, bool sync)
 
 	ASSERT_RTNL();
 
+	flush_work(&wdev->pmsr_free_wk);
+
 	nl80211_notify_iface(rdev, wdev, NL80211_CMD_DEL_INTERFACE);
 
 	list_del_rcu(&wdev->list);
diff --git a/net/wireless/pmsr.c b/net/wireless/pmsr.c
index de9286703280..78c3f5633692 100644
--- a/net/wireless/pmsr.c
+++ b/net/wireless/pmsr.c
@@ -256,8 +256,7 @@ int nl80211_pmsr_start(struct sk_buff *skb, struct genl_info *info)
 		if (err)
 			goto out_err;
 	} else {
-		memcpy(req->mac_addr, nla_data(info->attrs[NL80211_ATTR_MAC]),
-		       ETH_ALEN);
+		memcpy(req->mac_addr, wdev_address(wdev), ETH_ALEN);
 		memset(req->mac_addr_mask, 0xff, ETH_ALEN);
 	}
 
@@ -530,14 +529,14 @@ void cfg80211_pmsr_report(struct wireless_dev *wdev,
 }
 EXPORT_SYMBOL_GPL(cfg80211_pmsr_report);
 
-void cfg80211_pmsr_free_wk(struct work_struct *work)
+static void cfg80211_pmsr_process_abort(struct wireless_dev *wdev)
 {
-	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
-						 pmsr_free_wk);
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
 	struct cfg80211_pmsr_request *req, *tmp;
 	LIST_HEAD(free_list);
 
+	lockdep_assert_held(&wdev->mtx);
+
 	spin_lock_bh(&wdev->pmsr_lock);
 	list_for_each_entry_safe(req, tmp, &wdev->pmsr_list, list) {
 		if (req->nl_portid)
@@ -547,14 +546,22 @@ void cfg80211_pmsr_free_wk(struct work_struct *work)
 	spin_unlock_bh(&wdev->pmsr_lock);
 
 	list_for_each_entry_safe(req, tmp, &free_list, list) {
-		wdev_lock(wdev);
 		rdev_abort_pmsr(rdev, wdev, req);
-		wdev_unlock(wdev);
 
 		kfree(req);
 	}
 }
 
+void cfg80211_pmsr_free_wk(struct work_struct *work)
+{
+	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
+						 pmsr_free_wk);
+
+	wdev_lock(wdev);
+	cfg80211_pmsr_process_abort(wdev);
+	wdev_unlock(wdev);
+}
+
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev)
 {
 	struct cfg80211_pmsr_request *req;
@@ -568,8 +575,8 @@ void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev)
 	spin_unlock_bh(&wdev->pmsr_lock);
 
 	if (found)
-		schedule_work(&wdev->pmsr_free_wk);
-	flush_work(&wdev->pmsr_free_wk);
+		cfg80211_pmsr_process_abort(wdev);
+
 	WARN_ON(!list_empty(&wdev->pmsr_list));
 }
 
-- 
2.19.2


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

* [PATCH 14/24] mac80211: update driver when MU EDCA params change
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (12 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 13/24] cfg80211: pmsr: fix MAC address setting Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 15/24] cfg80211: fix ieee80211_get_vht_max_nss() Luca Coelho
                   ` (10 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Shaul Triebitz, Luca Coelho

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

Similar to WMM IE, if MU_EDCA IE parameters changed (or ceased to exist)
tell the Driver about it.

Signed-off-by: Shaul Triebitz <shaul.triebitz@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/ieee80211_i.h |  1 +
 net/mac80211/mlme.c        | 12 ++++++++++--
 net/mac80211/util.c        |  2 ++
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 10a05062e4a0..7dfb4e2f98b2 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -500,6 +500,7 @@ struct ieee80211_if_managed {
 	unsigned int uapsd_max_sp_len;
 
 	int wmm_last_param_set;
+	int mu_edca_last_param_set;
 
 	u8 use_4addr;
 
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 3d1334a4a264..975315b5689a 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1869,7 +1869,7 @@ ieee80211_sta_wmm_params(struct ieee80211_local *local,
 	struct ieee80211_tx_queue_params params[IEEE80211_NUM_ACS];
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	size_t left;
-	int count, ac;
+	int count, mu_edca_count, ac;
 	const u8 *pos;
 	u8 uapsd_queues = 0;
 
@@ -1889,9 +1889,16 @@ ieee80211_sta_wmm_params(struct ieee80211_local *local,
 		uapsd_queues = ifmgd->uapsd_queues;
 
 	count = wmm_param[6] & 0x0f;
-	if (count == ifmgd->wmm_last_param_set)
+	/* -1 is the initial value of ifmgd->mu_edca_last_param_set.
+	 * if mu_edca was preset before and now it disappeared tell
+	 * the driver about it.
+	 */
+	mu_edca_count = mu_edca ? mu_edca->mu_qos_info & 0x0f : -1;
+	if (count == ifmgd->wmm_last_param_set &&
+	    mu_edca_count == ifmgd->mu_edca_last_param_set)
 		return false;
 	ifmgd->wmm_last_param_set = count;
+	ifmgd->mu_edca_last_param_set = mu_edca_count;
 
 	pos = wmm_param + 8;
 	left = wmm_param_len - 8;
@@ -3349,6 +3356,7 @@ static bool ieee80211_assoc_success(struct ieee80211_sub_if_data *sdata,
 	 * 4-bit value.
 	 */
 	ifmgd->wmm_last_param_set = -1;
+	ifmgd->mu_edca_last_param_set = -1;
 
 	if (ifmgd->flags & IEEE80211_STA_DISABLE_WMM) {
 		ieee80211_set_wmm_default(sdata, false, false);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index dddfff7cf44f..d0eb38b890aa 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1223,6 +1223,8 @@ u32 ieee802_11_parse_elems_crc(const u8 *start, size_t len, bool action,
 			if (pos[0] == WLAN_EID_EXT_HE_MU_EDCA &&
 			    elen >= (sizeof(*elems->mu_edca_param_set) + 1)) {
 				elems->mu_edca_param_set = (void *)&pos[1];
+				if (calc_crc)
+					crc = crc32_be(crc, pos - 2, elen + 2);
 			} else if (pos[0] == WLAN_EID_EXT_HE_CAPABILITY) {
 				elems->he_cap = (void *)&pos[1];
 				elems->he_cap_len = elen - 1;
-- 
2.19.2


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

* [PATCH 15/24] cfg80211: fix ieee80211_get_vht_max_nss()
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (13 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 14/24] mac80211: update driver when MU EDCA params change Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 16/24] mac80211: Properly handle SKB with radiotap only Luca Coelho
                   ` (9 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg, Luca Coelho

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

Fix two bugs in ieee80211_get_vht_max_nss():
 * the spec says we should round down
   (reported by Nissim)
 * there's a double condition, the first one is wrong,
   supp_width == 0 / ext_nss_bw == 2 is valid in 80+80
   (found by smatch)

Fixes: b0aa75f0b1b2 ("ieee80211: add new VHT capability fields/parsing")
Reported-by: Nissim Bendanan <nissimx.bendanan@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/util.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/net/wireless/util.c b/net/wireless/util.c
index ef14d80ca03e..06a084236841 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -2013,33 +2013,32 @@ int ieee80211_get_vht_max_nss(struct ieee80211_vht_cap *cap,
 	case IEEE80211_VHT_CHANWIDTH_160MHZ:
 		if (supp_width == 0 &&
 		    (ext_nss_bw == 1 || ext_nss_bw == 2))
-			return DIV_ROUND_UP(max_vht_nss, 2);
+			return max_vht_nss / 2;
 		if (supp_width == 0 &&
 		    ext_nss_bw == 3)
-			return DIV_ROUND_UP(3 * max_vht_nss, 4);
+			return (3 * max_vht_nss) / 4;
 		if (supp_width == 1 &&
 		    ext_nss_bw == 3)
 			return 2 * max_vht_nss;
 		break;
 	case IEEE80211_VHT_CHANWIDTH_80P80MHZ:
-		if (supp_width == 0 &&
-		    (ext_nss_bw == 1 || ext_nss_bw == 2))
+		if (supp_width == 0 && ext_nss_bw == 1)
 			return 0; /* not possible */
 		if (supp_width == 0 &&
 		    ext_nss_bw == 2)
-			return DIV_ROUND_UP(max_vht_nss, 2);
+			return max_vht_nss / 2;
 		if (supp_width == 0 &&
 		    ext_nss_bw == 3)
-			return DIV_ROUND_UP(3 * max_vht_nss, 4);
+			return (3 * max_vht_nss) / 4;
 		if (supp_width == 1 &&
 		    ext_nss_bw == 0)
 			return 0; /* not possible */
 		if (supp_width == 1 &&
 		    ext_nss_bw == 1)
-			return DIV_ROUND_UP(max_vht_nss, 2);
+			return max_vht_nss / 2;
 		if (supp_width == 1 &&
 		    ext_nss_bw == 2)
-			return DIV_ROUND_UP(3 * max_vht_nss, 4);
+			return (3 * max_vht_nss) / 4;
 		break;
 	}
 
-- 
2.19.2


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

* [PATCH 16/24] mac80211: Properly handle SKB with radiotap only
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (14 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 15/24] cfg80211: fix ieee80211_get_vht_max_nss() Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 17/24] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH Luca Coelho
                   ` (8 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Ilan Peer, Luca Coelho

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

The monitor interface Rx handling of SKBs that contain only
radiotap information was buggy as it tried to access the
SKB assuming it contains a frame.

To fix this, check the RX_FLAG_NO_PSDU flag in the Rx status
(indicting that the SKB contains only radiotap information),
and do not perform data path specific processing when the flag
is set.

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

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 60d179bf2585..85c365fc7a0c 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -754,6 +754,7 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
 	struct ieee80211_sub_if_data *monitor_sdata =
 		rcu_dereference(local->monitor_sdata);
 	bool only_monitor = false;
+	unsigned int min_head_len;
 
 	if (status->flag & RX_FLAG_RADIOTAP_HE)
 		rtap_space += sizeof(struct ieee80211_radiotap_he);
@@ -767,6 +768,8 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
 		rtap_space += sizeof(*rtap) + rtap->len + rtap->pad;
 	}
 
+	min_head_len = rtap_space;
+
 	/*
 	 * First, we may need to make a copy of the skb because
 	 *  (1) we need to modify it for radiotap (if not present), and
@@ -776,18 +779,23 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
 	 * the SKB because it has a bad FCS/PLCP checksum.
 	 */
 
-	if (ieee80211_hw_check(&local->hw, RX_INCLUDES_FCS)) {
-		if (unlikely(origskb->len <= FCS_LEN)) {
-			/* driver bug */
-			WARN_ON(1);
-			dev_kfree_skb(origskb);
-			return NULL;
+	if (!(status->flag & RX_FLAG_NO_PSDU)) {
+		if (ieee80211_hw_check(&local->hw, RX_INCLUDES_FCS)) {
+			if (unlikely(origskb->len <= FCS_LEN + rtap_space)) {
+				/* driver bug */
+				WARN_ON(1);
+				dev_kfree_skb(origskb);
+				return NULL;
+			}
+			present_fcs_len = FCS_LEN;
 		}
-		present_fcs_len = FCS_LEN;
+
+		/* also consider the hdr->frame_control */
+		min_head_len += 2;
 	}
 
-	/* ensure hdr->frame_control and vendor radiotap data are in skb head */
-	if (!pskb_may_pull(origskb, 2 + rtap_space)) {
+	/* ensure that the expected data elements are in skb head */
+	if (!pskb_may_pull(origskb, min_head_len)) {
 		dev_kfree_skb(origskb);
 		return NULL;
 	}
-- 
2.19.2


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

* [PATCH 17/24] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (15 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 16/24] mac80211: Properly handle SKB with radiotap only Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2019-01-25 12:41   ` Johannes Berg
  2018-12-15  9:03 ` [PATCH 18/24] mac80211: set STA flag DISABLE_HE if HE is not supported Luca Coelho
                   ` (7 subsequent siblings)
  24 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Andrei Otcheretianski, Luca Coelho

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

This is needed for the devices that manage PMKSA caching internally and
don't implement SET/DEL PMKSA commands.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/cfg80211.h       |  6 ++++++
 include/uapi/linux/nl80211.h |  4 +++-
 net/wireless/nl80211.c       | 12 ++++++++++++
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ede7fcd68348..30618afab657 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2813,6 +2813,9 @@ struct cfg80211_pmk_conf {
  *	use %WLAN_STATUS_UNSPECIFIED_FAILURE if user space cannot give you
  *	the real status code for failures. Used only for the authentication
  *	response command interface (user space to driver).
+ * @pmk_len: Length of PMK if present.
+ * @pmk: Derived PMK
+ * @pmkid: PMKID of the derived PMK
  */
 struct cfg80211_external_auth_params {
 	enum nl80211_external_auth_action action;
@@ -2820,6 +2823,9 @@ struct cfg80211_external_auth_params {
 	struct cfg80211_ssid ssid;
 	unsigned int key_mgmt_suite;
 	u16 status;
+	int pmk_len;
+	const u8 *pmk;
+	const u8 *pmkid;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 2b53c0e949c7..3843214ec7ee 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1022,7 +1022,9 @@
  *	further with the association after getting successful authentication
  *	status. User space indicates the authentication status through
  *	%NL80211_ATTR_STATUS_CODE attribute in %NL80211_CMD_EXTERNAL_AUTH
- *	command interface.
+ *	command interface. In case of success, user space also includes the
+ *	derived PMK and PMKID through %NL80211_ATTR_PMK and
+ *	%NL80211_ATTR_PMKID.
  *
  *	Host driver reports this status on an authentication failure to the
  *	user space through the connect result as the user space would have
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e20329b34840..323cd91cf1e4 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12990,6 +12990,12 @@ static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info)
 	if (!info->attrs[NL80211_ATTR_STATUS_CODE])
 		return -EINVAL;
 
+	if ((info->attrs[NL80211_ATTR_PMK] &&
+	     !info->attrs[NL80211_ATTR_PMKID]) ||
+	    (info->attrs[NL80211_ATTR_PMKID] &&
+	     !info->attrs[NL80211_ATTR_PMK]))
+		return -EINVAL;
+
 	memset(&params, 0, sizeof(params));
 
 	params.ssid.ssid_len = nla_len(info->attrs[NL80211_ATTR_SSID]);
@@ -13004,6 +13010,12 @@ static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info)
 
 	params.status = nla_get_u16(info->attrs[NL80211_ATTR_STATUS_CODE]);
 
+	if (info->attrs[NL80211_ATTR_PMK] && info->attrs[NL80211_ATTR_PMKID]) {
+		params.pmk_len = nla_len(info->attrs[NL80211_ATTR_PMK]);
+		params.pmk = nla_data(info->attrs[NL80211_ATTR_PMK]);
+		params.pmkid = nla_data(info->attrs[NL80211_ATTR_PMKID]);
+	}
+
 	return rdev_external_auth(rdev, dev, &params);
 }
 
-- 
2.19.2


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

* [PATCH 18/24] mac80211: set STA flag DISABLE_HE if HE is not supported
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (16 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 17/24] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 19/24] mac80211: do not advertise HE cap IE if HE disabled Luca Coelho
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Shaul Triebitz, Luca Coelho

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

Up until now, the IEEE80211_STA_DISABLE_HE flag was set only based
on whether the AP has advertised HE capabilities.
This flag should be set also if STA does not support HE
(regardless of the AP support).

Signed-off-by: Shaul Triebitz <shaul.triebitz@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/mlme.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 975315b5689a..8c6a4dc017a5 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -4680,8 +4680,10 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 		}
 	}
 
-	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE) &&
-	    ieee80211_get_he_sta_cap(sband)) {
+	if (!ieee80211_get_he_sta_cap(sband))
+		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
+
+	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE)) {
 		const struct cfg80211_bss_ies *ies;
 		const u8 *he_oper_ie;
 
-- 
2.19.2


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

* [PATCH 19/24] mac80211: do not advertise HE cap IE if HE disabled
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (17 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 18/24] mac80211: set STA flag DISABLE_HE if HE is not supported Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 20/24] cfg80211: add some missing fall through annotations Luca Coelho
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Shaul Triebitz, Luca Coelho

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

When disabling HE due to the lack of HT/VHT, do it
at an earlier stage to avoid advertising HE capabilities IE.
Also, at this point, no need to check if AP supports HE, since
it is already checked earlier (in ieee80211_prep_channel).

Signed-off-by: Shaul Triebitz <shaul.triebitz@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/mlme.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 8c6a4dc017a5..54e511dbbc12 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -916,6 +916,15 @@ static void ieee80211_send_assoc(struct ieee80211_sub_if_data *sdata)
 		ieee80211_add_vht_ie(sdata, skb, sband,
 				     &assoc_data->ap_vht_cap);
 
+	/*
+	 * If AP doesn't support HT, mark HE as disabled.
+	 * If on the 5GHz band, make sure it supports VHT.
+	 */
+	if (ifmgd->flags & IEEE80211_STA_DISABLE_HT ||
+	    (sband->band == NL80211_BAND_5GHZ &&
+	     ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
+		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
+
 	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE))
 		ieee80211_add_he_ie(sdata, skb, sband);
 
@@ -3231,16 +3240,6 @@ static bool ieee80211_assoc_success(struct ieee80211_sub_if_data *sdata,
 		goto out;
 	}
 
-	/*
-	 * If AP doesn't support HT, or it doesn't have HE mandatory IEs, mark
-	 * HE as disabled. If on the 5GHz band, make sure it supports VHT.
-	 */
-	if (ifmgd->flags & IEEE80211_STA_DISABLE_HT ||
-	    (sband->band == NL80211_BAND_5GHZ &&
-	     ifmgd->flags & IEEE80211_STA_DISABLE_VHT) ||
-	    (!elems.he_cap && !elems.he_operation))
-		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
-
 	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE) &&
 	    (!elems.he_cap || !elems.he_operation)) {
 		mutex_unlock(&sdata->local->sta_mtx);
-- 
2.19.2


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

* [PATCH 20/24] cfg80211: add some missing fall through annotations
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (18 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 19/24] mac80211: do not advertise HE cap IE if HE disabled Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 21/24] nl80211: fix memory leak if validate_pae_over_nl80211() fails Luca Coelho
                   ` (4 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Luca Coelho

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

There are talks about enabling -Wimplicit-fallthrough warnings in the
mainline and it is already enabled in linux-next.  Add all the
missing annotations to prevent warnings when this happens.

And in one case, remove the extra text from the annotation so that the
compiler recognizes it.

Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/chan.c    | 3 +++
 net/wireless/nl80211.c | 9 +++++++++
 net/wireless/scan.c    | 2 +-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 2db713d18f71..7dc1bbd0888f 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -6,6 +6,7 @@
  *
  * Copyright 2009	Johannes Berg <johannes@sipsolutions.net>
  * Copyright 2013-2014  Intel Mobile Communications GmbH
+ * Copyright 2018       Intel Corporation
  */
 
 #include <linux/export.h>
@@ -747,6 +748,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
 	case NL80211_CHAN_WIDTH_20:
 		if (!ht_cap->ht_supported)
 			return false;
+		/* fall through */
 	case NL80211_CHAN_WIDTH_20_NOHT:
 		prohibited_flags |= IEEE80211_CHAN_NO_20MHZ;
 		width = 20;
@@ -769,6 +771,7 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
 		cap = vht_cap->cap & IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_MASK;
 		if (cap != IEEE80211_VHT_CAP_SUPP_CHAN_WIDTH_160_80PLUS80MHZ)
 			return false;
+		/* fall through */
 	case NL80211_CHAN_WIDTH_80:
 		if (!vht_cap->vht_supported)
 			return false;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 323cd91cf1e4..669cbdb6f849 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1851,6 +1851,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* fall through */
 	case 1:
 		if (nla_put(msg, NL80211_ATTR_CIPHER_SUITES,
 			    sizeof(u32) * rdev->wiphy.n_cipher_suites,
@@ -1897,6 +1898,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* fall through */
 	case 2:
 		if (nl80211_put_iftypes(msg, NL80211_ATTR_SUPPORTED_IFTYPES,
 					rdev->wiphy.interface_modes))
@@ -1904,6 +1906,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* fall through */
 	case 3:
 		nl_bands = nla_nest_start(msg, NL80211_ATTR_WIPHY_BANDS);
 		if (!nl_bands)
@@ -1929,6 +1932,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 				state->chan_start++;
 				if (state->split)
 					break;
+				/* fall through */
 			default:
 				/* add frequencies */
 				nl_freqs = nla_nest_start(
@@ -1982,6 +1986,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 			state->split_start++;
 		if (state->split)
 			break;
+		/* fall through */
 	case 4:
 		nl_cmds = nla_nest_start(msg, NL80211_ATTR_SUPPORTED_COMMANDS);
 		if (!nl_cmds)
@@ -2008,6 +2013,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* fall through */
 	case 5:
 		if (rdev->ops->remain_on_channel &&
 		    (rdev->wiphy.flags & WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL) &&
@@ -2025,6 +2031,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* fall through */
 	case 6:
 #ifdef CONFIG_PM
 		if (nl80211_send_wowlan(msg, rdev, state->split))
@@ -2035,6 +2042,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 #else
 		state->split_start++;
 #endif
+		/* fall through */
 	case 7:
 		if (nl80211_put_iftypes(msg, NL80211_ATTR_SOFTWARE_IFTYPES,
 					rdev->wiphy.software_iftypes))
@@ -2047,6 +2055,7 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 		state->split_start++;
 		if (state->split)
 			break;
+		/* fall through */
 	case 8:
 		if ((rdev->wiphy.flags & WIPHY_FLAG_HAVE_AP_SME) &&
 		    nla_put_u32(msg, NL80211_ATTR_DEVICE_AP_SME,
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index d0e7472dd9fd..5123667f4569 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1183,7 +1183,7 @@ cfg80211_inform_bss_data(struct wiphy *wiphy,
 	switch (ftype) {
 	case CFG80211_BSS_FTYPE_BEACON:
 		ies->from_beacon = true;
-		/* fall through to assign */
+		/* fall through */
 	case CFG80211_BSS_FTYPE_UNKNOWN:
 		rcu_assign_pointer(tmp.pub.beacon_ies, ies);
 		break;
-- 
2.19.2


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

* [PATCH 21/24] nl80211: fix memory leak if validate_pae_over_nl80211() fails
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (19 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 20/24] cfg80211: add some missing fall through annotations Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 22/24] cfg80211: clarify LCI/civic location documentation Luca Coelho
                   ` (3 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

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

If validate_pae_over_nl80211() were to fail in nl80211_crypto_settings(),
we might leak the 'connkeys' allocation. Fix this.

Fixes: 64bf3d4bc2b0 ("nl80211: Add CONTROL_PORT_OVER_NL80211 attribute")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/nl80211.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 669cbdb6f849..449a379e054a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -9097,8 +9097,10 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
 	if (info->attrs[NL80211_ATTR_CONTROL_PORT_OVER_NL80211]) {
 		int r = validate_pae_over_nl80211(rdev, info);
 
-		if (r < 0)
+		if (r < 0) {
+			kzfree(connkeys);
 			return r;
+		}
 
 		ibss.control_port_over_nl80211 = true;
 	}
-- 
2.19.2


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

* [PATCH 22/24] cfg80211: clarify LCI/civic location documentation
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (20 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 21/24] nl80211: fix memory leak if validate_pae_over_nl80211() fails Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 23/24] mac80211: ftm responder: remove pointless defensive coding Luca Coelho
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

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

The older code and current userspace assumed that this data
is the content of the Measurement Report element, starting
with the Measurement Token. Clarify this in the documentation.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h       |  6 ++++--
 include/uapi/linux/nl80211.h | 16 ++++++++++++----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 30618afab657..36f829b75428 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -777,8 +777,10 @@ struct cfg80211_crypto_settings {
  * @probe_resp: probe response template (AP mode only)
  * @ftm_responder: enable FTM responder functionality; -1 for no change
  *	(which also implies no change in LCI/civic location data)
- * @lci: LCI subelement content
- * @civicloc: Civic location subelement content
+ * @lci: Measurement Report element content, starting with Measurement Token
+ *	(measurement type 8)
+ * @civicloc: Measurement Report element content, starting with Measurement
+ *	Token (measurement type 11)
  * @lci_len: LCI data length
  * @civicloc_len: Civic location data length
  */
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 3843214ec7ee..81a1e83c589c 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -5895,9 +5895,11 @@ enum nl80211_external_auth_action {
  * @__NL80211_FTM_RESP_ATTR_INVALID: Invalid
  * @NL80211_FTM_RESP_ATTR_ENABLED: FTM responder is enabled
  * @NL80211_FTM_RESP_ATTR_LCI: The content of Measurement Report Element
- *	(9.4.2.22 in 802.11-2016) with type 8 - LCI (9.4.2.22.10)
+ *	(9.4.2.22 in 802.11-2016) with type 8 - LCI (9.4.2.22.10),
+ *	i.e. starting with the measurement token
  * @NL80211_FTM_RESP_ATTR_CIVIC: The content of Measurement Report Element
- *	(9.4.2.22 in 802.11-2016) with type 11 - Civic (Section 9.4.2.22.13)
+ *	(9.4.2.22 in 802.11-2016) with type 11 - Civic (Section 9.4.2.22.13),
+ *	i.e. starting with the measurement token
  * @__NL80211_FTM_RESP_ATTR_LAST: Internal
  * @NL80211_FTM_RESP_ATTR_MAX: highest FTM responder attribute.
  */
@@ -6297,9 +6299,15 @@ enum nl80211_peer_measurement_ftm_failure_reasons {
  * @NL80211_PMSR_FTM_RESP_ATTR_DIST_VARIANCE: distance variance (u64, mm^2, note
  *	that standard deviation is the square root of variance, optional)
  * @NL80211_PMSR_FTM_RESP_ATTR_DIST_SPREAD: distance spread (u64, mm, optional)
- * @NL80211_PMSR_FTM_RESP_ATTR_LCI: LCI data from peer (binary, optional)
+ * @NL80211_PMSR_FTM_RESP_ATTR_LCI: LCI data from peer (binary, optional);
+ *	this is the contents of the Measurement Report Element (802.11-2016
+ *	9.4.2.22.1) starting with the Measurement Token, with Measurement
+ *	Type 8.
  * @NL80211_PMSR_FTM_RESP_ATTR_CIVICLOC: civic location data from peer
- *	(binary, optional)
+ *	(binary, optional);
+ *	this is the contents of the Measurement Report Element (802.11-2016
+ *	9.4.2.22.1) starting with the Measurement Token, with Measurement
+ *	Type 11.
  * @NL80211_PMSR_FTM_RESP_ATTR_PAD: ignore, for u64/s64 padding only
  *
  * @NUM_NL80211_PMSR_FTM_RESP_ATTR: internal
-- 
2.19.2


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

* [PATCH 23/24] mac80211: ftm responder: remove pointless defensive coding
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (21 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 22/24] cfg80211: clarify LCI/civic location documentation Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-15  9:03 ` [PATCH 24/24] mac80211: Properly access radiotap vendor data Luca Coelho
  2018-12-18 12:06 ` [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Johannes Berg
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

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

The pointer and corresponding length is always set in pairs
in cfg80211, so no need to have this strange defensive check
that also confuses static checkers. Clean it up.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index cf8f946ae724..567c63ff830f 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -800,8 +800,8 @@ static int ieee80211_set_ftm_responder_params(
 	u8 *pos;
 	int len;
 
-	if ((!lci || !lci_len) && (!civicloc || !civicloc_len))
-		return 1;
+	if (!lci_len && !civicloc_len)
+		return 0;
 
 	bss_conf = &sdata->vif.bss_conf;
 	old = bss_conf->ftmr_params;
-- 
2.19.2


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

* [PATCH 24/24] mac80211: Properly access radiotap vendor data
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (22 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 23/24] mac80211: ftm responder: remove pointless defensive coding Luca Coelho
@ 2018-12-15  9:03 ` Luca Coelho
  2018-12-18 12:06 ` [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Johannes Berg
  24 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2018-12-15  9:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Ilan Peer, Luca Coelho

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

The radiotap vendor data might be placed after some other
radiotap elements, and thus when accessing it, need to access
the correct offset in the skb data. Fix the code accordingly.

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

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 85c365fc7a0c..45aad3d3108c 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -762,8 +762,12 @@ ieee80211_rx_monitor(struct ieee80211_local *local, struct sk_buff *origskb,
 	if (status->flag & RX_FLAG_RADIOTAP_HE_MU)
 		rtap_space += sizeof(struct ieee80211_radiotap_he_mu);
 
+	if (status->flag & RX_FLAG_RADIOTAP_LSIG)
+		rtap_space += sizeof(struct ieee80211_radiotap_lsig);
+
 	if (unlikely(status->flag & RX_FLAG_RADIOTAP_VENDOR_DATA)) {
-		struct ieee80211_vendor_radiotap *rtap = (void *)origskb->data;
+		struct ieee80211_vendor_radiotap *rtap =
+			(void *)(origskb->data + rtap_space);
 
 		rtap_space += sizeof(*rtap) + rtap->len + rtap->pad;
 	}
-- 
2.19.2


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

* Re: [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection
  2018-12-15  9:03 ` [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection Luca Coelho
@ 2018-12-15 17:31   ` Emmanuel Grumbach
  2018-12-18  0:55     ` Coelho, Luciano
  0 siblings, 1 reply; 40+ messages in thread
From: Emmanuel Grumbach @ 2018-12-15 17:31 UTC (permalink / raw)
  To: Luca Coelho
  Cc: Berg, Johannes, linux-wireless, Emmanuel Grumbach, stable,
	Coelho, Luciano

On Sat, Dec 15, 2018 at 11:20 AM Luca Coelho <luca@coelho.fi> wrote:
>
> From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
>
> NullFunc packets should never be duplicate just like
> QoS-NullFunc packets.
>
> We saw a client that enters / exits power save with
> NullFunc frames (and not with QoS-NullFunc) despite the
> fact that the association supports HT.
> This specific client also re-uses a non-zero sequence number
> for different NullFunc frames.
> At some point, the client had to send a retransmission of
> the NullFunc frame and we dropped it, leading to a
> misalignment in the power save state.
> Fix this by never consider a NullFunc frame as duplicate,
> just like we do for QoS NullFunc frames.
>
> This fixes https://bugzilla.kernel.org/show_bug.cgi?id=201449
>

This has already been sent, it is in net.git already :)

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

* Re: [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection
  2018-12-15 17:31   ` Emmanuel Grumbach
@ 2018-12-18  0:55     ` Coelho, Luciano
  0 siblings, 0 replies; 40+ messages in thread
From: Coelho, Luciano @ 2018-12-18  0:55 UTC (permalink / raw)
  To: egrumbach; +Cc: linux-wireless, stable, johannes, Grumbach, Emmanuel

On Sat, 2018-12-15 at 19:31 +0200, Emmanuel Grumbach wrote:
> On Sat, Dec 15, 2018 at 11:20 AM Luca Coelho <luca@coelho.fi> wrote:
> > From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > 
> > NullFunc packets should never be duplicate just like
> > QoS-NullFunc packets.
> > 
> > We saw a client that enters / exits power save with
> > NullFunc frames (and not with QoS-NullFunc) despite the
> > fact that the association supports HT.
> > This specific client also re-uses a non-zero sequence number
> > for different NullFunc frames.
> > At some point, the client had to send a retransmission of
> > the NullFunc frame and we dropped it, leading to a
> > misalignment in the power save state.
> > Fix this by never consider a NullFunc frame as duplicate,
> > just like we do for QoS NullFunc frames.
> > 
> > This fixes https://bugzilla.kernel.org/show_bug.cgi?id=201449
> > 
> 
> This has already been sent, it is in net.git already :)

Oops, sorry, my bad.  I obviously forgot to look it up before sending.

--
Cheers,
Luca.

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

* Re: [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15
  2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
                   ` (23 preceding siblings ...)
  2018-12-15  9:03 ` [PATCH 24/24] mac80211: Properly access radiotap vendor data Luca Coelho
@ 2018-12-18 12:06 ` Johannes Berg
  2018-12-18 12:08   ` Johannes Berg
  24 siblings, 1 reply; 40+ messages in thread
From: Johannes Berg @ 2018-12-18 12:06 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless, Luca Coelho

Ok...

Patch 1 is already in, from somebody else - dropped.
Patch 12 was already in from Emmanuel - dropped.

Applied to mac80211: 9, 11, 15, 21, 24

(yes, the others are left for mac80211-next, for later)

johannes


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

* Re: [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15
  2018-12-18 12:06 ` [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Johannes Berg
@ 2018-12-18 12:08   ` Johannes Berg
  0 siblings, 0 replies; 40+ messages in thread
From: Johannes Berg @ 2018-12-18 12:08 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless, Luca Coelho

On Tue, 2018-12-18 at 13:06 +0100, Johannes Berg wrote:
> Ok...
> 
> Patch 1 is already in, from somebody else - dropped.
> Patch 12 was already in from Emmanuel - dropped.
> 
> Applied to mac80211: 9, 11, 15, 21, 24
> 
> (yes, the others are left for mac80211-next, for later)

No, also took 5 to mac80211 now.

johannes


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

* Re: [PATCH 13/24] cfg80211: pmsr: fix MAC address setting
  2018-12-15  9:03 ` [PATCH 13/24] cfg80211: pmsr: fix MAC address setting Luca Coelho
@ 2018-12-18 12:17   ` Johannes Berg
  2019-02-06  5:47     ` Luca Coelho
  0 siblings, 1 reply; 40+ messages in thread
From: Johannes Berg @ 2018-12-18 12:17 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless

On Sat, 2018-12-15 at 11:03 +0200, Luca Coelho wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> When we *don't* have a MAC address attribute, we shouldn't
> try to use this - this was intended to copy the local MAC
> address instead, so fix it.


This patch

>  
> +	flush_work(&wdev->pmsr_free_wk);

Erroneously got some other stuff merged into it - dropped.

johannes


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

* Re: [PATCH 17/24] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH
  2018-12-15  9:03 ` [PATCH 17/24] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH Luca Coelho
@ 2019-01-25 12:41   ` Johannes Berg
  2019-02-06  8:02     ` [PATCH v2] " Luca Coelho
  0 siblings, 1 reply; 40+ messages in thread
From: Johannes Berg @ 2019-01-25 12:41 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless, Andrei Otcheretianski, Luca Coelho

On Sat, 2018-12-15 at 11:03 +0200, Luca Coelho wrote:
> From: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
> 
> This is needed for the devices that manage PMKSA caching internally and
> don't implement SET/DEL PMKSA commands.

It'd be nice to have more explanation here. Is this for station side? or
AP side? I would've guessed AP side based on the fact that I also have 
https://patchwork.kernel.org/patch/10777175/ which also adds the PMKID,
but since you talk about PMKSA caching and that's only added for AP side
in https://patchwork.kernel.org/patch/10769745/ I'm confused.

--> changes requested

I've asked Jouni to take a look at the two above-mentioned patches and
will likely accept them (it seems mostly reasonable to me) so in that
case please also rebase this patch to deal with the overlapping changes.

johannes


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

* Re: [PATCH 13/24] cfg80211: pmsr: fix MAC address setting
  2018-12-18 12:17   ` Johannes Berg
@ 2019-02-06  5:47     ` Luca Coelho
  2019-02-06  5:53       ` Luca Coelho
  0 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2019-02-06  5:47 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Tue, 2018-12-18 at 13:17 +0100, Johannes Berg wrote:
> On Sat, 2018-12-15 at 11:03 +0200, Luca Coelho wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > When we *don't* have a MAC address attribute, we shouldn't
> > try to use this - this was intended to copy the local MAC
> > address instead, so fix it.
> 
> This patch
> 
> >  
> > +	flush_work(&wdev->pmsr_free_wk);
> 
> Erroneously got some other stuff merged into it - dropped.

That's because I squashed some other patches that said to fix this one
into it.  I'll update the commit message so it makes more sense and
resend.

--
Cheers,
Luca.


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

* Re: [PATCH 13/24] cfg80211: pmsr: fix MAC address setting
  2019-02-06  5:47     ` Luca Coelho
@ 2019-02-06  5:53       ` Luca Coelho
  2019-02-06  5:59         ` [PATCH v2 1/2] " Luca Coelho
  0 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2019-02-06  5:53 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Wed, 2019-02-06 at 07:47 +0200, Luca Coelho wrote:
> On Tue, 2018-12-18 at 13:17 +0100, Johannes Berg wrote:
> > On Sat, 2018-12-15 at 11:03 +0200, Luca Coelho wrote:
> > > From: Johannes Berg <johannes.berg@intel.com>
> > > 
> > > When we *don't* have a MAC address attribute, we shouldn't
> > > try to use this - this was intended to copy the local MAC
> > > address instead, so fix it.
> > 
> > This patch
> > 
> > >  
> > > +	flush_work(&wdev->pmsr_free_wk);
> > 
> > Erroneously got some other stuff merged into it - dropped.
> 
> That's because I squashed some other patches that said to fix this
> one
> into it.  I'll update the commit message so it makes more sense and
> resend.

Actually, one of the patches I squashed was marked as fixing this one,
but it's not really, so I'll split that one out.

--
Luca.


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

* [PATCH v2 1/2] cfg80211: pmsr: fix MAC address setting
  2019-02-06  5:53       ` Luca Coelho
@ 2019-02-06  5:59         ` Luca Coelho
  2019-02-06  5:59           ` [PATCH 2/2] " Luca Coelho
  0 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2019-02-06  5:59 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

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

When we *don't* have a MAC address attribute, we shouldn't
try to use this - this was intended to copy the local MAC
address instead, so fix it.

Fixes: 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM initiator API")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/pmsr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/wireless/pmsr.c b/net/wireless/pmsr.c
index de9286703280..f2e388e329fd 100644
--- a/net/wireless/pmsr.c
+++ b/net/wireless/pmsr.c
@@ -256,8 +256,7 @@ int nl80211_pmsr_start(struct sk_buff *skb, struct genl_info *info)
 		if (err)
 			goto out_err;
 	} else {
-		memcpy(req->mac_addr, nla_data(info->attrs[NL80211_ATTR_MAC]),
-		       ETH_ALEN);
+		memcpy(req->mac_addr, wdev_address(wdev), ETH_ALEN);
 		memset(req->mac_addr_mask, 0xff, ETH_ALEN);
 	}
 
-- 
2.20.1


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

* [PATCH 2/2] cfg80211: pmsr: fix MAC address setting
  2019-02-06  5:59         ` [PATCH v2 1/2] " Luca Coelho
@ 2019-02-06  5:59           ` Luca Coelho
  2019-02-06  6:01             ` Luca Coelho
  0 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2019-02-06  5:59 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

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

When we destroy the interface we already hold the wdev->mtx
while calling cfg80211_pmsr_wdev_down(), which assumes this
isn't true and flushes the worker that takes the lock, thus
leading to a deadlock.

Fix this by refactoring the worker and calling its code in
cfg80211_pmsr_wdev_down() directly.

We still need to flush the work later to make sure it's not
still running and will crash, but it will not do anything.

Fixes: 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM initiator API")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/core.c |  2 ++
 net/wireless/pmsr.c | 22 +++++++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 623dfe5e211c..b36ad8efb5e5 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1068,6 +1068,8 @@ static void __cfg80211_unregister_wdev(struct wireless_dev *wdev, bool sync)
 
 	ASSERT_RTNL();
 
+	flush_work(&wdev->pmsr_free_wk);
+
 	nl80211_notify_iface(rdev, wdev, NL80211_CMD_DEL_INTERFACE);
 
 	list_del_rcu(&wdev->list);
diff --git a/net/wireless/pmsr.c b/net/wireless/pmsr.c
index f2e388e329fd..78c3f5633692 100644
--- a/net/wireless/pmsr.c
+++ b/net/wireless/pmsr.c
@@ -529,14 +529,14 @@ void cfg80211_pmsr_report(struct wireless_dev *wdev,
 }
 EXPORT_SYMBOL_GPL(cfg80211_pmsr_report);
 
-void cfg80211_pmsr_free_wk(struct work_struct *work)
+static void cfg80211_pmsr_process_abort(struct wireless_dev *wdev)
 {
-	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
-						 pmsr_free_wk);
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
 	struct cfg80211_pmsr_request *req, *tmp;
 	LIST_HEAD(free_list);
 
+	lockdep_assert_held(&wdev->mtx);
+
 	spin_lock_bh(&wdev->pmsr_lock);
 	list_for_each_entry_safe(req, tmp, &wdev->pmsr_list, list) {
 		if (req->nl_portid)
@@ -546,14 +546,22 @@ void cfg80211_pmsr_free_wk(struct work_struct *work)
 	spin_unlock_bh(&wdev->pmsr_lock);
 
 	list_for_each_entry_safe(req, tmp, &free_list, list) {
-		wdev_lock(wdev);
 		rdev_abort_pmsr(rdev, wdev, req);
-		wdev_unlock(wdev);
 
 		kfree(req);
 	}
 }
 
+void cfg80211_pmsr_free_wk(struct work_struct *work)
+{
+	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
+						 pmsr_free_wk);
+
+	wdev_lock(wdev);
+	cfg80211_pmsr_process_abort(wdev);
+	wdev_unlock(wdev);
+}
+
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev)
 {
 	struct cfg80211_pmsr_request *req;
@@ -567,8 +575,8 @@ void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev)
 	spin_unlock_bh(&wdev->pmsr_lock);
 
 	if (found)
-		schedule_work(&wdev->pmsr_free_wk);
-	flush_work(&wdev->pmsr_free_wk);
+		cfg80211_pmsr_process_abort(wdev);
+
 	WARN_ON(!list_empty(&wdev->pmsr_list));
 }
 
-- 
2.20.1


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

* Re: [PATCH 2/2] cfg80211: pmsr: fix MAC address setting
  2019-02-06  5:59           ` [PATCH 2/2] " Luca Coelho
@ 2019-02-06  6:01             ` Luca Coelho
  2019-02-06  6:03               ` [PATCH] cfg80211: pmsr: fix abort locking Luca Coelho
  0 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2019-02-06  6:01 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

On Wed, 2019-02-06 at 07:59 +0200, Luca Coelho wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> When we destroy the interface we already hold the wdev->mtx
> while calling cfg80211_pmsr_wdev_down(), which assumes this
> isn't true and flushes the worker that takes the lock, thus
> leading to a deadlock.
> 
> Fix this by refactoring the worker and calling its code in
> cfg80211_pmsr_wdev_down() directly.
> 
> We still need to flush the work later to make sure it's not
> still running and will crash, but it will not do anything.
> 
> Fixes: 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM
> initiator API")
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---

Oops, this came out with the wrong subject, please ignore it.  I'll
resend with the correct one.

--
Luca.


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

* [PATCH] cfg80211: pmsr: fix abort locking
  2019-02-06  6:01             ` Luca Coelho
@ 2019-02-06  6:03               ` Luca Coelho
  0 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2019-02-06  6:03 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Johannes Berg

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

When we destroy the interface we already hold the wdev->mtx
while calling cfg80211_pmsr_wdev_down(), which assumes this
isn't true and flushes the worker that takes the lock, thus
leading to a deadlock.

Fix this by refactoring the worker and calling its code in
cfg80211_pmsr_wdev_down() directly.

We still need to flush the work later to make sure it's not
still running and will crash, but it will not do anything.

Fixes: 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM initiator API")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/core.c |  2 ++
 net/wireless/pmsr.c | 22 +++++++++++++++-------
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 623dfe5e211c..b36ad8efb5e5 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1068,6 +1068,8 @@ static void __cfg80211_unregister_wdev(struct wireless_dev *wdev, bool sync)
 
 	ASSERT_RTNL();
 
+	flush_work(&wdev->pmsr_free_wk);
+
 	nl80211_notify_iface(rdev, wdev, NL80211_CMD_DEL_INTERFACE);
 
 	list_del_rcu(&wdev->list);
diff --git a/net/wireless/pmsr.c b/net/wireless/pmsr.c
index f2e388e329fd..78c3f5633692 100644
--- a/net/wireless/pmsr.c
+++ b/net/wireless/pmsr.c
@@ -529,14 +529,14 @@ void cfg80211_pmsr_report(struct wireless_dev *wdev,
 }
 EXPORT_SYMBOL_GPL(cfg80211_pmsr_report);
 
-void cfg80211_pmsr_free_wk(struct work_struct *work)
+static void cfg80211_pmsr_process_abort(struct wireless_dev *wdev)
 {
-	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
-						 pmsr_free_wk);
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
 	struct cfg80211_pmsr_request *req, *tmp;
 	LIST_HEAD(free_list);
 
+	lockdep_assert_held(&wdev->mtx);
+
 	spin_lock_bh(&wdev->pmsr_lock);
 	list_for_each_entry_safe(req, tmp, &wdev->pmsr_list, list) {
 		if (req->nl_portid)
@@ -546,14 +546,22 @@ void cfg80211_pmsr_free_wk(struct work_struct *work)
 	spin_unlock_bh(&wdev->pmsr_lock);
 
 	list_for_each_entry_safe(req, tmp, &free_list, list) {
-		wdev_lock(wdev);
 		rdev_abort_pmsr(rdev, wdev, req);
-		wdev_unlock(wdev);
 
 		kfree(req);
 	}
 }
 
+void cfg80211_pmsr_free_wk(struct work_struct *work)
+{
+	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
+						 pmsr_free_wk);
+
+	wdev_lock(wdev);
+	cfg80211_pmsr_process_abort(wdev);
+	wdev_unlock(wdev);
+}
+
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev)
 {
 	struct cfg80211_pmsr_request *req;
@@ -567,8 +575,8 @@ void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev)
 	spin_unlock_bh(&wdev->pmsr_lock);
 
 	if (found)
-		schedule_work(&wdev->pmsr_free_wk);
-	flush_work(&wdev->pmsr_free_wk);
+		cfg80211_pmsr_process_abort(wdev);
+
 	WARN_ON(!list_empty(&wdev->pmsr_list));
 }
 
-- 
2.20.1


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

* [PATCH v2] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH
  2019-01-25 12:41   ` Johannes Berg
@ 2019-02-06  8:02     ` Luca Coelho
  2019-02-22 12:41       ` Johannes Berg
  0 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2019-02-06  8:02 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless, Andrei Otcheretianski, Luca Coelho

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

This is needed for the devices that rely on user space to perform the
authentication, but offload the 4-way handshake and PMKSA caching.
Such devices don't implement SET/DEL_PMKSA commands, however they
still need to know the derived PMK and PMKID in order to proceed to
association and 4-way handshake phase.

Signed-off-by: Andrei Otcheretianski <andrei.otcheretianski@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/cfg80211.h       |  4 ++++
 include/uapi/linux/nl80211.h |  4 +++-
 net/wireless/nl80211.c       | 13 ++++++++++++-
 3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7f2739a90bdb..5566a95b27d8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2846,6 +2846,8 @@ struct cfg80211_pmk_conf {
  *	the real status code for failures. Used only for the authentication
  *	response command interface (user space to driver).
  * @pmkid: The identifier to refer a PMKSA.
+ * @pmk_len: Length of PMK if present.
+ * @pmk: Derived PMK
  */
 struct cfg80211_external_auth_params {
 	enum nl80211_external_auth_action action;
@@ -2854,6 +2856,8 @@ struct cfg80211_external_auth_params {
 	unsigned int key_mgmt_suite;
 	u16 status;
 	const u8 *pmkid;
+	int pmk_len;
+	const u8 *pmk;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index dd4f86ee286e..10315b181ec4 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1022,7 +1022,9 @@
  *	further with the association after getting successful authentication
  *	status. User space indicates the authentication status through
  *	%NL80211_ATTR_STATUS_CODE attribute in %NL80211_CMD_EXTERNAL_AUTH
- *	command interface.
+ *	command interface. In case of success, user space also includes the
+ *	derived PMK and PMKID through %NL80211_ATTR_PMK and
+ *	%NL80211_ATTR_PMKID.
  *
  *	Host driver reports this status on an authentication failure to the
  *	user space through the connect result as the user space would have
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index a3cc039b9f55..ce5d87d512e2 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -13098,6 +13098,12 @@ static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info)
 	if (!info->attrs[NL80211_ATTR_STATUS_CODE])
 		return -EINVAL;
 
+	if ((info->attrs[NL80211_ATTR_PMK] &&
+	     !info->attrs[NL80211_ATTR_PMKID]) ||
+	    (info->attrs[NL80211_ATTR_PMKID] &&
+	     !info->attrs[NL80211_ATTR_PMK]))
+		return -EINVAL;
+
 	memset(&params, 0, sizeof(params));
 
 	if (info->attrs[NL80211_ATTR_SSID]) {
@@ -13115,8 +13121,13 @@ static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info)
 
 	params.status = nla_get_u16(info->attrs[NL80211_ATTR_STATUS_CODE]);
 
-	if (info->attrs[NL80211_ATTR_PMKID])
+	if (info->attrs[NL80211_ATTR_PMKID]) {
+		if (info->attrs[NL80211_ATTR_PMK]) {
+			params.pmk_len = nla_len(info->attrs[NL80211_ATTR_PMK]);
+			params.pmk = nla_data(info->attrs[NL80211_ATTR_PMK]);
+		}
 		params.pmkid = nla_data(info->attrs[NL80211_ATTR_PMKID]);
+	}
 
 	return rdev_external_auth(rdev, dev, &params);
 }
-- 
2.20.1


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

* Re: [PATCH v2] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH
  2019-02-06  8:02     ` [PATCH v2] " Luca Coelho
@ 2019-02-22 12:41       ` Johannes Berg
  2019-03-08 11:26         ` Luciano Coelho
  0 siblings, 1 reply; 40+ messages in thread
From: Johannes Berg @ 2019-02-22 12:41 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless, Andrei Otcheretianski, Luca Coelho


> +++ b/net/wireless/nl80211.c
> @@ -13098,6 +13098,12 @@ static int nl80211_external_auth(struct sk_buff *skb, struct genl_info *info)
>  	if (!info->attrs[NL80211_ATTR_STATUS_CODE])
>  		return -EINVAL;
>  
> +	if ((info->attrs[NL80211_ATTR_PMK] &&
> +	     !info->attrs[NL80211_ATTR_PMKID]) ||
> +	    (info->attrs[NL80211_ATTR_PMKID] &&
> +	     !info->attrs[NL80211_ATTR_PMK]))
> +		return -EINVAL;

This constitutes a netlink API change, so no, can't be right? PMKID was
perfectly reasonable to pass by itself before.

johannes


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

* Re: [PATCH v2] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH
  2019-02-22 12:41       ` Johannes Berg
@ 2019-03-08 11:26         ` Luciano Coelho
  0 siblings, 0 replies; 40+ messages in thread
From: Luciano Coelho @ 2019-03-08 11:26 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Andrei Otcheretianski

On Fri, 2019-02-22 at 13:41 +0100, Johannes Berg wrote:
> > +++ b/net/wireless/nl80211.c
> > @@ -13098,6 +13098,12 @@ static int nl80211_external_auth(struct
> > sk_buff *skb, struct genl_info *info)
> >  	if (!info->attrs[NL80211_ATTR_STATUS_CODE])
> >  		return -EINVAL;
> >  
> > +	if ((info->attrs[NL80211_ATTR_PMK] &&
> > +	     !info->attrs[NL80211_ATTR_PMKID]) ||
> > +	    (info->attrs[NL80211_ATTR_PMKID] &&
> > +	     !info->attrs[NL80211_ATTR_PMK]))
> > +		return -EINVAL;
> 
> This constitutes a netlink API change, so no, can't be right? PMKID
> was
> perfectly reasonable to pass by itself before.

Good point.  Andrei, can you fix this? This can easily be changed to
accept PMKID alone but still do what you want when both are included.

--
Luca.


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

end of thread, other threads:[~2019-03-08 11:26 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-15  9:03 [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Luca Coelho
2018-12-15  9:03 ` [PATCH 01/24] mac80211: suspicious RCU usage fix Luca Coelho
2018-12-15  9:03 ` [PATCH 02/24] ieee80211: add bits for TWT in Extended Capabilities IE Luca Coelho
2018-12-15  9:03 ` [PATCH 03/24] mac80211: propagate the support for TWT to the driver Luca Coelho
2018-12-15  9:03 ` [PATCH 04/24] mac80211: update HE operation fields to D3.0 Luca Coelho
2018-12-15  9:03 ` [PATCH 05/24] mac80211: free skb fraglist before freeing the skb Luca Coelho
2018-12-15  9:03 ` [PATCH 06/24] mac80211: don't build AMSDU from GSO packets Luca Coelho
2018-12-15  9:03 ` [PATCH 07/24] mac80211: document RCU requirements for ieee80211_tx_dequeue() Luca Coelho
2018-12-15  9:03 ` [PATCH 08/24] mac80211: remove superfluous NULL check Luca Coelho
2018-12-15  9:03 ` [PATCH 09/24] mac80211: fix a kernel panic when TXing after TXQ teardown Luca Coelho
2018-12-15  9:03 ` [PATCH 10/24] mac80211: never pass NULL params to ieee80211_if_add() Luca Coelho
2018-12-15  9:03 ` [PATCH 11/24] mac80211: fix radiotap vendor presence bitmap handling Luca Coelho
2018-12-15  9:03 ` [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection Luca Coelho
2018-12-15 17:31   ` Emmanuel Grumbach
2018-12-18  0:55     ` Coelho, Luciano
2018-12-15  9:03 ` [PATCH 13/24] cfg80211: pmsr: fix MAC address setting Luca Coelho
2018-12-18 12:17   ` Johannes Berg
2019-02-06  5:47     ` Luca Coelho
2019-02-06  5:53       ` Luca Coelho
2019-02-06  5:59         ` [PATCH v2 1/2] " Luca Coelho
2019-02-06  5:59           ` [PATCH 2/2] " Luca Coelho
2019-02-06  6:01             ` Luca Coelho
2019-02-06  6:03               ` [PATCH] cfg80211: pmsr: fix abort locking Luca Coelho
2018-12-15  9:03 ` [PATCH 14/24] mac80211: update driver when MU EDCA params change Luca Coelho
2018-12-15  9:03 ` [PATCH 15/24] cfg80211: fix ieee80211_get_vht_max_nss() Luca Coelho
2018-12-15  9:03 ` [PATCH 16/24] mac80211: Properly handle SKB with radiotap only Luca Coelho
2018-12-15  9:03 ` [PATCH 17/24] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH Luca Coelho
2019-01-25 12:41   ` Johannes Berg
2019-02-06  8:02     ` [PATCH v2] " Luca Coelho
2019-02-22 12:41       ` Johannes Berg
2019-03-08 11:26         ` Luciano Coelho
2018-12-15  9:03 ` [PATCH 18/24] mac80211: set STA flag DISABLE_HE if HE is not supported Luca Coelho
2018-12-15  9:03 ` [PATCH 19/24] mac80211: do not advertise HE cap IE if HE disabled Luca Coelho
2018-12-15  9:03 ` [PATCH 20/24] cfg80211: add some missing fall through annotations Luca Coelho
2018-12-15  9:03 ` [PATCH 21/24] nl80211: fix memory leak if validate_pae_over_nl80211() fails Luca Coelho
2018-12-15  9:03 ` [PATCH 22/24] cfg80211: clarify LCI/civic location documentation Luca Coelho
2018-12-15  9:03 ` [PATCH 23/24] mac80211: ftm responder: remove pointless defensive coding Luca Coelho
2018-12-15  9:03 ` [PATCH 24/24] mac80211: Properly access radiotap vendor data Luca Coelho
2018-12-18 12:06 ` [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15 Johannes Berg
2018-12-18 12:08   ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).