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;
Message-ID: <715050f6db60d90b3fa6ae4193273dcb73838b64.camel@sipsolutions.net>
Subject: Re: [PATCH 0/1] Fix potential HE IE bug and some other questions
From:   Johannes Berg <johannes@sipsolutions.net>
To:     James Prestwood <prestwoj@gmail.com>,
        linux-wireless@vger.kernel.org
Date:   Thu, 29 Sep 2022 11:56:27 +0200
In-Reply-To: <20220928224910.453232-1-prestwoj@gmail.com>
References: <20220928224910.453232-1-prestwoj@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4 (3.44.4-2.fc36) 
MIME-Version: 1.0
Precedence: bulk

On Wed, 2022-09-28 at 15:49 -0700, James Prestwood wrote:
> I believe there is a bug when building the probe request IEs for the
> HE capabilities. More info in the patch.
>=20

That fix seems right.

> While looking at this I
> noticed some other confusing code related to building the probe
> request.
>=20
> Looking at ieee80211_build_preq_ies. It is passed 'bands_used' which
> is a bitmask of bands. A probe request is only sent out on a single
> band so why would this contain multiple bands?=C2=A0
>=20

The function can be used to prepare a HW scan request, which can contain
the elements for all bands that the HW is being asked to scan on.

> We then loop over these
> bands and call ieee80211_build_preq_ies_band for each one.

Correct, and ie_desc->ies[band]/len[band] gets the pointer/size for each
band.

> This, AFAICT, would append the same IEs multiple times if 'bands_used'
> contained more than one band.
>=20

Correct.

> Internal to mac80211/util.c its only
> passed BIT(chan->band), but mac80211/scan.c seems to pass a list...

Right, that's because "internal" is ieee80211_build_probe_req(), which
builds only a single probe request, while the other code is for HW scan.

> Below is the warning I am seeing (many, many times). It says the warning
> is in build_preq_ies, but it really seems like this is not correct
> and its actually in ieee80211_get_he_6ghz_capa since I see no warning
> message as others _should_ have.
>=20
> [  732.130000] ------------[ cut here ]------------
> [  732.130000] WARNING: CPU: 0 PID: 1352 at include/net/cfg80211.h:608 ie=
ee80211_build_preq_ies+0x766/0x84d

The line number is in ieee80211_get_he_6ghz_capa() but that's inlined,
and that doesn't always work so well for the symbol resolution.

johannes