[lvc-project] [PATCH] wifi: iwlegacy: Check rate_idx range after addition

[Fedor Pchelkin]

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 at ispras.ru>
> Reported-by: Alexei Safin <a.safin at rosa.ru>
> Signed-off-by: Stanislaw Gruszka <stf_xl at wp.pl>
> ---

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