| Message ID | 20190503115333.GA23109@redhat.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [RFC] rtw88: fix subscript above array bounds compiler warning | expand |
> Subject: [RFC] rtw88: fix subscript above array bounds compiler warning > > My compiler complain about: > > drivers/net/wireless/realtek/rtw88/phy.c: In function > ‘rtw_phy_rf_power_2_rssi’: > drivers/net/wireless/realtek/rtw88/phy.c:430:26: warning: array subscript is > above array bounds [-Warray-bounds] > linear = db_invert_table[i][j]; > > According to comment power_db should be in range 1 ~ 96 . > Correct rtw_phy_power_2_db() to make max power 96 db > (still min is 0). This make the warning gone. > > However power >= 20 check still looks somewhat suspicious to me. > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > --- > drivers/net/wireless/realtek/rtw88/phy.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/realtek/rtw88/phy.c > b/drivers/net/wireless/realtek/rtw88/phy.c > index 35a35dbca85f..a716a44d78b0 100644 > --- a/drivers/net/wireless/realtek/rtw88/phy.c > +++ b/drivers/net/wireless/realtek/rtw88/phy.c > @@ -410,12 +410,12 @@ void rtw_phy_dynamic_mechanism(struct rtw_dev > *rtwdev) > > static u8 rtw_phy_power_2_db(s8 power) > { > - if (power <= -100 || power >= 20) > + if (power <= -96 || power >= 20) > return 0; > else if (power >= 0) > - return 100; > + return 96; > else > - return 100 + power; > + return 96 + power; > } > > static u64 rtw_phy_db_2_linear(u8 power_db) > -- I think I should check with the radio team, that if the power from the rx descriptor generated by hardware can possibly get >= 20 And also check what the actual logic they expected to deal with the power. Thanks for reporting it. Yan-Hsuan
On Fri, May 03, 2019 at 12:01:05PM +0000, Tony Chuang wrote: > > Subject: [RFC] rtw88: fix subscript above array bounds compiler warning > > > > My compiler complain about: > > > > drivers/net/wireless/realtek/rtw88/phy.c: In function > > ‘rtw_phy_rf_power_2_rssi’: > > drivers/net/wireless/realtek/rtw88/phy.c:430:26: warning: array subscript is > > above array bounds [-Warray-bounds] > > linear = db_invert_table[i][j]; > > > > According to comment power_db should be in range 1 ~ 96 . > > Correct rtw_phy_power_2_db() to make max power 96 db > > (still min is 0). This make the warning gone. > > > > However power >= 20 check still looks somewhat suspicious to me. > > > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > > --- > > drivers/net/wireless/realtek/rtw88/phy.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/wireless/realtek/rtw88/phy.c > > b/drivers/net/wireless/realtek/rtw88/phy.c > > index 35a35dbca85f..a716a44d78b0 100644 > > --- a/drivers/net/wireless/realtek/rtw88/phy.c > > +++ b/drivers/net/wireless/realtek/rtw88/phy.c > > @@ -410,12 +410,12 @@ void rtw_phy_dynamic_mechanism(struct rtw_dev > > *rtwdev) > > > > static u8 rtw_phy_power_2_db(s8 power) > > { > > - if (power <= -100 || power >= 20) > > + if (power <= -96 || power >= 20) > > return 0; > > else if (power >= 0) > > - return 100; > > + return 96; > > else > > - return 100 + power; > > + return 96 + power; > > } > > > > static u64 rtw_phy_db_2_linear(u8 power_db) > > -- > > I think I should check with the radio team, that if the power from the > rx descriptor generated by hardware can possibly get >= 20 > > And also check what the actual logic they expected to deal with the power. > Thanks for reporting it. Yeah, this could be just teoretical issue as we can not get power values >= 0 from HW. However I think compiler correctly complains, as for power_db=100 we get i = ((100 - 1) >> 3) = 12 , what exceed by one max first index of db_invert_table[][], which should be in range 0 - 11. Stanislaw
> Subject: Re: [RFC] rtw88: fix subscript above array bounds compiler warning > > On Fri, May 03, 2019 at 12:01:05PM +0000, Tony Chuang wrote: > > > Subject: [RFC] rtw88: fix subscript above array bounds compiler warning > > > > > > My compiler complain about: > > > > > > drivers/net/wireless/realtek/rtw88/phy.c: In function > > > ‘rtw_phy_rf_power_2_rssi’: > > > drivers/net/wireless/realtek/rtw88/phy.c:430:26: warning: array subscript is > > > above array bounds [-Warray-bounds] > > > linear = db_invert_table[i][j]; > > > > > > According to comment power_db should be in range 1 ~ 96 . > > > Correct rtw_phy_power_2_db() to make max power 96 db > > > (still min is 0). This make the warning gone. > > > > > > However power >= 20 check still looks somewhat suspicious to me. > > > > > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > > > --- > > > drivers/net/wireless/realtek/rtw88/phy.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/net/wireless/realtek/rtw88/phy.c > > > b/drivers/net/wireless/realtek/rtw88/phy.c > > > index 35a35dbca85f..a716a44d78b0 100644 > > > --- a/drivers/net/wireless/realtek/rtw88/phy.c > > > +++ b/drivers/net/wireless/realtek/rtw88/phy.c > > > @@ -410,12 +410,12 @@ void rtw_phy_dynamic_mechanism(struct > rtw_dev > > > *rtwdev) > > > > > > static u8 rtw_phy_power_2_db(s8 power) > > > { > > > - if (power <= -100 || power >= 20) > > > + if (power <= -96 || power >= 20) > > > return 0; > > > else if (power >= 0) > > > - return 100; > > > + return 96; > > > else > > > - return 100 + power; > > > + return 96 + power; > > > } > > > > > > static u64 rtw_phy_db_2_linear(u8 power_db) > > > -- > > > > I think I should check with the radio team, that if the power from the > > rx descriptor generated by hardware can possibly get >= 20 > > > > And also check what the actual logic they expected to deal with the power. > > Thanks for reporting it. > > Yeah, this could be just teoretical issue as we can not get power > values >= 0 from HW. However I think compiler correctly complains, as > for power_db=100 we get i = ((100 - 1) >> 3) = 12 , what exceed by one > max first index of db_invert_table[][], which should be in range > 0 - 11. > I checked it. The power sum could actually be like 20 or something. And the recommended modification is to restrict the value used for array subscript between 1~96 before access the array. Such as: @@ -578,6 +578,11 @@ static u64 rtw_phy_db_2_linear(u8 power_db) u8 i, j; u64 linear; + if (power_db > 96) + power_db = 96; + else if (power_db < 1) + return 1; + /* 1dB ~ 96dB */ i = (power_db - 1) >> 3; j = (power_db - 1) - (i << 3); Yan-Hsuan
diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c index 35a35dbca85f..a716a44d78b0 100644 --- a/drivers/net/wireless/realtek/rtw88/phy.c +++ b/drivers/net/wireless/realtek/rtw88/phy.c @@ -410,12 +410,12 @@ void rtw_phy_dynamic_mechanism(struct rtw_dev *rtwdev) static u8 rtw_phy_power_2_db(s8 power) { - if (power <= -100 || power >= 20) + if (power <= -96 || power >= 20) return 0; else if (power >= 0) - return 100; + return 96; else - return 100 + power; + return 96 + power; } static u64 rtw_phy_db_2_linear(u8 power_db)
My compiler complain about: drivers/net/wireless/realtek/rtw88/phy.c: In function ‘rtw_phy_rf_power_2_rssi’: drivers/net/wireless/realtek/rtw88/phy.c:430:26: warning: array subscript is above array bounds [-Warray-bounds] linear = db_invert_table[i][j]; According to comment power_db should be in range 1 ~ 96 . Correct rtw_phy_power_2_db() to make max power 96 db (still min is 0). This make the warning gone. However power >= 20 check still looks somewhat suspicious to me. Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> --- drivers/net/wireless/realtek/rtw88/phy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)