re: b43: N-PHY: implement RSSI calibration for rev3+

re: b43: N-PHY: implement RSSI calibration for rev3+

[Dan Carpenter]

Hello Rafał Miłecki,

The patch e0c9a0219a8f: "b43: N-PHY: implement RSSI calibration for 
rev3+" from Jan 5, 2012, leads to the following Smatch warning:
drivers/net/wireless/b43/phy_n.c +1381 b43_nphy_rev3_rssi_cal()
	 error: buffer overflow 'results[j]' 4 <= 4


+               for (i = 0; i < 4; i++) {
+                       s32 curr;
+                       s32 mind = 40;
+                       s32 minpoll = 249;
+                       u8 minvcm = 0;
+                       if (2 * core != i)
+                               continue;
+                       for (j = 0; j < 8; j++) {
+                               curr = results[j][i] * results[j][i] +
+                                       results[j][i + 1] * results[j][i];
                                                   ^^^^^
On the last iteration through the loop "i + 1" = 4.

+                               if (curr < mind) {
+                                       mind = curr;
+                                       minvcm = j;
+                               }
+                               if (results[j][i] < minpoll)
+                                       minpoll = results[j][i];
+                       }
+                       vcm_final = minvcm;
+                       results_min[i] = minpoll;
+               }

I don't know the code well enough to say if this can happen or not.
Perhaps on the last iteration we always hit the "if (2 * core != i)
continue" condition.  Anyway, since this is the first time this has hit
linux-next, I thought I would let you know.

regards,
dan carpenter

Re: b43: N-PHY: implement RSSI calibration for rev3+

[Larry Finger]

On 01/25/2012 02:18 AM, Dan Carpenter wrote:
> Hello Rafał Miłecki,
>
> The patch e0c9a0219a8f: "b43: N-PHY: implement RSSI calibration for
> rev3+" from Jan 5, 2012, leads to the following Smatch warning:
> drivers/net/wireless/b43/phy_n.c +1381 b43_nphy_rev3_rssi_cal()
> 	 error: buffer overflow 'results[j]' 4<= 4
>
>
> +               for (i = 0; i<  4; i++) {
> +                       s32 curr;
> +                       s32 mind = 40;
> +                       s32 minpoll = 249;
> +                       u8 minvcm = 0;
> +                       if (2 * core != i)
> +                               continue;
> +                       for (j = 0; j<  8; j++) {
> +                               curr = results[j][i] * results[j][i] +
> +                                       results[j][i + 1] * results[j][i];
>                                                     ^^^^^
> On the last iteration through the loop "i + 1" = 4.
>
> +                               if (curr<  mind) {
> +                                       mind = curr;
> +                                       minvcm = j;
> +                               }
> +                               if (results[j][i]<  minpoll)
> +                                       minpoll = results[j][i];
> +                       }
> +                       vcm_final = minvcm;
> +                       results_min[i] = minpoll;
> +               }
>
> I don't know the code well enough to say if this can happen or not.
> Perhaps on the last iteration we always hit the "if (2 * core != i)
> continue" condition.  Anyway, since this is the first time this has hit
> linux-next, I thought I would let you know.

The condition you point out will occur for i equals 3. As 2 * anything will 
never be equal to 3, the continue will definitely be executed for that case.

This idea would need further investigation, but it certainly appears the the for 
loop could be changed to "for (i = 0; i < 4; i += 2)", which would accomplish 
the same end and should have the side effect of silencing the Smatch warning.

Larry