2023-01-16 20:38:24

by Doug Brown

[permalink] [raw]
Subject: [PATCH v3 0/4] wifi: libertas: IE handling fixes

This series implements two fixes for the libertas driver that restore
compatibility with modern wpa_supplicant versions, and adds (or at least
improves) support for WPS in the process.

1) Better handling of the RSN/WPA IE in association requests:
The previous logic was always just grabbing the first one, and didn't
handle multiple IEs properly, which wpa_supplicant adds nowadays.

2) Support for IEs in scan requests:
Modern wpa_supplicant always adds an "extended capabilities" IE,
which violates max_scan_ie_len in this driver. Go ahead and allow
scan IEs, and handle WPS based on the info that Dan provided.

These changes have been tested on a Marvell PXA168-based device with a
Marvell 88W8686 Wi-Fi chipset. I've confirmed that with these changes
applied, modern wpa_supplicant versions connect properly and WPS also
works correctly (tested with "wpa_cli -i wlan0 wps_pbc any").

Changes since V2:
- Add missing cpu_to_le16 as suggested by Simon Horman

Changes since V1 (which was a single patch linked here [1]):

- Switch to cfg80211_find_*_elem when looking for specific IEs,
resulting in cleaner/safer code.
- Use mrvl_ie_data struct for cleaner manipulation of TLV buffer, and
fix capitalization of the "data" member to avoid checkpatch warnings.
- Implement idea suggested by Dan to change max_scan_ie_len to be
nonzero and enable WPS support in probe requests while we're at it.
- Remove "Fixes:" tag; I'm not sure if it's still appropriate or not
with it depending on the capitalization fix.
- Clarify comments.

[1] https://lore.kernel.org/all/[email protected]/

Doug Brown (4):
wifi: libertas: fix capitalization in mrvl_ie_data struct
wifi: libertas: only add RSN/WPA IE in lbs_add_wpa_tlv
wifi: libertas: add new TLV type for WPS enrollee IE
wifi: libertas: add support for WPS enrollee IE in probe requests

drivers/net/wireless/marvell/libertas/cfg.c | 76 +++++++++++++++----
drivers/net/wireless/marvell/libertas/types.h | 3 +-
2 files changed, 65 insertions(+), 14 deletions(-)

--
2.34.1


2023-01-16 20:39:01

by Doug Brown

[permalink] [raw]
Subject: [PATCH v3 4/4] wifi: libertas: add support for WPS enrollee IE in probe requests

Add compatibility with WPS by passing on WPS enrollee information in
probe requests. Ignore other IEs supplied in the scan request. This also
has the added benefit of restoring compatibility with newer
wpa_supplicant versions that always add scan IEs. Previously, with
max_scan_ie_len set to 0, scans would always fail.

Suggested-by: Dan Williams <[email protected]>
Signed-off-by: Doug Brown <[email protected]>
---
drivers/net/wireless/marvell/libertas/cfg.c | 48 +++++++++++++++++++--
1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c
index 3f35dc7a1d7d..b700c213d10c 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -446,6 +446,41 @@ static int lbs_add_wpa_tlv(u8 *tlv, const u8 *ie, u8 ie_len)
return sizeof(struct mrvl_ie_header) + wpaie->datalen;
}

+/* Add WPS enrollee TLV
+ */
+#define LBS_MAX_WPS_ENROLLEE_TLV_SIZE \
+ (sizeof(struct mrvl_ie_header) \
+ + 256)
+
+static int lbs_add_wps_enrollee_tlv(u8 *tlv, const u8 *ie, size_t ie_len)
+{
+ struct mrvl_ie_data *wpstlv = (struct mrvl_ie_data *)tlv;
+ const struct element *wpsie;
+
+ /* Look for a WPS IE and add it to the probe request */
+ wpsie = cfg80211_find_vendor_elem(WLAN_OUI_MICROSOFT,
+ WLAN_OUI_TYPE_MICROSOFT_WPS,
+ ie, ie_len);
+ if (!wpsie)
+ return 0;
+
+ /* Convert the WPS IE to a TLV. The IE looks like this:
+ * u8 type (WLAN_EID_VENDOR_SPECIFIC)
+ * u8 len
+ * u8[] data
+ * but the TLV will look like this instead:
+ * __le16 type (TLV_TYPE_WPS_ENROLLEE)
+ * __le16 len
+ * u8[] data
+ */
+ wpstlv->header.type = cpu_to_le16(TLV_TYPE_WPS_ENROLLEE);
+ wpstlv->header.len = cpu_to_le16(wpsie->datalen);
+ memcpy(wpstlv->data, wpsie->data, wpsie->datalen);
+
+ /* Return the total number of bytes added to the TLV buffer */
+ return sizeof(struct mrvl_ie_header) + wpsie->datalen;
+}
+
/*
* Set Channel
*/
@@ -672,14 +707,15 @@ static int lbs_ret_scan(struct lbs_private *priv, unsigned long dummy,


/*
- * Our scan command contains a TLV, consting of a SSID TLV, a channel list
- * TLV and a rates TLV. Determine the maximum size of them:
+ * Our scan command contains a TLV, consisting of a SSID TLV, a channel list
+ * TLV, a rates TLV, and an optional WPS IE. Determine the maximum size of them:
*/
#define LBS_SCAN_MAX_CMD_SIZE \
(sizeof(struct cmd_ds_802_11_scan) \
+ LBS_MAX_SSID_TLV_SIZE \
+ LBS_MAX_CHANNEL_LIST_TLV_SIZE \
- + LBS_MAX_RATES_TLV_SIZE)
+ + LBS_MAX_RATES_TLV_SIZE \
+ + LBS_MAX_WPS_ENROLLEE_TLV_SIZE)

/*
* Assumes priv->scan_req is initialized and valid
@@ -728,6 +764,11 @@ static void lbs_scan_worker(struct work_struct *work)
/* add rates TLV */
tlv += lbs_add_supported_rates_tlv(tlv);

+ /* add optional WPS enrollee TLV */
+ if (priv->scan_req->ie && priv->scan_req->ie_len)
+ tlv += lbs_add_wps_enrollee_tlv(tlv, priv->scan_req->ie,
+ priv->scan_req->ie_len);
+
if (priv->scan_channel < priv->scan_req->n_channels) {
cancel_delayed_work(&priv->scan_work);
if (netif_running(priv->dev))
@@ -2114,6 +2155,7 @@ int lbs_cfg_register(struct lbs_private *priv)
int ret;

wdev->wiphy->max_scan_ssids = 1;
+ wdev->wiphy->max_scan_ie_len = 256;
wdev->wiphy->signal_type = CFG80211_SIGNAL_TYPE_MBM;

wdev->wiphy->interface_modes =
--
2.34.1

2023-01-16 20:41:19

by Doug Brown

[permalink] [raw]
Subject: [PATCH v3 3/4] wifi: libertas: add new TLV type for WPS enrollee IE

Add a define for the TLV type that will be used to add WPS enrollee
information to probe requests.

Suggested-by: Dan Williams <[email protected]>
Signed-off-by: Doug Brown <[email protected]>
---
drivers/net/wireless/marvell/libertas/types.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/marvell/libertas/types.h b/drivers/net/wireless/marvell/libertas/types.h
index 398e3272e85f..39deb2b8bc82 100644
--- a/drivers/net/wireless/marvell/libertas/types.h
+++ b/drivers/net/wireless/marvell/libertas/types.h
@@ -93,6 +93,7 @@ union ieee_phy_param_set {
#define TLV_TYPE_TSFTIMESTAMP (PROPRIETARY_TLV_BASE_ID + 19)
#define TLV_TYPE_RSSI_HIGH (PROPRIETARY_TLV_BASE_ID + 22)
#define TLV_TYPE_SNR_HIGH (PROPRIETARY_TLV_BASE_ID + 23)
+#define TLV_TYPE_WPS_ENROLLEE (PROPRIETARY_TLV_BASE_ID + 27)
#define TLV_TYPE_AUTH_TYPE (PROPRIETARY_TLV_BASE_ID + 31)
#define TLV_TYPE_MESH_ID (PROPRIETARY_TLV_BASE_ID + 37)
#define TLV_TYPE_OLD_MESH_ID (PROPRIETARY_TLV_BASE_ID + 291)
--
2.34.1

2023-01-17 20:20:03

by Dan Williams

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] wifi: libertas: add support for WPS enrollee IE in probe requests

On Mon, 2023-01-16 at 12:21 -0800, Doug Brown wrote:
> Add compatibility with WPS by passing on WPS enrollee information in
> probe requests. Ignore other IEs supplied in the scan request. This
> also
> has the added benefit of restoring compatibility with newer
> wpa_supplicant versions that always add scan IEs. Previously, with
> max_scan_ie_len set to 0, scans would always fail.
>
> Suggested-by: Dan Williams <[email protected]>
> Signed-off-by: Doug Brown <[email protected]>

Reviewed-by: Dan Williams <[email protected]>

(don't know if I can/should ack anything anymore, given I haven't
touched the driver in like 4 years...)

> ---
>  drivers/net/wireless/marvell/libertas/cfg.c | 48
> +++++++++++++++++++--
>  1 file changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/libertas/cfg.c
> b/drivers/net/wireless/marvell/libertas/cfg.c
> index 3f35dc7a1d7d..b700c213d10c 100644
> --- a/drivers/net/wireless/marvell/libertas/cfg.c
> +++ b/drivers/net/wireless/marvell/libertas/cfg.c
> @@ -446,6 +446,41 @@ static int lbs_add_wpa_tlv(u8 *tlv, const u8
> *ie, u8 ie_len)
>         return sizeof(struct mrvl_ie_header) + wpaie->datalen;
>  }
>  
> +/* Add WPS enrollee TLV
> + */
> +#define LBS_MAX_WPS_ENROLLEE_TLV_SIZE          \
> +       (sizeof(struct mrvl_ie_header)          \
> +        + 256)
> +
> +static int lbs_add_wps_enrollee_tlv(u8 *tlv, const u8 *ie, size_t
> ie_len)
> +{
> +       struct mrvl_ie_data *wpstlv = (struct mrvl_ie_data *)tlv;
> +       const struct element *wpsie;
> +
> +       /* Look for a WPS IE and add it to the probe request */
> +       wpsie = cfg80211_find_vendor_elem(WLAN_OUI_MICROSOFT,
> +                                        
> WLAN_OUI_TYPE_MICROSOFT_WPS,
> +                                         ie, ie_len);
> +       if (!wpsie)
> +               return 0;
> +
> +       /* Convert the WPS IE to a TLV. The IE looks like this:
> +        *   u8      type (WLAN_EID_VENDOR_SPECIFIC)
> +        *   u8      len
> +        *   u8[]    data
> +        * but the TLV will look like this instead:
> +        *   __le16  type (TLV_TYPE_WPS_ENROLLEE)
> +        *   __le16  len
> +        *   u8[]    data
> +        */
> +       wpstlv->header.type = cpu_to_le16(TLV_TYPE_WPS_ENROLLEE);
> +       wpstlv->header.len = cpu_to_le16(wpsie->datalen);
> +       memcpy(wpstlv->data, wpsie->data, wpsie->datalen);
> +
> +       /* Return the total number of bytes added to the TLV buffer
> */
> +       return sizeof(struct mrvl_ie_header) + wpsie->datalen;
> +}
> +
>  /*
>   * Set Channel
>   */
> @@ -672,14 +707,15 @@ static int lbs_ret_scan(struct lbs_private
> *priv, unsigned long dummy,
>  
>  
>  /*
> - * Our scan command contains a TLV, consting of a SSID TLV, a
> channel list
> - * TLV and a rates TLV. Determine the maximum size of them:
> + * Our scan command contains a TLV, consisting of a SSID TLV, a
> channel list
> + * TLV, a rates TLV, and an optional WPS IE. Determine the maximum
> size of them:
>   */
>  #define LBS_SCAN_MAX_CMD_SIZE                  \
>         (sizeof(struct cmd_ds_802_11_scan)      \
>          + LBS_MAX_SSID_TLV_SIZE                \
>          + LBS_MAX_CHANNEL_LIST_TLV_SIZE        \
> -        + LBS_MAX_RATES_TLV_SIZE)
> +        + LBS_MAX_RATES_TLV_SIZE               \
> +        + LBS_MAX_WPS_ENROLLEE_TLV_SIZE)
>  
>  /*
>   * Assumes priv->scan_req is initialized and valid
> @@ -728,6 +764,11 @@ static void lbs_scan_worker(struct work_struct
> *work)
>         /* add rates TLV */
>         tlv += lbs_add_supported_rates_tlv(tlv);
>  
> +       /* add optional WPS enrollee TLV */
> +       if (priv->scan_req->ie && priv->scan_req->ie_len)
> +               tlv += lbs_add_wps_enrollee_tlv(tlv, priv->scan_req-
> >ie,
> +                                               priv->scan_req-
> >ie_len);
> +
>         if (priv->scan_channel < priv->scan_req->n_channels) {
>                 cancel_delayed_work(&priv->scan_work);
>                 if (netif_running(priv->dev))
> @@ -2114,6 +2155,7 @@ int lbs_cfg_register(struct lbs_private *priv)
>         int ret;
>  
>         wdev->wiphy->max_scan_ssids = 1;
> +       wdev->wiphy->max_scan_ie_len = 256;
>         wdev->wiphy->signal_type = CFG80211_SIGNAL_TYPE_MBM;
>  
>         wdev->wiphy->interface_modes =