Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 13 Oct 2022 12:40:11 +0300
From:   Jouni Malinen <j@w1.fi>
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     Ajay.Kathat@microchip.com, kvalo@kernel.org,
        linux-wireless@vger.kernel.org, Claudiu.Beznea@microchip.com,
        Sripad.Balwadgi@microchip.com, lkp@intel.com,
        hostap@lists.infradead.org
Subject: Re: [PATCH 1/8] wifi: wilc1000: fix incorrect type assignment sparse
 warning
Message-ID: <20221013094011.GB3862@w1.fi>
References: <20220720160302.231516-1-ajay.kathat@microchip.com>
 <87v8rik8vp.fsf@kernel.org>
 <2b432ae1-48fc-5a70-0afe-2b9f788f14e4@microchip.com>
 <e677e3cd1b5c34146017a65f28fabdb673a91d23.camel@sipsolutions.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <e677e3cd1b5c34146017a65f28fabdb673a91d23.camel@sipsolutions.net>
Precedence: bulk

On Thu, Oct 13, 2022 at 09:39:56AM +0200, Johannes Berg wrote:
> On Wed, 2022-07-27 at 17:32 +0000, Ajay.Kathat@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. 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.. Furthermore, it
looks like this has missed the new RSN_AUTH_KEY_MGMT_SAE_EXT_KEY
selector update as well.

> 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 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. It's always
confusing to review the driver_nl80211* changes for this area since I
have to convince myself each time that this really is a native byte
order u32 value.. That sme_external_auth_trigger() case is outside
driver_nl80211* so it did not get the same detail of review
unfortunately.

-- 
Jouni Malinen                                            PGP id EFC895FA