On Sat, 17. May 09:40, Stanislaw Gruszka wrote: > Move rate_idx range check after we add IL_FIRST_OFDM_RATE for it > for 5GHz band. > > Additionally use ">= RATE_COUNT" check instead of "> RATE_COUNT_LEGACY" > to avoid possible reviewers and static code analyzers confusion about > size of il_rate array. > > Reported-by: Fedor Pchelkin <pchelkin@xxxxxxxxx> > Reported-by: Alexei Safin <a.safin@xxxxxxx> > Signed-off-by: Stanislaw Gruszka <stf_xl@xxxxx> > --- Thank you for the patch, Stanislaw! Please see some comments below. > drivers/net/wireless/intel/iwlegacy/4965-mac.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c b/drivers/net/wireless/intel/iwlegacy/4965-mac.c > index dc8c408902e6..2294ea43b4c7 100644 > --- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c > +++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c > @@ -1567,16 +1567,19 @@ il4965_tx_cmd_build_rate(struct il_priv *il, > /** > * If the current TX rate stored in mac80211 has the MCS bit set, it's > * not really a TX rate. Thus, we use the lowest supported rate for > - * this band. Also use the lowest supported rate if the stored rate > - * idx is invalid. > + * this band. > */ > rate_idx = info->control.rates[0].idx; > - if ((info->control.rates[0].flags & IEEE80211_TX_RC_MCS) || rate_idx < 0 > - || rate_idx > RATE_COUNT_LEGACY) > + if (info->control.rates[0].flags & IEEE80211_TX_RC_MCS) > rate_idx = rate_lowest_index(&il->bands[info->band], sta); > - /* For 5 GHZ band, remap mac80211 rate indices into driver indices */ > - if (info->band == NL80211_BAND_5GHZ) > + else if (info->band == NL80211_BAND_5GHZ) 5GHZ shouldn't be in 'else if' clause, I think. Is it mutually exclusive with IEEE80211_TX_RC_MCS ? > + /* For 5 GHZ band, remap mac80211 rate indices into driver indices */ > rate_idx += IL_FIRST_OFDM_RATE; > + > + /* Use the lowest supported rate if the stored rate idx is invalid. */ > + if (rate_idx < 0 || rate_idx >= RATE_COUNT) There is a check inside il4965_rs_get_rate(): /* Check for invalid rates */ if (rate_idx < 0 || rate_idx >= RATE_COUNT_LEGACY || (sband->band == NL80211_BAND_5GHZ && rate_idx < IL_FIRST_OFDM_RATE)) rate_idx = rate_lowest_index(sband, sta); so RATE_COUNT_LEGACY (60M) is considered invalid there but is accepted here in il4965_tx_cmd_build_rate(). It looks strange, at least for the fresh reader as me.. > + rate_idx = rate_lowest_index(&il->bands[info->band], sta); > + > /* Get PLCP rate for tx_cmd->rate_n_flags */ > rate_plcp = il_rates[rate_idx].plcp; > /* Zero out flags for this packet */ > -- > 2.25.4
