Re: [PATCH 2/2] rc80211-pid: add sanity check

[Larry Finger <larry.finger@lwfinger.net>]

Stefano Brivio wrote:
> From: Larry Finger <larry.finger@lwfinger.net>
> 
> Add a sanity check in rate_control_pid_adjust_rate(). Thanks to Larry Finger
> for suggesting this and reporting a related bug.
> 
> Signed-off-by: Stefano Brivio <stefano.brivio@polimi.it>
> NOT-Signed-off-by: Larry Finger <larry.finger@lwfinger.net>
> ---
> Index: wireless-2.6/net/mac80211/rc80211_pid_algo.c
> ===================================================================
> --- wireless-2.6.orig/net/mac80211/rc80211_pid_algo.c
> +++ wireless-2.6/net/mac80211/rc80211_pid_algo.c
> @@ -128,6 +128,11 @@ static void rate_control_pid_adjust_rate
>  		}
>  
>  		newidx += back;
> +
> +		if (newidx < 0 || newidx >= sband->n_bitrates) {
> +			WARN_ON(1);
> +			break;
> +		}
>  	}
>  
>  #ifdef CONFIG_MAC80211_DEBUGFS

The bug is triggered when the following conditions are true: (1) an 802.11g device is being used on
an 802.11b network, (2) "newidx" from rate_control_pid_shift_adjust is higher than the index of 
rates used by the AP, and (3) the value of "back" is 1. The loop keeps increasing "newidx" but 
sta->supp_rates & BIT(newidx) can never be true.

Although the real cause of the bug is due to selecting a trial value for the new rate that can never 
be satisfied, I think that this sanity check is a reasonable way to detect and stop the runaway. As 
this condition is likely to affect a lot of systems, I think the WARN_ON(1) should be removed from 
the final version of the patch. With it in, a lot of logs will get spammed.

Larry