This series builds around the patch from Hector Martin which enables
SAE offload for WCC vendor devices like the BCM4378.
Other patches involve exposing firmware interface layer functions to
per-vendor modules and allowing per-vendor feature detection or override.
This series applies to the main branch of the wireless-next repository.
Arend van Spriel (3):
wifi: brcmfmac: export firmware interface functions
wifi: brcmfmac: add per-vendor feature detection callback
wifi: brcmfmac: move feature overrides before feature_disable
Hector Martin (1):
wifi: brcmfmac: cfg80211: Use WSEC to set SAE password
.../broadcom/brcm80211/brcmfmac/bca/core.c | 8 ++
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 60 ++++-----
.../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 +
.../broadcom/brcm80211/brcmfmac/core.c | 2 +-
.../broadcom/brcm80211/brcmfmac/cyw/core.c | 28 ++++
.../broadcom/brcm80211/brcmfmac/feature.c | 11 +-
.../broadcom/brcm80211/brcmfmac/fwil.c | 116 ++--------------
.../broadcom/brcm80211/brcmfmac/fwil.h | 127 +++++++++++++++---
.../broadcom/brcm80211/brcmfmac/fwil_types.h | 2 +-
.../broadcom/brcm80211/brcmfmac/fwvid.h | 25 ++++
.../broadcom/brcm80211/brcmfmac/wcc/core.c | 9 ++
11 files changed, 223 insertions(+), 167 deletions(-)
base-commit: 968509128207f122d7177ffb6ff51c9c6fa7e13d
--
2.32.0
On Wed, Jan 3, 2024 at 4:57 AM Arend van Spriel
<[email protected]> wrote:
>
> From: Hector Martin <[email protected]>
>
> Using the WSEC command instead of sae_password seems to be the supported
> mechanism on newer firmware, and also how the brcmdhd driver does it.
>
> According to user reports [1], the sae_password codepath doesn't actually
> work on machines with Cypress chips anyway, so no harm in removing it.
>
> This makes WPA3 work with iwd, or with wpa_supplicant pending a support
> patchset [2].
>
> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/
> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html
>
> Signed-off-by: Hector Martin <[email protected]>
> Reviewed-by: Neal Gompa <[email protected]>
> [[email protected]: use multi-vendor framework]
> Signed-off-by: Arend van Spriel <[email protected]>
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 56 ++++++++-----------
> .../broadcom/brcm80211/brcmfmac/cfg80211.h | 2 +
> .../broadcom/brcm80211/brcmfmac/cyw/core.c | 28 ++++++++++
> .../broadcom/brcm80211/brcmfmac/fwil.c | 1 +
> .../broadcom/brcm80211/brcmfmac/fwil_types.h | 2 +-
> .../broadcom/brcm80211/brcmfmac/fwvid.h | 13 +++++
> .../broadcom/brcm80211/brcmfmac/wcc/core.c | 9 +++
> 7 files changed, 76 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 4df3d53bf5d3..03e5d4f986ca 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -32,6 +32,7 @@
> #include "vendor.h"
> #include "bus.h"
> #include "common.h"
> +#include "fwvid.h"
>
> #define BRCMF_SCAN_IE_LEN_MAX 2048
>
> @@ -1687,52 +1688,39 @@ static u16 brcmf_map_fw_linkdown_reason(const struct brcmf_event_msg *e)
> return reason;
> }
>
> -static int brcmf_set_pmk(struct brcmf_if *ifp, const u8 *pmk_data, u16 pmk_len)
> +int brcmf_set_wsec(struct brcmf_if *ifp, const u8 *key, u16 key_len, u16 flags)
> {
> struct brcmf_pub *drvr = ifp->drvr;
> struct brcmf_wsec_pmk_le pmk;
> int err;
>
> + if (key_len > sizeof(pmk.key)) {
> + bphy_err(drvr, "key must be less than %zu bytes\n",
> + sizeof(pmk.key));
> + return -EINVAL;
> + }
> +
> memset(&pmk, 0, sizeof(pmk));
>
> - /* pass pmk directly */
> - pmk.key_len = cpu_to_le16(pmk_len);
> - pmk.flags = cpu_to_le16(0);
> - memcpy(pmk.key, pmk_data, pmk_len);
> + /* pass key material directly */
> + pmk.key_len = cpu_to_le16(key_len);
> + pmk.flags = cpu_to_le16(flags);
> + memcpy(pmk.key, key, key_len);
>
> - /* store psk in firmware */
> + /* store key material in firmware */
> err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_WSEC_PMK,
> &pmk, sizeof(pmk));
> if (err < 0)
> bphy_err(drvr, "failed to change PSK in firmware (len=%u)\n",
> - pmk_len);
> + key_len);
>
> return err;
> }
> +BRCMF_EXPORT_SYMBOL_GPL(brcmf_set_wsec);
>
> -static int brcmf_set_sae_password(struct brcmf_if *ifp, const u8 *pwd_data,
> - u16 pwd_len)
> +static int brcmf_set_pmk(struct brcmf_if *ifp, const u8 *pmk_data, u16 pmk_len)
> {
> - struct brcmf_pub *drvr = ifp->drvr;
> - struct brcmf_wsec_sae_pwd_le sae_pwd;
> - int err;
> -
> - if (pwd_len > BRCMF_WSEC_MAX_SAE_PASSWORD_LEN) {
> - bphy_err(drvr, "sae_password must be less than %d\n",
> - BRCMF_WSEC_MAX_SAE_PASSWORD_LEN);
> - return -EINVAL;
> - }
> -
> - sae_pwd.key_len = cpu_to_le16(pwd_len);
> - memcpy(sae_pwd.key, pwd_data, pwd_len);
> -
> - err = brcmf_fil_iovar_data_set(ifp, "sae_password", &sae_pwd,
> - sizeof(sae_pwd));
> - if (err < 0)
> - bphy_err(drvr, "failed to set SAE password in firmware (len=%u)\n",
> - pwd_len);
> -
> - return err;
> + return brcmf_set_wsec(ifp, pmk_data, pmk_len, 0);
> }
>
> static void brcmf_link_down(struct brcmf_cfg80211_vif *vif, u16 reason,
> @@ -2503,8 +2491,7 @@ brcmf_cfg80211_connect(struct wiphy *wiphy, struct net_device *ndev,
> bphy_err(drvr, "failed to clean up user-space RSNE\n");
> goto done;
> }
> - err = brcmf_set_sae_password(ifp, sme->crypto.sae_pwd,
> - sme->crypto.sae_pwd_len);
> + err = brcmf_fwvid_set_sae_password(ifp, &sme->crypto);
> if (!err && sme->crypto.psk)
> err = brcmf_set_pmk(ifp, sme->crypto.psk,
> BRCMF_WSEC_MAX_PSK_LEN);
> @@ -5252,8 +5239,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
> if (crypto->sae_pwd) {
> brcmf_dbg(INFO, "using SAE offload\n");
> profile->use_fwauth |= BIT(BRCMF_PROFILE_FWAUTH_SAE);
> - err = brcmf_set_sae_password(ifp, crypto->sae_pwd,
> - crypto->sae_pwd_len);
> + err = brcmf_fwvid_set_sae_password(ifp, crypto);
> if (err < 0)
> goto exit;
> }
> @@ -5360,10 +5346,12 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> msleep(400);
>
> if (profile->use_fwauth != BIT(BRCMF_PROFILE_FWAUTH_NONE)) {
> + struct cfg80211_crypto_settings crypto = {};
> +
> if (profile->use_fwauth & BIT(BRCMF_PROFILE_FWAUTH_PSK))
> brcmf_set_pmk(ifp, NULL, 0);
> if (profile->use_fwauth & BIT(BRCMF_PROFILE_FWAUTH_SAE))
> - brcmf_set_sae_password(ifp, NULL, 0);
> + brcmf_fwvid_set_sae_password(ifp, &crypto);
> profile->use_fwauth = BIT(BRCMF_PROFILE_FWAUTH_NONE);
> }
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> index 0e1fa3f0dea2..dc3a6a537507 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> @@ -468,4 +468,6 @@ void brcmf_set_mpc(struct brcmf_if *ndev, int mpc);
> void brcmf_abort_scanning(struct brcmf_cfg80211_info *cfg);
> void brcmf_cfg80211_free_netdev(struct net_device *ndev);
>
> +int brcmf_set_wsec(struct brcmf_if *ifp, const u8 *key, u16 key_len, u16 flags);
> +
> #endif /* BRCMFMAC_CFG80211_H */
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> index b75652ba9359..24670497f1a4 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> @@ -7,6 +7,7 @@
> #include <core.h>
> #include <bus.h>
> #include <fwvid.h>
> +#include <fwil.h>
>
> #include "vops.h"
>
> @@ -21,7 +22,34 @@ static void brcmf_cyw_detach(struct brcmf_pub *drvr)
> pr_err("%s: executing\n", __func__);
> }
>
> +static int brcmf_cyw_set_sae_pwd(struct brcmf_if *ifp,
> + struct cfg80211_crypto_settings *crypto)
> +{
> + struct brcmf_pub *drvr = ifp->drvr;
> + struct brcmf_wsec_sae_pwd_le sae_pwd;
> + u16 pwd_len = crypto->sae_pwd_len;
> + int err;
> +
> + if (pwd_len > BRCMF_WSEC_MAX_SAE_PASSWORD_LEN) {
> + bphy_err(drvr, "sae_password must be less than %d\n",
> + BRCMF_WSEC_MAX_SAE_PASSWORD_LEN);
> + return -EINVAL;
> + }
> +
> + sae_pwd.key_len = cpu_to_le16(pwd_len);
> + memcpy(sae_pwd.key, crypto->sae_pwd, pwd_len);
> +
> + err = brcmf_fil_iovar_data_set(ifp, "sae_password", &sae_pwd,
> + sizeof(sae_pwd));
> + if (err < 0)
> + bphy_err(drvr, "failed to set SAE password in firmware (len=%u)\n",
> + pwd_len);
> +
> + return err;
> +}
> +
> const struct brcmf_fwvid_ops brcmf_cyw_ops = {
> .attach = brcmf_cyw_attach,
> .detach = brcmf_cyw_detach,
> + .set_sae_password = brcmf_cyw_set_sae_pwd,
> };
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
> index 2aec7d2abd52..bc1c6b5a6e31 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil.c
> @@ -211,6 +211,7 @@ brcmf_fil_iovar_data_set(struct brcmf_if *ifp, const char *name, const void *dat
> mutex_unlock(&drvr->proto_block);
> return err;
> }
> +BRCMF_EXPORT_SYMBOL_GPL(brcmf_fil_iovar_data_set);
>
> s32
> brcmf_fil_iovar_data_get(struct brcmf_if *ifp, const char *name, void *data,
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> index 9d248ba1c0b2..e74a23e11830 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> @@ -584,7 +584,7 @@ struct brcmf_wsec_key_le {
> struct brcmf_wsec_pmk_le {
> __le16 key_len;
> __le16 flags;
> - u8 key[2 * BRCMF_WSEC_MAX_PSK_LEN + 1];
> + u8 key[BRCMF_WSEC_MAX_SAE_PASSWORD_LEN];
> };
>
> /**
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwvid.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwvid.h
> index 17fbdbb76f51..d9fc76b46db9 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwvid.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwvid.h
> @@ -6,6 +6,7 @@
> #define FWVID_H_
>
> #include "firmware.h"
> +#include "cfg80211.h"
>
> struct brcmf_pub;
> struct brcmf_if;
> @@ -14,6 +15,7 @@ struct brcmf_fwvid_ops {
> int (*attach)(struct brcmf_pub *drvr);
> void (*detach)(struct brcmf_pub *drvr);
> void (*feat_attach)(struct brcmf_if *ifp);
> + int (*set_sae_password)(struct brcmf_if *ifp, struct cfg80211_crypto_settings *crypto);
> };
>
> /* exported functions */
> @@ -56,4 +58,15 @@ static inline void brcmf_fwvid_feat_attach(struct brcmf_if *ifp)
> vops->feat_attach(ifp);
> }
>
> +static inline int brcmf_fwvid_set_sae_password(struct brcmf_if *ifp,
> + struct cfg80211_crypto_settings *crypto)
> +{
> + const struct brcmf_fwvid_ops *vops = ifp->drvr->vops;
> +
> + if (!vops || !vops->set_sae_password)
> + return -EOPNOTSUPP;
> +
> + return vops->set_sae_password(ifp, crypto);
> +}
> +
> #endif /* FWVID_H_ */
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/wcc/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/wcc/core.c
> index 5573a47766ad..2d8f80bd7382 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/wcc/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/wcc/core.c
> @@ -7,6 +7,7 @@
> #include <core.h>
> #include <bus.h>
> #include <fwvid.h>
> +#include <fwil.h>
>
> #include "vops.h"
>
> @@ -21,7 +22,15 @@ static void brcmf_wcc_detach(struct brcmf_pub *drvr)
> pr_debug("%s: executing\n", __func__);
> }
>
> +static int brcmf_wcc_set_sae_pwd(struct brcmf_if *ifp,
> + struct cfg80211_crypto_settings *crypto)
> +{
> + return brcmf_set_wsec(ifp, crypto->sae_pwd, crypto->sae_pwd_len,
> + BRCMF_WSEC_PASSPHRASE);
> +}
> +
> const struct brcmf_fwvid_ops brcmf_wcc_ops = {
> .attach = brcmf_wcc_attach,
> .detach = brcmf_wcc_detach,
> + .set_sae_password = brcmf_wcc_set_sae_pwd,
> };
> --
> 2.32.0
>
Though my R-b tag is there already from the previous version, this
patch still looks good to me to land.
Thanks for adapting it for the multi-vendor framework. :)
--
真実はいつも一つ！/ Always, there's only one truth!
Hi Arend,
> Adding a .feat_attach() callback allowing per-vendor overrides
> of the driver feature flags. In this patch the callback is only
> provided by BCA vendor to disable SAE feature as it has not been
> confirmed yet. BCA chips generally do not have the in-driver
> supplicant (idsup) feature so they rely on NL80211_CMD_EXTERNAL_AUTH
> to trigger user-space authentication.
is there any way to tell if idsup is supported (firmware string or
some other mechanism). The just disabling and not implementing support
for external_auth is really having this stuck in WPA2 legacy world.
The whole out-dated firmware game on Raspberry Pi is already a mess.
My problem is really that WiFi 6E and WiFi 7 are essentially mandating
WPA3/SAE only support as soon as you move into 6 GHz operation and the
errors presented to the user are not decodable if you don’t have a
good understanding on what WiFi certification mandates.
Regards
Marcel
Arend van Spriel <[email protected]> wrote:
> From: Hector Martin <[email protected]>
>
> Using the WSEC command instead of sae_password seems to be the supported
> mechanism on newer firmware, and also how the brcmdhd driver does it.
>
> According to user reports [1], the sae_password codepath doesn't actually
> work on machines with Cypress chips anyway, so no harm in removing it.
>
> This makes WPA3 work with iwd, or with wpa_supplicant pending a support
> patchset [2].
>
> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/
> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html
>
> Signed-off-by: Hector Martin <[email protected]>
> Reviewed-by: Neal Gompa <[email protected]>
> [[email protected]: use multi-vendor framework]
> Signed-off-by: Arend van Spriel <[email protected]>
Should we also update the commit message to match the code? We are not removing
the Cypress codepath anymore, right?
I can do the update, just let me know what to add. Preferably something I can
just copy&paste.
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 1/18/2024 2:14 PM, Kalle Valo wrote:
> Arend van Spriel <[email protected]> wrote:
>
>> From: Hector Martin <[email protected]>
>>
>> Using the WSEC command instead of sae_password seems to be the supported
>> mechanism on newer firmware, and also how the brcmdhd driver does it.
>>
>> According to user reports [1], the sae_password codepath doesn't actually
>> work on machines with Cypress chips anyway, so no harm in removing it.
Replace above paragraph with:
The existing firmware mechanism intended for (some) Cypress chips has
been separated from the new firmware mechanism using the multi-vendor
framework. Depending on the device it will select the appropriate
firmware mechanism.
Regards,
Arend
>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support
>> patchset [2].
>>
>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/
>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html
>>
>> Signed-off-by: Hector Martin <[email protected]>
>> Reviewed-by: Neal Gompa <[email protected]>
>> [[email protected]: use multi-vendor framework]
>> Signed-off-by: Arend van Spriel <[email protected]>
>
> Should we also update the commit message to match the code? We are not removing
> the Cypress codepath anymore, right?
>
> I can do the update, just let me know what to add. Preferably something I can
> just copy&paste.
>
Arend van Spriel <[email protected]> wrote:
> With multi-vendor support the vendor-specific module may need to use
> the firmware interface functions so export them using the macro
> BRCMF_EXPORT_SYMBOL_GPL() which exports them to driver namespace.
>
> Signed-off-by: Arend van Spriel <[email protected]>
4 patches applied to wireless-next.git, thanks.
31343230abb1 wifi: brcmfmac: export firmware interface functions
14e1391b7102 wifi: brcmfmac: add per-vendor feature detection callback
ba4d4726335c wifi: brcmfmac: move feature overrides before feature_disable
9f7861c56b51 wifi: brcmfmac: cfg80211: Use WSEC to set SAE password
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches