linux-kernel - Re: [PATCH 4/5] wifi: brcmfmac: Support bss

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAF4BwTUCtARMGwUfUiaJK+1DfYtnm8pTZ6sA2UCWEaw0XihbCw@mail.gmail.com>
Date:   Tue, 31 Oct 2023 10:04:00 -0400
From:   Daniel Berlin <dberlin@...rlin.org>
To:     Arend van Spriel <arend.vanspriel@...adcom.com>
Cc:     Arend van Spriel <aspriel@...il.com>,
        Franky Lin <franky.lin@...adcom.com>,
        Hante Meuleman <hante.meuleman@...adcom.com>,
        linux-wireless@...r.kernel.org,
        brcm80211-dev-list.pdl@...adcom.com,
        SHA-cyfmac-dev-list@...ineon.com, linux-kernel@...r.kernel.org,
        Hector Martin <marcan@...can.st>
Subject: Re: [PATCH 4/5] wifi: brcmfmac: Support bss_info up to v112

On Fri, Oct 20, 2023 at 1:31 PM Daniel Berlin <dberlin@...rlin.org> wrote:
>
> So, at least in the example code I have, all variants of the 109 and
> 112 structures both have bcnflags in that place - it was always
> missing here.
> For example, see
> https://android.googlesource.com/kernel/google-modules/wlan/bcmdhd/bcm4398/+/refs/heads/android-gs-shusky-5.15-u-qpr1-beta2/include/wlioctl.h
> and  compare v109 and v112 of bssinfo.
>
> As such, the 109 and 112 structures are compatible given these definitions.
>
> I don't know if what is there right now is wrong, or it is "yet
> another variant" of the 109 structure that we need to handle.
> Any idea what the ground truth is?

Circling back to this - i have checked other sources as well -
infineon also has u8 (though it's marked as padding) there, etc.

As far as i can tell, the structure should have that u8 there in all
versions i can find.

Nevertheless, I think I can make it not matter ;)

I'm going to post an RFC of some patches that handle this and other
structure versioning
things through function pointer structures that we set based on
interface versions available. So instead of setting feature flags, we
query the various iovars/firmware
info for the right interface versions, and set up the
structures/function pointers to handle the versions
we will get from the firmware.

This also makes the common code cleaner as they no longer have to deal
with the structure differences - for example,  brcmf_cfg80211_connect
now just calls
something like drvr->join_params_handler.get_struct_from_connect to
get an extended join params struct of the right version
and doesn't look inside the result anymore. It's an RFC so we can
argue about the approach and APIs, i just did what seemed
easy/obvious.

I have converted bss info, join params, scan params, and netinfo
versions to use this approach, and all now support all versions of the
structures available.

If we go that route, it wont matter if we overlay bss info structures or not.

As an aside, while doing the function pointer work, i discovered some
other structure bugs (missing fields) that seem to be causing
failures/fallbacks on every chip i tested (no matter who the vendor
was).

So for example, some of the extended join parameter substructs are
missing fields, and every chip i tried (5 variants, representing 2
vendors and 3 join_param versions) all gave
BUFTOSHORT when trying to use the join iovar as a result, and all fell
back to using the SET_SSID method.
With the v0/v1 structure fixed, they all use the join iovar successfully and
never fall back.

--Dan