[PATCH 1/8] wifi: wilc1000: fix incorrect type assignment sparse warning

[Johannes Berg]

On Thu, 2022-10-13 at 12:40 +0300, Jouni Malinen wrote:
> On Thu, Oct 13, 2022 at 09:39:56AM +0200, Johannes Berg wrote:
> > On Wed, 2022-07-27 at 17:32 +0000, Ajay.Kathat at microchip.com wrote:
> > > I think, there is an another way to handle this issue. 'key_mgmt_suite' 
> > > element in 'cfg80211_external_auth_params' struct should be converted to 
> > > '__be32' type(like below code snippet) because wpa_s expects the value 
> > > in big-endian format . After this change, the type case can be avoided. 
> > > Though I am not sure if these changes can have impact on other driver.
> > > 
> > 
> > Ugh. I think maybe it would be better to fix wpa_supplicant?
> 
> Assuming you are referring to what sme_external_auth_trigger() does,
> yes, the use of RSN_SELECTOR_GET() there on an unsigned int variable is
> clearly wrong.

Right, that's what I meant.

> As a workaround, it could be modified to accept both the
> big endian and host byte order since the risk of conflicting with a
> vendor specific AKM suite here would be very minimal.. 

True.

> > Thing is, we use the NL80211_ATTR_AKM_SUITES attribute here for it, and
> > even wpa_supplicant mostly uses that in host endian:
> 
> By the way, that use of a u32 attribute (or an array of such) in an
> interface is quite confusing for suite selectors (both AKM and cipher)
> since a suite selector is really a four octet binary string that starts
> with three octet OUI that is followed with a single octet integer
> value. nl80211.h does not seem to provide any documentation on what the
> exact value is supposed to be.

I guess we should fix the documentation then, but basically, for a
selector of

 O-U-I:D

we use

 (O << 24) | (U << 16) | (I << 8) | D

for the ID. If we had consistently used be32 then that'd actually at
least match the four octet binary string and it'd be irrelevant, but ...
we clearly didn't.

> I can understand use of u32 and native byte order as an implementation
> internal thing, but it should not really be used in an interface since
> it is just waiting for this type of issues to show up.

Yeah, can't really disagree with that, though I think it's a bit late
now, unfortunately.

johannes