Received-SPF: pass (google.com: domain of linux-wireless+bounces-2549-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <818ba565b910f87d6863576044a32459cbb84839.camel@sipsolutions.net>
Subject: Re: [PATCH 1/1] wifi: mac80211: ignore the bitrate mask when
 scanning
From: Johannes Berg <johannes@sipsolutions.net>
To: Dmitry Antipov <dmantipov@yandex.ru>
Cc: Kalle Valo <kvalo@kernel.org>, linux-wireless@vger.kernel.org, 
	lvc-project@linuxtesting.org, 
	syzbot+fdc5123366fb9c3fdc6d@syzkaller.appspotmail.com
Date: Fri, 26 Jan 2024 11:06:50 +0100
In-Reply-To: <20240125090737.49673-2-dmantipov@yandex.ru>
References: 
	<58970b94a314414402a932576ec54188c0675016.camel@sipsolutions.net>
	 <20240125090737.49673-1-dmantipov@yandex.ru>
	 <20240125090737.49673-2-dmantipov@yandex.ru>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.50.3 (3.50.3-1.fc39) 
Precedence: bulk
MIME-Version: 1.0

On Thu, 2024-01-25 at 12:07 +0300, Dmitry Antipov wrote:
>=20
> +++ b/net/mac80211/rate.c
> @@ -351,6 +351,7 @@ static void __rate_control_send_low(struct ieee80211_=
hw *hw,
>  	int i;
>  	u32 rate_flags =3D
>  		ieee80211_chandef_rate_flags(&hw->conf.chandef);
> +	bool scanning =3D !!(info->control.flags & IEEE80211_TX_CTRL_SCAN_TX);
> =20
>  	if (sband->band =3D=3D NL80211_BAND_S1GHZ) {
>  		info->control.rates[0].flags |=3D IEEE80211_TX_RC_S1G_MCS;
> @@ -364,7 +365,8 @@ static void __rate_control_send_low(struct ieee80211_=
hw *hw,
> =20
>  	info->control.rates[0].idx =3D 0;
>  	for (i =3D 0; i < sband->n_bitrates; i++) {
> -		if (!(rate_mask & BIT(i)))
> +		/* Do not use the bitrate mask when scanning. */
> +		if (!scanning && !(rate_mask & BIT(i)))
>  			continue;
>=20

I know my patch didn't work, but I think I'd still prefer if we didn't
pass a 0 rate mask around to ignore it - maybe we can just make it ~0 in
the outer place that looks at the sdata mask(s)?

I'm not sure why we fill "BIT(sband->n_bitrates) - 1" rather than ~0 in
the first place, we always (have to) check anyway, so wouldn't really
care about having more bits set than rates exist ...

johannes