params->crypto.n_akm_suites seems to be limited to two AKM suites. Once
there are more they will be passed as extra elements of type WLAN_EID_RSN
or WLAN_EID_VENDOR_SPECIFIC.
This takes some snippets from the downstream vendor driver to parse
these elements and to set the correct protocol and key_mgmt bits to
enable the desired key managements algorithms in the hardware.
This patch is not a request for inclusion, more a heads up that there's
something missing and the question if the approach taken is the right
one or if there are other preferred ways to fix this issue.
Signed-off-by: Sascha Hauer <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/fw.h | 3 +
.../net/wireless/marvell/mwifiex/uap_cmd.c | 149 +++++++++++++++---
2 files changed, 132 insertions(+), 20 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 3adc447b715f6..d576b2d71a6b9 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -415,6 +415,9 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
#define KEY_MGMT_NONE 0x04
#define KEY_MGMT_PSK 0x02
#define KEY_MGMT_EAP 0x01
+#define KEY_MGMT_PSK_SHA256 0x100
+#define KEY_MGMT_OWE 0x200
+#define KEY_MGMT_SAE 0x400
#define CIPHER_TKIP 0x04
#define CIPHER_AES_CCMP 0x08
#define VALID_CIPHER_BITMAP 0x0c
diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
index 491e366119096..4b21626e2dd7f 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
@@ -9,6 +9,112 @@
#include "11ac.h"
#include "11n.h"
+struct wpa_suite_ucast {
+ /* count */
+ u16 count;
+ /** wpa_suite list */
+ __be32 suite[1];
+} __packed;
+
+struct IEEEtypes_Rsn_t {
+ /** Rsn : version */
+ u16 version;
+ /** Rsn : group cipher */
+ __be32 group_cipher;
+ /** Rsn : pairwise cipher */
+ struct wpa_suite_ucast pairwise_cipher;
+} __packed;
+
+static void woal_check_rsn_ie(const struct IEEEtypes_Rsn_t *rsn_ie, int len,
+ struct mwifiex_uap_bss_param *bss_config, u8 *pairwise_cipher)
+{
+ int left, count, i;
+ struct wpa_suite_ucast *key_mgmt;
+
+ left = len;
+ if (left < (int)sizeof(struct IEEEtypes_Rsn_t))
+ return;
+
+ bss_config->wpa_cfg.group_cipher = 0;
+ *pairwise_cipher = 0;
+ bss_config->key_mgmt = 0;
+
+ /* check the group cipher */
+ switch (be32_to_cpu(rsn_ie->group_cipher)) {
+ case WLAN_CIPHER_SUITE_TKIP:
+ bss_config->wpa_cfg.group_cipher = CIPHER_TKIP;
+ break;
+ case WLAN_CIPHER_SUITE_CCMP:
+ bss_config->wpa_cfg.group_cipher = CIPHER_AES_CCMP;
+ break;
+ default:
+ break;
+ }
+
+ count = le16_to_cpu(rsn_ie->pairwise_cipher.count);
+ for (i = 0; i < count; i++) {
+ switch (be32_to_cpu(rsn_ie->pairwise_cipher.suite[i])) {
+ case WLAN_CIPHER_SUITE_TKIP:
+ *pairwise_cipher |= CIPHER_TKIP;
+ break;
+ case WLAN_CIPHER_SUITE_CCMP:
+ *pairwise_cipher |= CIPHER_AES_CCMP;
+ break;
+ default:
+ break;
+ }
+ }
+ left -= sizeof(struct IEEEtypes_Rsn_t) + (count - 1) * sizeof(__be32);
+ if (left < (int)sizeof(struct wpa_suite_ucast))
+ return;
+
+ key_mgmt = ((void *)rsn_ie + sizeof(struct IEEEtypes_Rsn_t) + (count - 1) * sizeof(__be32));
+ count = le16_to_cpu(key_mgmt->count);
+ if (left < (int)(sizeof(struct wpa_suite_ucast) +
+ (count - 1) * sizeof(__be32)))
+ return;
+
+ for (i = 0; i < count; i++) {
+ switch (be32_to_cpu(key_mgmt->suite[i])) {
+ case WLAN_AKM_SUITE_8021X:
+ bss_config->key_mgmt |= KEY_MGMT_EAP;
+ break;
+ case WLAN_AKM_SUITE_PSK:
+ bss_config->key_mgmt |= KEY_MGMT_PSK;
+ break;
+ case WLAN_AKM_SUITE_PSK_SHA256:
+ bss_config->key_mgmt |= KEY_MGMT_PSK_SHA256;
+ break;
+ case WLAN_AKM_SUITE_SAE:
+ bss_config->key_mgmt |= KEY_MGMT_SAE;
+ break;
+ case WLAN_AKM_SUITE_OWE:
+ bss_config->key_mgmt |= KEY_MGMT_OWE;
+ break;
+ }
+ }
+}
+
+static void woal_find_wpa_ies(const void *ie, int len, struct mwifiex_uap_bss_param *bss_config)
+{
+ const struct element *e;
+
+ e = cfg80211_find_elem(WLAN_EID_RSN, ie, len);
+ if (e) {
+ woal_check_rsn_ie((void *)e->data, e->datalen, bss_config,
+ &bss_config->wpa_cfg.pairwise_cipher_wpa2);
+
+ bss_config->protocol |= PROTOCOL_WPA2;
+ }
+
+ e = cfg80211_find_vendor_elem(WLAN_EID_VENDOR_SPECIFIC, 0x1, ie, len);
+ if (e) {
+ woal_check_rsn_ie((void *)e->data, e->datalen, bss_config,
+ &bss_config->wpa_cfg.pairwise_cipher_wpa);
+ bss_config->protocol |= PROTOCOL_WPA;
+ }
+}
+
/* This function parses security related parameters from cfg80211_ap_settings
* and sets into FW understandable bss_config structure.
*/
@@ -17,6 +123,11 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
struct cfg80211_ap_settings *params) {
int i;
struct mwifiex_wep_key wep_key;
+ const u8 *ie = NULL;
+ int ie_len;
+
+ ie = params->beacon.tail;
+ ie_len = params->beacon.tail_len;
if (!params->privacy) {
bss_config->protocol = PROTOCOL_NO_SECURITY;
@@ -46,36 +157,34 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
bss_config->key_mgmt_operation |= KEY_MGMT_ON_HOST;
+ if (params->crypto.wpa_versions & NL80211_WPA_VERSION_1)
+ bss_config->protocol |= PROTOCOL_WPA;
+ if (params->crypto.wpa_versions & NL80211_WPA_VERSION_2)
+ bss_config->protocol |= PROTOCOL_WPA2;
+
+ woal_find_wpa_ies(ie, ie_len, bss_config);
+
for (i = 0; i < params->crypto.n_akm_suites; i++) {
+ mwifiex_dbg(priv->adapter, MSG, "suite%d: 0x%08x\n", i, params->crypto.akm_suites[i]);
+
switch (params->crypto.akm_suites[i]) {
case WLAN_AKM_SUITE_8021X:
- if (params->crypto.wpa_versions &
- NL80211_WPA_VERSION_1) {
- bss_config->protocol = PROTOCOL_WPA;
- bss_config->key_mgmt = KEY_MGMT_EAP;
- }
- if (params->crypto.wpa_versions &
- NL80211_WPA_VERSION_2) {
- bss_config->protocol |= PROTOCOL_WPA2;
- bss_config->key_mgmt = KEY_MGMT_EAP;
- }
+ bss_config->key_mgmt |= KEY_MGMT_EAP;
break;
case WLAN_AKM_SUITE_PSK:
- if (params->crypto.wpa_versions &
- NL80211_WPA_VERSION_1) {
- bss_config->protocol = PROTOCOL_WPA;
- bss_config->key_mgmt = KEY_MGMT_PSK;
- }
- if (params->crypto.wpa_versions &
- NL80211_WPA_VERSION_2) {
- bss_config->protocol |= PROTOCOL_WPA2;
- bss_config->key_mgmt = KEY_MGMT_PSK;
- }
+ bss_config->key_mgmt |= KEY_MGMT_PSK;
+ break;
+ case WLAN_AKM_SUITE_PSK_SHA256:
+ bss_config->key_mgmt |= KEY_MGMT_PSK_SHA256;
break;
default:
break;
}
}
+
+ mwifiex_dbg(priv->adapter, MSG, "protocol: 0x%08x key_mgmt: 0x%08x\n",
+ bss_config->protocol, bss_config->key_mgmt);
+
for (i = 0; i < params->crypto.n_ciphers_pairwise; i++) {
switch (params->crypto.ciphers_pairwise[i]) {
case WLAN_CIPHER_SUITE_WEP40:
--
2.39.2
Hi Sascha,
kernel test robot noticed the following build warnings:
[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.9 next-20240523]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sascha-Hauer/wifi-mwifiex-fix-parsing-of-more-than-two-AKM-suites/20240523-161947
base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link: https://lore.kernel.org/r/20240523081428.2852276-1-s.hauer%40pengutronix.de
patch subject: [PATCH] wifi: mwifiex: fix parsing of more than two AKM suites
config: x86_64-randconfig-122-20240525 (https://download.01.org/0day-ci/archive/20240525/[email protected]/config)
compiler: gcc-11 (Ubuntu 11.4.0-4ubuntu1) 11.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240525/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
sparse warnings: (new ones prefixed by >>)
>> drivers/net/wireless/marvell/mwifiex/uap_cmd.c:54:17: sparse: sparse: cast to restricted __le16
drivers/net/wireless/marvell/mwifiex/uap_cmd.c:72:17: sparse: sparse: cast to restricted __le16
vim +54 drivers/net/wireless/marvell/mwifiex/uap_cmd.c
27
28 static void woal_check_rsn_ie(const struct IEEEtypes_Rsn_t *rsn_ie, int len,
29 struct mwifiex_uap_bss_param *bss_config, u8 *pairwise_cipher)
30 {
31 int left, count, i;
32 struct wpa_suite_ucast *key_mgmt;
33
34 left = len;
35 if (left < (int)sizeof(struct IEEEtypes_Rsn_t))
36 return;
37
38 bss_config->wpa_cfg.group_cipher = 0;
39 *pairwise_cipher = 0;
40 bss_config->key_mgmt = 0;
41
42 /* check the group cipher */
43 switch (be32_to_cpu(rsn_ie->group_cipher)) {
44 case WLAN_CIPHER_SUITE_TKIP:
45 bss_config->wpa_cfg.group_cipher = CIPHER_TKIP;
46 break;
47 case WLAN_CIPHER_SUITE_CCMP:
48 bss_config->wpa_cfg.group_cipher = CIPHER_AES_CCMP;
49 break;
50 default:
51 break;
52 }
53
> 54 count = le16_to_cpu(rsn_ie->pairwise_cipher.count);
55 for (i = 0; i < count; i++) {
56 switch (be32_to_cpu(rsn_ie->pairwise_cipher.suite[i])) {
57 case WLAN_CIPHER_SUITE_TKIP:
58 *pairwise_cipher |= CIPHER_TKIP;
59 break;
60 case WLAN_CIPHER_SUITE_CCMP:
61 *pairwise_cipher |= CIPHER_AES_CCMP;
62 break;
63 default:
64 break;
65 }
66 }
67 left -= sizeof(struct IEEEtypes_Rsn_t) + (count - 1) * sizeof(__be32);
68 if (left < (int)sizeof(struct wpa_suite_ucast))
69 return;
70
71 key_mgmt = ((void *)rsn_ie + sizeof(struct IEEEtypes_Rsn_t) + (count - 1) * sizeof(__be32));
72 count = le16_to_cpu(key_mgmt->count);
73 if (left < (int)(sizeof(struct wpa_suite_ucast) +
74 (count - 1) * sizeof(__be32)))
75 return;
76
77 for (i = 0; i < count; i++) {
78 switch (be32_to_cpu(key_mgmt->suite[i])) {
79 case WLAN_AKM_SUITE_8021X:
80 bss_config->key_mgmt |= KEY_MGMT_EAP;
81 break;
82 case WLAN_AKM_SUITE_PSK:
83 bss_config->key_mgmt |= KEY_MGMT_PSK;
84 break;
85 case WLAN_AKM_SUITE_PSK_SHA256:
86 bss_config->key_mgmt |= KEY_MGMT_PSK_SHA256;
87 break;
88 case WLAN_AKM_SUITE_SAE:
89 bss_config->key_mgmt |= KEY_MGMT_SAE;
90 break;
91 case WLAN_AKM_SUITE_OWE:
92 bss_config->key_mgmt |= KEY_MGMT_OWE;
93 break;
94 }
95 }
96 }
97
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Sascha Hauer <[email protected]> writes:
> params->crypto.n_akm_suites seems to be limited to two AKM suites. Once
> there are more they will be passed as extra elements of type WLAN_EID_RSN
> or WLAN_EID_VENDOR_SPECIFIC.
>
> This takes some snippets from the downstream vendor driver to parse
> these elements and to set the correct protocol and key_mgmt bits to
> enable the desired key managements algorithms in the hardware.
>
> This patch is not a request for inclusion, more a heads up that there's
> something missing and the question if the approach taken is the right
> one or if there are other preferred ways to fix this issue.
Please mark patches like this as "[PATCH RFC]", that way we maintainers
know to drop them automatically.
> --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> @@ -9,6 +9,112 @@
> #include "11ac.h"
> #include "11n.h"
>
> +struct wpa_suite_ucast {
> + /* count */
> + u16 count;
> + /** wpa_suite list */
> + __be32 suite[1];
> +} __packed;
The comments here are not adding any extra information, please remove.
In general having a some kind of prefix in the struct name would be
nice. I don't know what mwifiex uses (if any) but, for example, in
ath12k we use 'ath12k_'.
> +struct IEEEtypes_Rsn_t {
Lower case, no '_t' and also improve the naming.
> + /** Rsn : version */
> + u16 version;
> + /** Rsn : group cipher */
> + __be32 group_cipher;
> + /** Rsn : pairwise cipher */
> + struct wpa_suite_ucast pairwise_cipher;
> +} __packed;
Useless comments.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Sat, May 25, 2024 at 12:15:22PM +0300, Kalle Valo wrote:
> Sascha Hauer <[email protected]> writes:
>
> > params->crypto.n_akm_suites seems to be limited to two AKM suites. Once
> > there are more they will be passed as extra elements of type WLAN_EID_RSN
> > or WLAN_EID_VENDOR_SPECIFIC.
> >
> > This takes some snippets from the downstream vendor driver to parse
> > these elements and to set the correct protocol and key_mgmt bits to
> > enable the desired key managements algorithms in the hardware.
> >
> > This patch is not a request for inclusion, more a heads up that there's
> > something missing and the question if the approach taken is the right
> > one or if there are other preferred ways to fix this issue.
>
> Please mark patches like this as "[PATCH RFC]", that way we maintainers
> know to drop them automatically.
Yes, will do. I should have known that.
The question I had with this patch is more like whether the approach is
fine.
I wonder why I have to parse the WLAN_EID_RSN element in driver specific
code and why I do not find similar code in other drivers which should
suffer from the same problem.
Looking deeper at it the Kernel by default only supports two AKM suites.
wiphy->max_num_akm_suites is initialized to NL80211_MAX_NR_AKM_SUITES
(which is 2). Whenever wpa_supplicant/hostapd specify more AKM suites
the suites exceeding 2 are encoded in the WLAN_EID_RSN element and I
would expect other drivers to handle this as well.
I realized that the Kernel can handle up to 10 AKM suites by setting
wiphy->max_num_akm_suites to CFG80211_MAX_NUM_AKM_SUITES and that
seems to do the trick as well, at least when I patch wpa_supplicant
to actually use this increased number.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |