2012-01-25 08:18:25

by Dan Carpenter

[permalink] [raw]
Subject: re: b43: N-PHY: implement RSSI calibration for rev3+

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



2012-01-25 12:42:54

by Larry Finger

[permalink] [raw]
Subject: Re: b43: N-PHY: implement RSSI calibration for rev3+

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