2018-12-15 09:03:42

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15

From: Luca Coelho <[email protected]>

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



2018-12-15 09:03:37

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 04/24] mac80211: update HE operation fields to D3.0

From: Shaul Triebitz <[email protected]>

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

Signed-off-by: Shaul Triebitz <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:03:37

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 05/24] mac80211: free skb fraglist before freeing the skb

From: Sara Sharon <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:03:37

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 03/24] mac80211: propagate the support for TWT to the driver

From: Emmanuel Grumbach <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:03:38

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 06/24] mac80211: don't build AMSDU from GSO packets

From: Sara Sharon <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:03:37

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 02/24] ieee80211: add bits for TWT in Extended Capabilities IE

From: Emmanuel Grumbach <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:03:40

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 09/24] mac80211: fix a kernel panic when TXing after TXQ teardown

From: Sara Sharon <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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 <[email protected]>
* 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


2018-12-15 09:03:39

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 07/24] mac80211: document RCU requirements for ieee80211_tx_dequeue()

From: Johannes Berg <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:03:40

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 08/24] mac80211: remove superfluous NULL check

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2018-12-15 09:03:43

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 01/24] mac80211: suspicious RCU usage fix

From: Lior Cohen <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:03:52

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 20/24] cfg80211: add some missing fall through annotations

From: Luca Coelho <[email protected]>

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 <[email protected]>
---
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 <[email protected]>
* 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


2018-12-15 09:05:33

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 16/24] mac80211: Properly handle SKB with radiotap only

From: Ilan Peer <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:07:14

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 15/24] cfg80211: fix ieee80211_get_vht_max_nss()

From: Johannes Berg <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:08:55

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 22/24] cfg80211: clarify LCI/civic location documentation

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2018-12-15 09:10:35

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 13/24] cfg80211: pmsr: fix MAC address setting

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2018-12-15 09:12:16

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 10/24] mac80211: never pass NULL params to ieee80211_if_add()

From: Johannes Berg <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:13:58

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 14/24] mac80211: update driver when MU EDCA params change

From: Shaul Triebitz <[email protected]>

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

Signed-off-by: Shaul Triebitz <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:15:38

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 21/24] nl80211: fix memory leak if validate_pae_over_nl80211() fails

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2018-12-15 09:17:19

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 24/24] mac80211: Properly access radiotap vendor data

From: Ilan Peer <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:19:00

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection

From: Emmanuel Grumbach <[email protected]>

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: <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:20:40

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 11/24] mac80211: fix radiotap vendor presence bitmap handling

From: Johannes Berg <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:22:21

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 19/24] mac80211: do not advertise HE cap IE if HE disabled

From: Shaul Triebitz <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:24:02

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 18/24] mac80211: set STA flag DISABLE_HE if HE is not supported

From: Shaul Triebitz <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:24:48

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 17/24] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH

From: Andrei Otcheretianski <[email protected]>

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

Signed-off-by: Andrei Otcheretianski <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2018-12-15 09:25:42

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 23/24] mac80211: ftm responder: remove pointless defensive coding

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2018-12-15 17:31:53

by Emmanuel Grumbach

[permalink] [raw]
Subject: Re: [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection

On Sat, Dec 15, 2018 at 11:20 AM Luca Coelho <[email protected]> wrote:
>
> From: Emmanuel Grumbach <[email protected]>
>
> 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 :)

2018-12-18 00:55:33

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH 12/24] mac80211: ignore NullFunc frames in the duplicate detection

On Sat, 2018-12-15 at 19:31 +0200, Emmanuel Grumbach wrote:
> On Sat, Dec 15, 2018 at 11:20 AM Luca Coelho <[email protected]> wrote:
> > From: Emmanuel Grumbach <[email protected]>
> >
> > 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.

2018-12-18 12:07:02

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15

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


2018-12-18 12:08:18

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 00/24] cfg80211/mac80211 patches from our internal tree 2018-12-15

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


2018-12-18 12:17:12

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 13/24] cfg80211: pmsr: fix MAC address setting

On Sat, 2018-12-15 at 11:03 +0200, Luca Coelho wrote:
> From: Johannes Berg <[email protected]>
>
> 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


2019-01-25 12:41:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 17/24] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH

On Sat, 2018-12-15 at 11:03 +0200, Luca Coelho wrote:
> From: Andrei Otcheretianski <[email protected]>
>
> 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


2019-02-06 05:47:14

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 13/24] cfg80211: pmsr: fix MAC address setting

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 <[email protected]>
> >
> > 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.


2019-02-06 05:53:38

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 13/24] cfg80211: pmsr: fix MAC address setting

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 <[email protected]>
> > >
> > > 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.


2019-02-06 05:59:53

by Luca Coelho

[permalink] [raw]
Subject: [PATCH 2/2] cfg80211: pmsr: fix MAC address setting

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2019-02-06 05:59:54

by Luca Coelho

[permalink] [raw]
Subject: [PATCH v2 1/2] cfg80211: pmsr: fix MAC address setting

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2019-02-06 06:01:26

by Luca Coelho

[permalink] [raw]
Subject: Re: [PATCH 2/2] cfg80211: pmsr: fix MAC address setting

On Wed, 2019-02-06 at 07:59 +0200, Luca Coelho wrote:
> From: Johannes Berg <[email protected]>
>
> 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 <[email protected]>
> ---

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

--
Luca.


2019-02-06 06:03:18

by Luca Coelho

[permalink] [raw]
Subject: [PATCH] cfg80211: pmsr: fix abort locking

From: Johannes Berg <[email protected]>

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 <[email protected]>
---
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


2019-02-06 08:02:47

by Luca Coelho

[permalink] [raw]
Subject: [PATCH v2] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH

From: Andrei Otcheretianski <[email protected]>

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 <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
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


2019-02-22 12:41:30

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH


> +++ 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


2019-03-08 11:26:41

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v2] cfg80211: Include the PMK and PMKID in NL80211_CMD_EXTERNAL_AUTH

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.