Re: drivers/net/wireless/microchip/wilc1000/cfg80211.c:361:42: sparse: sparse: incorrect type in assignment (different base types)

Alexis Lothoré
2024-02-14T08:42:46Z

Hello Ajay, Kalle,

On 8/29/23 10:31, Kalle Valo wrote:
> Kalle Valo <kvalo@kernel.org> writes:
>
>> kernel test robot <lkp@intel.com> writes:

[...]

>>> sparse warnings: (new ones prefixed by >>)
>>>>> drivers/net/wireless/microchip/wilc1000/cfg80211.c:361:42: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned int key_mgmt_suite @@ got restricted __be32 [usertype] @@
>>> drivers/net/wireless/microchip/wilc1000/cfg80211.c:361:42: sparse: expected unsigned int key_mgmt_suite
>>> drivers/net/wireless/microchip/wilc1000/cfg80211.c:361:42: sparse: got restricted __be32 [usertype]
>>
>> Yeah, this is an old issue but we really should try to fix this,
>> especially so as I would like to make wireless code sparse warning free
>> in the near future. IIRC there were some problems with nl80211 interface
>> as well so this might not be simple fix still.
>
> For reference here's the old discussion:
>
> https://patchwork.kernel.org/project/linux-wireless/patch/20220720160302.231516-1-ajay.kathat@microchip.com/
>
> Any volunteers to help fix this? I would prefers fixes for issues like
> this compared to questionable random cleanups we always get.

I'm bumping this old thread because it looks like the sparse warning is still
present in WILC driver, and I would gladly help getting rid of it, but since
there's already been a fair amount of discussions around it, I am not sure what
is expected/what should be done. Here is my understanding so far:
- Ajay has proposed a patch ([1]) which indeed fixes the warning but involves
many casts
- Johannes and Jouni then gave details about the original issue leading to those
casts ([2]): wpa_supplicant somehow forces the AKM suites values to be be32 at
some point, while it should be treated in host endianness
- as pointed by Ajay, the corresponding fix has been made since then by Jouni in
wpa_supplicant ([3]). The fix make sure to handle key_mgmt_suite in host
endianness AND to keep compatibility with current drivers having the be32 fix. -
- It could have allowed to simply get rid of the all casts on AKM suites in
wilc driver ([4]), but then new kernel/drivers would break with existing
userspace, so it is not an option

Now, I see multiple options to fix the sparse warning:
- apply the same fix as for wpa_supplicant ([3]) in wilc driver (so basically,
become compatible with both endianness)
- apply the same fix as for wpa_supplicant ([3]), not in wilc but in nl80211
(may need to update not only wilc but any driver having trailing be32 cast on
AKM suites)
- take the initial quick but not-so-nice double cast fix and call it a day

What's your opinion on this ?

[1]
https://lore.kernel.org/linux-wireless/20220720160302.231516-1-ajay.kathat@microchip.com/
[2]
https://lore.kernel.org/linux-wireless/e677e3cd1b5c34146017a65f28fabdb673a91d23.camel@sipsolutions.net/
[3] https://w1.fi/cgit/hostap/commit/?id=8392ea9e75eacf30cb09671e463d9a37c3eadd6a
[4]
https://lore.kernel.org/linux-wireless/20230830042300.87184-1-ajay.kathat@microchip.com/

--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering