On Sat, Jan 07, 2023 at 05:30:14PM -0800, Doug Brown wrote:
> The existing code only converts the first IE to a TLV, but it returns a
> value that takes the length of all IEs into account. When there is more
> than one IE (which happens with modern wpa_supplicant versions for
> example), the returned length is too long and extra junk TLVs get sent
> to the firmware, resulting in an association failure.
>
> Fix this by finding the first RSN or WPA IE and only adding that. This
> has the extra benefit of working properly if the RSN/WPA IE isn't the
> first one in the IE buffer.
>
> While we're at it, clean up the code to use the available structs like
> the other lbs_add_* functions instead of directly manipulating the TLV
> buffer.
>
> Signed-off-by: Doug Brown <doug@xxxxxxxxxxxxx>
> ---
> drivers/net/wireless/marvell/libertas/cfg.c | 28 +++++++++++++--------
> 1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/libertas/cfg.c b/drivers/net/wireless/marvell/libertas/cfg.c
> index 3e065cbb0af9..5cd78fefbe4c 100644
> --- a/drivers/net/wireless/marvell/libertas/cfg.c
> +++ b/drivers/net/wireless/marvell/libertas/cfg.c
> @@ -416,10 +416,20 @@ static int lbs_add_cf_param_tlv(u8 *tlv)
>
> static int lbs_add_wpa_tlv(u8 *tlv, const u8 *ie, u8 ie_len)
> {
> - size_t tlv_len;
> + struct mrvl_ie_data *wpatlv = (struct mrvl_ie_data *)tlv;
> + const struct element *wpaie;
> +
> + /* Find the first RSN or WPA IE to use */
> + wpaie = cfg80211_find_elem(WLAN_EID_RSN, ie, ie_len);
> + if (!wpaie)
> + wpaie = cfg80211_find_vendor_elem(WLAN_OUI_MICROSOFT,
> + WLAN_OUI_TYPE_MICROSOFT_WPA,
> + ie, ie_len);
> + if (!wpaie || wpaie->datalen > 128)
> + return 0;
>
> /*
> - * We need just convert an IE to an TLV. IEs use u8 for the header,
> + * Convert the found IE to a TLV. IEs use u8 for the header,
> * u8 type
> * u8 len
> * u8[] data
> @@ -428,14 +438,12 @@ static int lbs_add_wpa_tlv(u8 *tlv, const u8 *ie, u8 ie_len)
> * __le16 len
> * u8[] data
> */
> - *tlv++ = *ie++;
> - *tlv++ = 0;
> - tlv_len = *tlv++ = *ie++;
> - *tlv++ = 0;
> - while (tlv_len--)
> - *tlv++ = *ie++;
> - /* the TLV is two bytes larger than the IE */
> - return ie_len + 2;
> + wpatlv->header.type = wpaie->id;
> + wpatlv->header.len = wpaie->datalen;
Hi Doug,
For correctness should type and len be converted to little endian,
f.e. using cpu_to_le16() ?
Likewise in patch 4/4.
> + memcpy(wpatlv->data, wpaie->data, wpaie->datalen);
> +
> + /* Return the total number of bytes added to the TLV buffer */
> + return sizeof(struct mrvl_ie_header) + wpaie->datalen;
> }
>
> /*
> --
> 2.34.1
>
