On Mon, Jan 16, 2023 at 12:21:24PM -0800, Doug Brown wrote: > [You don't often get email from doug@xxxxxxxxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > 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..3f35dc7a1d7d 100644 > --- a/drivers/net/wireless/marvell/libertas/cfg.c > +++ b/drivers/net/wireless/marvell/libertas/cfg.c ... > @@ -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 = cpu_to_le16(wpaie->id); > + wpatlv->header.len = cpu_to_le16(wpaie->datalen); > + memcpy(wpatlv->data, wpaie->data, wpaie->datalen); Hi Doug, Thanks for fixing the endiness issues with cpu_to_le16() This part looks good to me now. Likewise for patch 4/4. One suggestion I have, which is probably taking things to far, is a helper for what seems to be repeated code-pattern. But I don't feel strongly about that. > + > + /* Return the total number of bytes added to the TLV buffer */ > + return sizeof(struct mrvl_ie_header) + wpaie->datalen; > } > > /* > -- > 2.34.1 >
