Skip to contentSkip to conversation

rtw88: fix subscript above array bounds compiler warning

RFC In review Stanislaw Gruszka 2 participants 4 messages May 3, 2019 – May 6, 2019 Realtek wireless driver (rtw88)

lore
Share & tools
lore

Stanislaw Gruszka

author
#0 lore raw

RFC 1/1 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 <[email protected]>
---
 drivers/net/wireless/realtek/rtw88/phy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/wireless/realtek/rtw88/phy.c +3 -3
index 35a35dbca85f..a716a44d78b0 100644
@@ -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)
Signature · 1 line
1.9.3
#1 lore raw
Quoted 42 lines

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 <[email protected]>
---
 drivers/net/wireless/realtek/rtw88/phy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/wireless/realtek/rtw88/phy.c +3 -3
index 35a35dbca85f..a716a44d78b0 100644
@@ -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;
 }
Quoted 2 lines

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

author
#2 lore raw
Quoted 42 lines · Tony Chuang
Quoted 42 lines

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 <[email protected]>
---
 drivers/net/wireless/realtek/rtw88/phy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/wireless/realtek/rtw88/phy.c +3 -3
index 35a35dbca85f..a716a44d78b0 100644
@@ -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;
 }
Quoted 8 lines
Quoted 2 lines

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

#3 lore raw
Quoted 52 lines

Subject: Re: [RFC] rtw88: fix subscript above array bounds compiler warning

Quoted 21 lines · Tony Chuang
Quoted 21 lines

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 <[email protected]>
---
 drivers/net/wireless/realtek/rtw88/phy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Quoted 6 lines

drivers/net/wireless/realtek/rtw88/phy.c void rtw_phy_dynamic_mechanism(struct

Quoted 6 lines
drivers/net/wireless/realtek/rtw88/phy.c +0 -0
index 35a35dbca85f..a716a44d78b0 100644
@@ -410,12 +410,12 @@ void rtw_phy_dynamic_mechanism(struct

rtw_dev

Quoted 21 lines
Quoted 15 lines
drivers/net/wireless/realtek/rtw88/phy.c +3 -3
 *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;
 }
Quoted 2 lines

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

Quoted 9 lines
Quoted 2 lines

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