Re: [PATCH] ath10k: fix recent bandwidth conversion bug

[Kalle Valo <kvalo@qca.qualcomm.com>]

Christian Lamparter <chunkeey@gmail.com> writes:

> On Monday, November 20, 2017 11:57:21 AM CET Kalle Valo wrote:
>> Christian Lamparter <chunkeey@gmail.com> writes:
>>=20
>> > On Wednesday, November 1, 2017 9:37:53 PM CET Sebastian Gottschall wro=
te:
>> >> a additional array bounds check would be good
>> >
>> > Ah, about that:
>> >
>> > the bw variable in ath10k_htt_rx_h_rates() is extracted from info2
>> > in the following way [0]:
>> > |	bw =3D info2 & 3;
>> >
>> > the txrate.bw variable in ath10k_update_per_peer_tx_stats() is set by =
[1]:
>> > |	txrate.bw =3D ATH10K_HW_BW(peer_stats->flags);
>> >
>> > ATH10K_HW_BW is a macro defined as [2]:
>> > |	#define ATH10K_HW_BW(flags)		(((flags) >> 3) & 0x3)
>> >
>> > In both cases the bandwidth values already are limited to 0-3 by
>> > the "and 3" operation.
>>=20
>> Until someone changes that part of the code (and the firmware
>> interface). IMHO a switch is safer as there we don't have any risk of
>> out of bands access.
>
> The kbuild-bot/CI can catch this too.=20
>
> For example, it will look like this:
> drivers/net/wireless/ath/ath10k//htt_rx.c:710:52: warning: invalid
> access past the end of 'ath10k_bw_to_mac80211' (4 4)

Sure, but after reading about all these security vulnerabilities I have
become even more cautious and try to avoid all tricky stuff.

> BTW:
> Have you noticed:
>
> <https://github.com/lede-project/source/blob/master/package/kernel/mac802=
11/patches/319-ath10k-fix-recent-bandwidth-conversion-bug.patch>
>
> Is this really your signed-off-by or not?

I suspect that patch is taken from my pending branch.

> In any case, you - as the maintainer - can modify the patch as
> you see fit. So, please do so.

Ok, we'll send v2.

--=20
Kalle Valo=