| Message ID | 20240215-nl80211_fix_akm_suites_endianness-v1-1-57e902632f9d@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: nl80211/wilc1000: force WLAN_AKM_SUITE_SAE to big endian in NL80211_CMD_EXTERNAL_AUTH | expand |
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.
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é <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.
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) ||
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(-)