2020-10-02 17:54:53

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH 1/2] mac80211: handle lack of sband->bitrates in rates

Even though a driver or mac80211 shouldn't produce a
legacy bitrate if sband->bitrates doesn't exist, don't
crash if that is the case either.

This fixes a kernel panic if station dump is run before
last_rate can be updated with a data frame when
sband->bitrates is missing (eg. in S1G bands).

Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/cfg.c | 3 ++-
net/mac80211/sta_info.c | 4 ++++
2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index da70f174d629..e40160114824 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -709,7 +709,8 @@ void sta_set_rate_info_tx(struct sta_info *sta,
u16 brate;

sband = ieee80211_get_sband(sta->sdata);
- if (sband) {
+ WARN_ON(sband && !sband->bitrates);
+ if (sband && sband->bitrates) {
brate = sband->bitrates[rate->idx].bitrate;
rinfo->legacy = DIV_ROUND_UP(brate, 1 << shift);
}
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index f2840d1d95cf..0efb66b8f185 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2122,6 +2122,10 @@ static void sta_stats_decode_rate(struct ieee80211_local *local, u32 rate,
int rate_idx = STA_STATS_GET(LEGACY_IDX, rate);

sband = local->hw.wiphy->bands[band];
+
+ if (WARN_ON(!sband->bitrates))
+ break;
+
brate = sband->bitrates[rate_idx].bitrate;
if (rinfo->bw == RATE_INFO_BW_5)
shift = 2;
--
2.20.1


2020-10-02 17:56:02

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH 2/2] mac80211: initialize last_rate for S1G STAs

last_rate is initialized to zero by sta_info_alloc(), but
this indicates legacy bitrate for the last TX rate (and
invalid for the last RX rate). To avoid a warning when
decoding the last rate as legacy (before a data frame
has been sent), initialize them as S1G MCS.

Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/Makefile | 1 +
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/mlme.c | 4 +++-
net/mac80211/rate.c | 1 +
net/mac80211/s1g.c | 17 +++++++++++++++++
net/mac80211/sta_info.h | 1 +
6 files changed, 26 insertions(+), 1 deletion(-)
create mode 100644 net/mac80211/s1g.c

diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index 6cbb1286d6c0..ad04c361cba5 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -13,6 +13,7 @@ mac80211-y := \
ht.o agg-tx.o agg-rx.o \
vht.o \
he.o \
+ s1g.o \
ibss.o \
iface.o \
rate.o \
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c3e3578574a6..d9d7a34480b0 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1928,6 +1928,9 @@ void
ieee80211_he_op_ie_to_bss_conf(struct ieee80211_vif *vif,
const struct ieee80211_he_operation *he_op_ie_elem);

+/* S1G */
+void ieee80211_s1g_sta_init(struct sta_info *sta);
+
/* Spectrum management */
void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata,
struct ieee80211_mgmt *mgmt,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e9a8e8e94ee6..fe6cfb8d477f 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5190,8 +5190,10 @@ static int ieee80211_prep_connection(struct ieee80211_sub_if_data *sdata,
int shift = ieee80211_vif_get_shift(&sdata->vif);

/* TODO: S1G Basic Rate Set is expressed elsewhere */
- if (cbss->channel->band == NL80211_BAND_S1GHZ)
+ if (cbss->channel->band == NL80211_BAND_S1GHZ) {
+ ieee80211_s1g_sta_init(new_sta);
goto skip_rates;
+ }

ieee80211_get_rates(sband, bss->supp_rates,
bss->supp_rates_len,
diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 0cba7fed28cf..723762a4052b 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -53,6 +53,7 @@ void rate_control_rate_init(struct sta_info *sta)

/* TODO: check for minstrel_s1g ? */
if (sband->band == NL80211_BAND_S1GHZ) {
+ ieee80211_s1g_sta_init(sta);
rcu_read_unlock();
return;
}
diff --git a/net/mac80211/s1g.c b/net/mac80211/s1g.c
new file mode 100644
index 000000000000..dbc216ad0a6e
--- /dev/null
+++ b/net/mac80211/s1g.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+/* S1G handling
+ * Copyright(c) 2020 Adapt-IP
+ */
+
+#include <linux/ieee80211.h>
+#include <net/mac80211.h>
+
+#include "ieee80211_i.h"
+
+void ieee80211_s1g_sta_init(struct sta_info *sta)
+{
+ /* avoid indicating legacy bitrates for S1G STAs */
+ sta->tx_stats.last_rate.flags |= IEEE80211_TX_RC_S1G_MCS;
+ sta->rx_stats.last_rate =
+ STA_STATS_FIELD(TYPE, STA_STATS_RATE_TYPE_S1G);
+}
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 91a61b44b4e0..00ae81e9e1a1 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -823,6 +823,7 @@ enum sta_stats_type {
STA_STATS_RATE_TYPE_HT,
STA_STATS_RATE_TYPE_VHT,
STA_STATS_RATE_TYPE_HE,
+ STA_STATS_RATE_TYPE_S1G,
};

#define STA_STATS_FIELD_HT_MCS GENMASK( 7, 0)
--
2.20.1

2020-10-02 18:59:19

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: handle lack of sband->bitrates in rates

On 10/2/20 10:53 AM, Thomas Pedersen wrote:
> Even though a driver or mac80211 shouldn't produce a
> legacy bitrate if sband->bitrates doesn't exist, don't
> crash if that is the case either.
>
> This fixes a kernel panic if station dump is run before
> last_rate can be updated with a data frame when
> sband->bitrates is missing (eg. in S1G bands).
>
> Signed-off-by: Thomas Pedersen <[email protected]>
> ---
> net/mac80211/cfg.c | 3 ++-
> net/mac80211/sta_info.c | 4 ++++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index da70f174d629..e40160114824 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -709,7 +709,8 @@ void sta_set_rate_info_tx(struct sta_info *sta,
> u16 brate;
>
> sband = ieee80211_get_sband(sta->sdata);
> - if (sband) {
> + WARN_ON(sband && !sband->bitrates);

Maybe WARN_ON_ONCE to keep the spam down in case this is hit repeatedly
for some reason?

Same below...

Thanks,
Ben

> + if (sband && sband->bitrates) {
> brate = sband->bitrates[rate->idx].bitrate;
> rinfo->legacy = DIV_ROUND_UP(brate, 1 << shift);
> }
> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
> index f2840d1d95cf..0efb66b8f185 100644
> --- a/net/mac80211/sta_info.c
> +++ b/net/mac80211/sta_info.c
> @@ -2122,6 +2122,10 @@ static void sta_stats_decode_rate(struct ieee80211_local *local, u32 rate,
> int rate_idx = STA_STATS_GET(LEGACY_IDX, rate);
>
> sband = local->hw.wiphy->bands[band];
> +
> + if (WARN_ON(!sband->bitrates))
> + break;
> +
> brate = sband->bitrates[rate_idx].bitrate;
> if (rinfo->bw == RATE_INFO_BW_5)
> shift = 2;
>


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2020-10-02 20:40:00

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: handle lack of sband->bitrates in rates

On 2020-10-02 11:55, Ben Greear wrote:
> On 10/2/20 10:53 AM, Thomas Pedersen wrote:
>> Even though a driver or mac80211 shouldn't produce a
>> legacy bitrate if sband->bitrates doesn't exist, don't
>> crash if that is the case either.
>>
>> This fixes a kernel panic if station dump is run before
>> last_rate can be updated with a data frame when
>> sband->bitrates is missing (eg. in S1G bands).
>>
>> Signed-off-by: Thomas Pedersen <[email protected]>
>> ---
>> net/mac80211/cfg.c | 3 ++-
>> net/mac80211/sta_info.c | 4 ++++
>> 2 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>> index da70f174d629..e40160114824 100644
>> --- a/net/mac80211/cfg.c
>> +++ b/net/mac80211/cfg.c
>> @@ -709,7 +709,8 @@ void sta_set_rate_info_tx(struct sta_info *sta,
>> u16 brate;
>> sband = ieee80211_get_sband(sta->sdata);
>> - if (sband) {
>> + WARN_ON(sband && !sband->bitrates);
>
> Maybe WARN_ON_ONCE to keep the spam down in case this is hit repeatedly
> for some reason?

Thanks, I originally had it as WARN_ON_ONCE(), then changed it. Not sure
why,
I don't feel strongly about it either way :)

I'll make them both WARN_ON_ONCE().

>
>> + if (sband && sband->bitrates) {
>> brate = sband->bitrates[rate->idx].bitrate;
>> rinfo->legacy = DIV_ROUND_UP(brate, 1 << shift);
>> }
>> diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
>> index f2840d1d95cf..0efb66b8f185 100644
>> --- a/net/mac80211/sta_info.c
>> +++ b/net/mac80211/sta_info.c
>> @@ -2122,6 +2122,10 @@ static void sta_stats_decode_rate(struct
>> ieee80211_local *local, u32 rate,
>> int rate_idx = STA_STATS_GET(LEGACY_IDX, rate);
>> sband = local->hw.wiphy->bands[band];
>> +
>> + if (WARN_ON(!sband->bitrates))
>> + break;
>> +
>> brate = sband->bitrates[rate_idx].bitrate;
>> if (rinfo->bw == RATE_INFO_BW_5)
>> shift = 2;
>>

--
thomas