| Message ID | 20201002175308.16374-1-thomas@adapt-ip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | [1/2] mac80211: handle lack of sband->bitrates in rates | expand |
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 <thomas@adapt-ip.com> > --- > 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; >
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 <thomas@adapt-ip.com> >> --- >> 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; >>
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;
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 <thomas@adapt-ip.com> --- net/mac80211/cfg.c | 3 ++- net/mac80211/sta_info.c | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-)