Message-ID: <09eeb7d4-c922-45ee-a1ac-59942153dbce@bootlin.com>
Date: Wed, 14 Feb 2024 09:42:43 +0100
From: Alexis Lothoré <alexis.lothore@...tlin.com>
To: Kalle Valo <kvalo@...nel.org>, kernel test robot <lkp@...el.com>
Cc: Ajay Singh <ajay.kathat@...rochip.com>, oe-kbuild-all@...ts.linux.dev,
 linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org,
 Johannes Berg <johannes@...solutions.net>
Subject: Re: drivers/net/wireless/microchip/wilc1000/cfg80211.c:361:42:
 sparse: sparse: incorrect type in assignment (different base types)

Hello Ajay, Kalle,

On 8/29/23 10:31, Kalle Valo wrote:
> Kalle Valo <kvalo@...nel.org> writes:
> 
>> kernel test robot <lkp@...el.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
https://bootlin.com

Powered by blists - more mailing lists