[PATCH 0/2] wifi: nl80211/wilc1000: force WLAN_AKM_SUITE_SAE to big endian in NL80211_CMD_EXTERNAL_AUTH

[PATCH 0/2] wifi: nl80211/wilc1000: force WLAN_AKM_SUITE_SAE to big endian in NL80211_CMD_EXTERNAL_AUTH

[Alexis Lothoré]

This small series is the follow-up to discussions started around a sparse
warning in wilc1000 driver ([1]) and implements the solution suggested by
Johannes. It moves a historically needed conversion to be32 in nl80211 (in
NL80211_CMD_EXTERNAL_AUTH, specifically on NL80211_ATTR_AKM_SUITES property
_only_ when it is set to WLAN_AKM_SUITE_SAE) The user scenario affected by
this update is a connect process on a WPA3-protected access point with
authentication offloaded to user-space. Two drivers are affected by the
update: wilc1000 and qtnfmac. wilc1000 case is handled by a small
companion patch which also fixes the sparse warning.

For the quantenna driver, I don't really get how it manipulates AKM suites.
The only thing it currently does on it before calling nl80211 is a
le32_to_cpu. IIUC the raw value (before applying le32_to_cpu) comes from
chip/firmware:
<interrupt>
 qtnf_shm_ipc_irq_handler
  <some callbacks chains>
   qtnf_pcie_control_rx_callback
    qtnf_trans_handle_rx_ctl_packet
     qtnf_trans_event_enqueue => queue skb to processing queue

qtnf_event_work_handler <= dequeue corresponding skb to process
 qtnf_event_process_skb
  qtnf_event_parse
   qtnf_event_handle_external_auth
    cfg80211_external_auth_request => sends NL80211_CMD_EXTERNAL_AUTH

There is no cast to big endian on AKM suite at any point in this chain, but
there are plenty of leXX_to_cpu, so I assume the chip/its firmware sends
its data in little endian. Then, since the be32 conversion is _needed_ with
current wpa_supplicant, I wonder if it works at all in current state, so I
did not modify it. Or has it been tested with another supplicant (iwd ?)
which handles WLAN_AKM_SUITE_SAE differently ? Opinions (and even some
testing) are welcome for this driver, since I do not have the corresponding
hardware.

[1] https://lore.kernel.org/linux-wireless/87a5uatfl1.fsf@kernel.org/

To: Johannes Berg <johannes@sipsolutions.net>
Cc: Ajay Singh <ajay.kathat@microchip.com>
Cc: Kalle Valo <kvalo@kernel.org>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: <linux-wireless@vger.kernel.org>
Cc: <linux-kernel@vger.kernel.org>
Cc: Igor Mitsyanko <imitsyanko@quantenna.com>
Cc: Sergey Matyukevich <geomatsi@gmail.com>

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
Alexis Lothoré (2):
      wifi: nl80211: force WLAN_AKM_SUITE_SAE in big endian in NL80211_CMD_EXTERNAL_AUTH
      wifi: wilc1000: remove AKM suite be32 conversion for external auth request

 drivers/net/wireless/microchip/wilc1000/cfg80211.c |  2 +-
 net/wireless/nl80211.c                             | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)
---
base-commit: a4c7842e88b0f7d937015e4588ea2a1dec33cf2c
change-id: 20240214-nl80211_fix_akm_suites_endianness-2750a5d7da83

Best regards,
-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 1/2] wifi: nl80211: force WLAN_AKM_SUITE_SAE in big endian in NL80211_CMD_EXTERNAL_AUTH

[Alexis Lothoré]

User-space supplicant (observed at least on wpa_supplicant) historically
parses the NL80211_ATTR_AKM_SUITES from the NL80211_CMD_EXTERNAL_AUTH
message as big endian _only_ when its value is WLAN_AKM_SUITE_SAE, while
processing anything else in host endian. This behavior makes any driver
relying on SAE external auth to switch AKM suite to big endian if it is
WLAN_AKM_SUITE_SAE. A fix bringing compatibility with both endianness
has been brought into wpa_supplicant, however we must keep compatibility
with older versions, while trying to reduce the occurences of this manual
conversion in wireless drivers.

Add the be32 conversion specifically on WLAN_AKM_SUITE_SAE in nl80211 layer
to keep compatibility with older wpa_supplicant versions.

Suggested-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
Only a few minor modifications from Johannes' initial suggestion:
- fixed unbalanced parenthesis
- slightly reworded the inlined documentation
---
 net/wireless/nl80211.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5f18cbf7cc3d..046ce0513d31 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -20136,9 +20136,26 @@ int cfg80211_external_auth_request(struct net_device *dev,
 	if (!hdr)
 		goto nla_put_failure;
 
+	/* Some historical mistakes in drivers <-> userspace interface (notably
+	 * between drivers and wpa_supplicant) led to a big-endian conversion
+	 * being needed on NL80211_ATTR_AKM_SUITES _only_ when its value is
+	 * WLAN_AKM_SUITE_SAE. This is now fixed on userspace side, but for the
+	 * benefit of older wpa_supplicant versions, send this particular value
+	 * in big-endian. Note that newer wpa_supplicant will also detect this
+	 * particular value in big endian still, so it all continues to work.
+	 */
+	if (params->key_mgmt_suite == WLAN_AKM_SUITE_SAE) {
+		if (nla_put_be32(msg, NL80211_ATTR_AKM_SUITES,
+				 cpu_to_be32(WLAN_AKM_SUITE_SAE)))
+			goto nla_put_failure;
+	} else {
+		if (nla_put_u32(msg, NL80211_ATTR_AKM_SUITES,
+				params->key_mgmt_suite))
+			goto nla_put_failure;
+	}
+
 	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
 	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, dev->ifindex) ||
-	    nla_put_u32(msg, NL80211_ATTR_AKM_SUITES, params->key_mgmt_suite) ||
 	    nla_put_u32(msg, NL80211_ATTR_EXTERNAL_AUTH_ACTION,
 			params->action) ||
 	    nla_put(msg, NL80211_ATTR_BSSID, ETH_ALEN, params->bssid) ||

-- 
2.43.0

Re: [PATCH 1/2] wifi: nl80211: force WLAN_AKM_SUITE_SAE in big endian in NL80211_CMD_EXTERNAL_AUTH

[Kalle Valo]

Alexis Lothoré <alexis.lothore@bootlin.com> writes:

> User-space supplicant (observed at least on wpa_supplicant) historically
> parses the NL80211_ATTR_AKM_SUITES from the NL80211_CMD_EXTERNAL_AUTH
> message as big endian _only_ when its value is WLAN_AKM_SUITE_SAE, while
> processing anything else in host endian. This behavior makes any driver
> relying on SAE external auth to switch AKM suite to big endian if it is
> WLAN_AKM_SUITE_SAE. A fix bringing compatibility with both endianness
> has been brought into wpa_supplicant, however we must keep compatibility
> with older versions, while trying to reduce the occurences of this manual
> conversion in wireless drivers.
>
> Add the be32 conversion specifically on WLAN_AKM_SUITE_SAE in nl80211 layer
> to keep compatibility with older wpa_supplicant versions.
>
> Suggested-by: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>

A pointer to the discussion would be nice to have:

Link: https://lore.kernel.org/all/09eeb7d4-c922-45ee-a1ac-59942153dbce@bootlin.com/

I assume Johannes can add that.

Alexis, thanks so much for working on this! This has been bugging me for
long but never found the time to investigate it.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 1/2] wifi: nl80211: force WLAN_AKM_SUITE_SAE in big endian in NL80211_CMD_EXTERNAL_AUTH

[Alexis Lothoré]

On 2/15/24 17:58, Kalle Valo wrote:
> Alexis Lothoré <alexis.lothore@bootlin.com> writes:
> 
>> User-space supplicant (observed at least on wpa_supplicant) historically
>> parses the NL80211_ATTR_AKM_SUITES from the NL80211_CMD_EXTERNAL_AUTH
>> message as big endian _only_ when its value is WLAN_AKM_SUITE_SAE, while
>> processing anything else in host endian. This behavior makes any driver
>> relying on SAE external auth to switch AKM suite to big endian if it is
>> WLAN_AKM_SUITE_SAE. A fix bringing compatibility with both endianness
>> has been brought into wpa_supplicant, however we must keep compatibility
>> with older versions, while trying to reduce the occurences of this manual
>> conversion in wireless drivers.
>>
>> Add the be32 conversion specifically on WLAN_AKM_SUITE_SAE in nl80211 layer
>> to keep compatibility with older wpa_supplicant versions.
>>
>> Suggested-by: Johannes Berg <johannes@sipsolutions.net>
>> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
> 
> A pointer to the discussion would be nice to have:
> 
> Link: https://lore.kernel.org/all/09eeb7d4-c922-45ee-a1ac-59942153dbce@bootlin.com/
> 
> I assume Johannes can add that.

Ah yes, indeed. Johannes, please let me know if you prefer me to resend it with
the link in the commit message.

> Alexis, thanks so much for working on this! This has been bugging me for
> long but never found the time to investigate it.

I'm glad to help, especially since I have the corresponding hardware. This
warning was on my radar, and your last complaint about remaining sparse warnings
in the wireless tree eventually triggered the action :)

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/2] wifi: nl80211: force WLAN_AKM_SUITE_SAE in big endian in NL80211_CMD_EXTERNAL_AUTH

[Kalle Valo]

Alexis Lothoré <alexis.lothore@bootlin.com> writes:

>> Alexis, thanks so much for working on this! This has been bugging me for
>> long but never found the time to investigate it.
>
> I'm glad to help, especially since I have the corresponding hardware. This
> warning was on my radar, and your last complaint about remaining sparse warnings
> in the wireless tree eventually triggered the action :)

Hah, at least there's one person who reads my complaints :)

I owe you a beer on this one, if we ever meet please remind me.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 2/2] wifi: wilc1000: remove AKM suite be32 conversion for external auth request

[Alexis Lothoré]

The driver currently raises the following sparse warning:
[...] cfg80211.c:360:42: warning: incorrect type in assignment (different
base types)
[...] cfg80211.c:360:42:    expected unsigned int key_mgmt_suite
[...] cfg80211.c:360:42:    got restricted __be32 [usertype]
  CHECK   drivers/net/wireless/microchip/wilc1000/netdev.c

This conversion was needed because historically the external supplicant
(observed with wpa_supplicant) expects AKM suite as big endian in
NL80211_CMD_EXTERNAL_AUTH message when the AKM suite is WLAN_AKM_SUITE_SAE.
This is not needed anymore:
- new (to be released) versions of wpa_supplicant now reads it in host
  endian _while_ keeping compatibility for older drivers
- for new drivers used with current/old wpa_supplicant, this conversion has
  been added to nl80211 to force big endian when the AKM suite is
  WLAN_AKM_SUITE_SAE

Remove this not-needed-anymore conversion to fix the sparse warning.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202308290615.lUTIgqUl-lkp@intel.com/
Tested-on: WILC1000 hwB SPI WILC_WIFI_FW_REL_16_1-13452
Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 drivers/net/wireless/microchip/wilc1000/cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/cfg80211.c b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
index f03fd15c0c97..cb564e2bad29 100644
--- a/drivers/net/wireless/microchip/wilc1000/cfg80211.c
+++ b/drivers/net/wireless/microchip/wilc1000/cfg80211.c
@@ -356,7 +356,7 @@ static int connect(struct wiphy *wiphy, struct net_device *dev,
 			memcpy(vif->auth.ssid.ssid, sme->ssid, sme->ssid_len);
 			vif->auth.ssid.ssid_len = sme->ssid_len;
 		}
-		vif->auth.key_mgmt_suite = cpu_to_be32(sme->crypto.akm_suites[0]);
+		vif->auth.key_mgmt_suite = sme->crypto.akm_suites[0];
 		ether_addr_copy(vif->auth.bssid, sme->bssid);
 		break;
 

-- 
2.43.0

Re: [PATCH 2/2] wifi: wilc1000: remove AKM suite be32 conversion for external auth request

[Kalle Valo]

Alexis Lothoré <alexis.lothore@bootlin.com> wrote:

> The driver currently raises the following sparse warning:
> [...] cfg80211.c:360:42: warning: incorrect type in assignment (different
> base types)
> [...] cfg80211.c:360:42:    expected unsigned int key_mgmt_suite
> [...] cfg80211.c:360:42:    got restricted __be32 [usertype]
>   CHECK   drivers/net/wireless/microchip/wilc1000/netdev.c
> 
> This conversion was needed because historically the external supplicant
> (observed with wpa_supplicant) expects AKM suite as big endian in
> NL80211_CMD_EXTERNAL_AUTH message when the AKM suite is WLAN_AKM_SUITE_SAE.
> This is not needed anymore:
> - new (to be released) versions of wpa_supplicant now reads it in host
>   endian _while_ keeping compatibility for older drivers
> - for new drivers used with current/old wpa_supplicant, this conversion has
>   been added to nl80211 to force big endian when the AKM suite is
>   WLAN_AKM_SUITE_SAE
> 
> Remove this not-needed-anymore conversion to fix the sparse warning.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202308290615.lUTIgqUl-lkp@intel.com/
> Tested-on: WILC1000 hwB SPI WILC_WIFI_FW_REL_16_1-13452
> Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>

Patch applied to wireless-next.git, thanks.

00413dd36414 wifi: wilc1000: remove AKM suite be32 conversion for external auth request

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240215-nl80211_fix_akm_suites_endianness-v1-2-57e902632f9d@bootlin.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 0/2] wifi: nl80211/wilc1000: force WLAN_AKM_SUITE_SAE to big endian in NL80211_CMD_EXTERNAL_AUTH

[Alexis Lothoré]

On 2/15/24 15:13, Alexis Lothoré wrote:
> This small series is the follow-up to discussions started around a sparse
> warning in wilc1000 driver ([1]) and implements the solution suggested by
> Johannes. It moves a historically needed conversion to be32 in nl80211 (in
> NL80211_CMD_EXTERNAL_AUTH, specifically on NL80211_ATTR_AKM_SUITES property
> _only_ when it is set to WLAN_AKM_SUITE_SAE) The user scenario affected by
> this update is a connect process on a WPA3-protected access point with
> authentication offloaded to user-space. Two drivers are affected by the
> update: wilc1000 and qtnfmac. wilc1000 case is handled by a small
> companion patch which also fixes the sparse warning.

Adding Claudio Beznea (co-maintainer for WILC), who got lost when I prepared the
series, sorry.

Also, my mail provider returns error 550 (No Such User Here) for quantenna
driver maintainer (<imitsyanko@quantenna.com>, taken from MAINTAINERS). I've
seen no recent activity from him on the ML, is he still around ?

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

wireless: orphan qfnfmac and plfxlc drivers?

[Kalle Valo]

(changing subject)

Alexis Lothoré <alexis.lothore@bootlin.com> writes:

> Also, my mail provider returns error 550 (No Such User Here) for quantenna
> driver maintainer (<imitsyanko@quantenna.com>, taken from MAINTAINERS). I've
> seen no recent activity from him on the ML, is he still around ?

I found a bounce for imitsyanko@quantenna.com from 2022 so I guess it's
time to orphan qtnfmac?

Also the purelife maintainer Srini Raju (CCed) is bouncing, that's
another candidate to orphan.

PURELIFI PLFXLC DRIVER
M:      Srinivasan Raju <srini.raju@purelifi.com>
L:      linux-wireless@vger.kernel.org
S:      Supported
F:      drivers/net/wireless/purelifi/plfxlc/

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches