2014-10-13 09:48:41

by Karl Beldan

Subject: [PATCH] mac80211: fix typo in starting baserate for rts_cts_rate_idx

From: Karl Beldan <[email protected]>

It affects non-(V)HT rates and can lead to selecting an rts_cts rate
that is not a basic rate or way superior to the reference rate (ATM
rates[0] used for the 1st attempt of the protected frame data).

E.g, assuming drivers register growing (bitrate) sorted tables of
ieee80211_rate-s, having :
- rates[0].idx == d'2 and basic_rates == b'10100
will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise
- rates[0].idx == d'2 and basic_rates == b'10001
will select rts_cts idx b'10000
The first is not a basic rate and the second is > rates[0].

Also, wrt severity of the addressed misbehavior, ATM we only have one
rts_cts_rate_idx rather than one per rate table entry, so this idx might
still point to bitrates > rates[1..MAX_RATES].

Fixes: 5253ffb8 ("mac80211: always pick a basic rate to tx RTS/CTS for pre-HT rates")
Signed-off-by: Karl Beldan <[email protected]>
---
net/mac80211/rate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 8fdadfd..6081329 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -448,7 +448,7 @@ static void rate_fixup_ratelist(struct ieee80211_vif *vif,
*/
if (!(rates[0].flags & IEEE80211_TX_RC_MCS)) {
u32 basic_rates = vif->bss_conf.basic_rates;
- s8 baserate = basic_rates ? ffs(basic_rates - 1) : 0;
+ s8 baserate = basic_rates ? ffs(basic_rates) - 1 : 0;

rate = &sband->bitrates[rates[0].idx];

--
2.0.1

2014-10-13 10:24:32

by Karl Beldan

Subject: Re: [PATCH] mac80211: fix typo in starting baserate for rts_cts_rate_idx

Properly cc'ing stable (I used kernel.org instead of vger.kernel.org).

On Mon, Oct 13, 2014 at 11:48:12AM +0200, Karl Beldan wrote:
> From: Karl Beldan <[email protected]>
>
> It affects non-(V)HT rates and can lead to selecting an rts_cts rate
> that is not a basic rate or way superior to the reference rate (ATM
> rates[0] used for the 1st attempt of the protected frame data).
>
> E.g, assuming drivers register growing (bitrate) sorted tables of
> ieee80211_rate-s, having :
> - rates[0].idx == d'2 and basic_rates == b'10100
> will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise
> - rates[0].idx == d'2 and basic_rates == b'10001
> will select rts_cts idx b'10000
> The first is not a basic rate and the second is > rates[0].
>
> Also, wrt severity of the addressed misbehavior, ATM we only have one
> rts_cts_rate_idx rather than one per rate table entry, so this idx might
> still point to bitrates > rates[1..MAX_RATES].
>
> Fixes: 5253ffb8 ("mac80211: always pick a basic rate to tx RTS/CTS for pre-HT rates")
> Signed-off-by: Karl Beldan <[email protected]>
> ---
> net/mac80211/rate.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
> index 8fdadfd..6081329 100644
> --- a/net/mac80211/rate.c
> +++ b/net/mac80211/rate.c
> @@ -448,7 +448,7 @@ static void rate_fixup_ratelist(struct ieee80211_vif *vif,
> */
> if (!(rates[0].flags & IEEE80211_TX_RC_MCS)) {
> u32 basic_rates = vif->bss_conf.basic_rates;
> - s8 baserate = basic_rates ? ffs(basic_rates - 1) : 0;
> + s8 baserate = basic_rates ? ffs(basic_rates) - 1 : 0;
>
> rate = &sband->bitrates[rates[0].idx];
>
> --
> 2.0.1
>

2014-10-13 12:35:16

by Karl Beldan

Subject: [PATCH v2] mac80211: fix typo in starting baserate for rts_cts_rate_idx

From: Karl Beldan <[email protected]>

It affects non-(V)HT rates and can lead to selecting an rts_cts rate
that is not a basic rate or way superior to the reference rate (ATM
rates[0] used for the 1st attempt of the protected frame data).

E.g, assuming drivers register growing (bitrate) sorted tables of
ieee80211_rate-s, having :
- rates[0].idx == d'2 and basic_rates == b'10100
will select rts_cts idx b'10011 & ~d'(BIT(2)-1), i.e. 1, likewise
- rates[0].idx == d'2 and basic_rates == b'10001
will select rts_cts idx b'10000
The first is not a basic rate and the second is > rates[0].

Also, wrt severity of the addressed misbehavior, ATM we only have one
rts_cts_rate_idx rather than one per rate table entry, so this idx might
still point to bitrates > rates[1..MAX_RATES].

Fixes: 5253ffb8c9e1 ("mac80211: always pick a basic rate to tx RTS/CTS for pre-HT rates")
Cc: [email protected]
Signed-off-by: Karl Beldan <[email protected]>
---
net/mac80211/rate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 8fdadfd..6081329 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -448,7 +448,7 @@ static void rate_fixup_ratelist(struct ieee80211_vif *vif,
*/
if (!(rates[0].flags & IEEE80211_TX_RC_MCS)) {
u32 basic_rates = vif->bss_conf.basic_rates;
- s8 baserate = basic_rates ? ffs(basic_rates - 1) : 0;
+ s8 baserate = basic_rates ? ffs(basic_rates) - 1 : 0;

rate = &sband->bitrates[rates[0].idx];

--
2.0.1

2014-10-14 09:17:45

by Johannes Berg

Subject: Re: [PATCH v2] mac80211: fix typo in starting baserate for rts_cts_rate_idx

On Mon, 2014-10-13 at 14:34 +0200, Karl Beldan wrote:
> From: Karl Beldan <[email protected]>
>
> It affects non-(V)HT rates and can lead to selecting an rts_cts rate
> that is not a basic rate or way superior to the reference rate (ATM
> rates[0] used for the 1st attempt of the protected frame data).

Applied.

johannes