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

[Johannes Berg <johannes@xxxxxxxxxxxxxxxx>]

On Wed, 2022-07-27 at 17:32 +0000, Ajay.Kathat@xxxxxxxxxxxxx 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?

Thing is, we use the NL80211_ATTR_AKM_SUITES attribute here for it, and
even wpa_supplicant mostly uses that in host endian:

        num_suites = wpa_key_mgmt_to_suites(params->key_mgmt_suites,
                                            suites, ARRAY_SIZE(suites));
...
                 nla_put(msg, NL80211_ATTR_AKM_SUITES, num_suites * sizeof(u32),
                         suites))

with

static int wpa_key_mgmt_to_suites(unsigned int key_mgmt_suites, u32 suites[],
                                  int max_suites)
{
        int num_suites = 0;

#define __AKM(a, b) \
        if (num_suites < max_suites && \
            (key_mgmt_suites & (WPA_KEY_MGMT_ ## a))) \
                suites[num_suites++] = (RSN_AUTH_KEY_MGMT_ ## b)
        __AKM(IEEE8021X, UNSPEC_802_1X);




and also

                case WPA_KEY_MGMT_FT_FILS_SHA384:
                        mgmt = RSN_AUTH_KEY_MGMT_FT_FILS_SHA384;
                        break;
                case WPA_KEY_MGMT_PSK:
                default:
                        mgmt = RSN_AUTH_KEY_MGMT_PSK_OVER_802_1X;
                        break;
                }
                wpa_printf(MSG_DEBUG, "  * akm=0x%x", mgmt);
                if (nla_put_u32(msg, NL80211_ATTR_AKM_SUITES, mgmt))
                        return -1;


Now those are all userspace->kernel direction, but also:


        wiphy_info_akm_suites(info, tb[NL80211_ATTR_AKM_SUITES]);

which eventually uses

static unsigned int get_akm_suites_info(struct nlattr *tb)
{
        int i, num;
        unsigned int key_mgmt = 0;
        u32 *akms;

        if (!tb)
                return 0;

        num = nla_len(tb) / sizeof(u32);
        akms = nla_data(tb);
        for (i = 0; i < num; i++) {
                switch (akms[i]) {
                case RSN_AUTH_KEY_MGMT_UNSPEC_802_1X:


so again it's in native endianness.


So IMHO

commit 5ff39c1380d9dea794c5102c0b6d11d1b1e23ad0
Author: Sunil Dutt <usdutt@xxxxxxxxxxxxxx>
Date:   Thu Feb 1 17:01:28 2018 +0530

    SAE: Support external authentication offload for driver-SME cases


is the problem there in that it assumed big endian for a value that's
clearly not meant to be big endian. And what garbage out-of-tree drivers
do we don't know ...

Even in the kernel, we have


static int
qtnf_event_handle_external_auth(struct qtnf_vif *vif,
                                const struct qlink_event_external_auth *ev,
                                u16 len)
{
        struct cfg80211_external_auth_params auth = {0};
[...]
        auth.key_mgmt_suite = le32_to_cpu(ev->akm_suite);
[...]
        ret = cfg80211_external_auth_request(vif->netdev, &auth, GFP_KERNEL);


but maybe that was never tested?

johannes