[v2] net/txgbe: fix out of bound access
Checks
Commit Message
Reported by SuSe CI [1] by GCC [2], possibly false positive. Error:
In function 'txgbe_host_interface_command',
inlined from 'txgbe_host_interface_command'
at ../drivers/net/txgbe/base/txgbe_mng.c:104:1,
inlined from 'txgbe_hic_reset'
at ../drivers/net/txgbe/base/txgbe_mng.c:345:9:
../drivers/net/txgbe/base/txgbe_mng.c:145:36:
error: array subscript 2 is outside array bounds ofr
'struct txgbe_hic_reset[1]' [-Werror=array-bounds=]
145 | buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi);
../drivers/net/txgbe/base/txgbe_mng.c: In function 'txgbe_hic_reset':
../drivers/net/txgbe/base/txgbe_mng.c:331:32:
note: at offset 8 into object 'reset_cmd' of size 8
331 | struct txgbe_hic_reset reset_cmd;
| ^~~~~~~~~
Access to buffer done based on command code, the case complained by
FW_RESET_CMD has short buffer but this code path only taken with command
0x30, so this shouldn't be a problem.
Command 0x30 no more used, removing this exception check that cause
build error.
[1]
https://build.opensuse.org/public/build/home:bluca:dpdk/openSUSE_Factory_ARM/armv7l/dpdk-20.11/_log
[2]
gcc 13.2.1 "cc (SUSE Linux) 13.2.1 20230912
Fixes: 35c90ecccfd4 ("net/txgbe: add EEPROM functions")
Cc: stable@dpdk.org
Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>
---
Cc: jiawenwu@trustnetic.com
Cc: jianwang@trustnetic.com
v2:
* Removed exception check for command 0x30
---
drivers/net/txgbe/base/txgbe_mng.c | 16 +---------------
1 file changed, 1 insertion(+), 15 deletions(-)
Comments
On Friday, November 17, 2023 6:12 PM, Ferruh.Yigit@amd.com wrote:
> Reported by SuSe CI [1] by GCC [2], possibly false positive. Error:
>
> In function 'txgbe_host_interface_command',
> inlined from 'txgbe_host_interface_command'
> at ../drivers/net/txgbe/base/txgbe_mng.c:104:1,
> inlined from 'txgbe_hic_reset'
> at ../drivers/net/txgbe/base/txgbe_mng.c:345:9:
> ../drivers/net/txgbe/base/txgbe_mng.c:145:36:
> error: array subscript 2 is outside array bounds ofr
> 'struct txgbe_hic_reset[1]' [-Werror=array-bounds=]
> 145 | buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi);
> ../drivers/net/txgbe/base/txgbe_mng.c: In function 'txgbe_hic_reset':
> ../drivers/net/txgbe/base/txgbe_mng.c:331:32:
> note: at offset 8 into object 'reset_cmd' of size 8
> 331 | struct txgbe_hic_reset reset_cmd;
> | ^~~~~~~~~
>
> Access to buffer done based on command code, the case complained by
> FW_RESET_CMD has short buffer but this code path only taken with command
> 0x30, so this shouldn't be a problem.
>
> Command 0x30 no more used, removing this exception check that cause
> build error.
>
> [1]
> https://build.opensuse.org/public/build/home:bluca:dpdk/openSUSE_Factory_ARM/armv7l/dpdk-20.11/_log
>
> [2]
> gcc 13.2.1 "cc (SUSE Linux) 13.2.1 20230912
>
> Fixes: 35c90ecccfd4 ("net/txgbe: add EEPROM functions")
> Cc: stable@dpdk.org
>
> Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>
> ---
> Cc: jiawenwu@trustnetic.com
> Cc: jianwang@trustnetic.com
>
> v2:
> * Removed exception check for command 0x30
> ---
> drivers/net/txgbe/base/txgbe_mng.c | 16 +---------------
> 1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/drivers/net/txgbe/base/txgbe_mng.c b/drivers/net/txgbe/base/txgbe_mng.c
> index df7145094f84..029a0a1fe143 100644
> --- a/drivers/net/txgbe/base/txgbe_mng.c
> +++ b/drivers/net/txgbe/base/txgbe_mng.c
> @@ -141,21 +141,7 @@ txgbe_host_interface_command(struct txgbe_hw *hw, u32 *buffer,
> for (bi = 0; bi < dword_len; bi++)
> buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi);
>
> - /*
> - * If there is any thing in data position pull it in
> - * Read Flash command requires reading buffer length from
> - * two byes instead of one byte
> - */
> - if (resp->cmd == 0x30) {
> - for (; bi < dword_len + 2; bi++)
> - buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi);
> -
> - buf_len = (((u16)(resp->cmd_or_resp.ret_status) << 3)
> - & 0xF00) | resp->buf_len;
> - hdr_size += (2 << 2);
> - } else {
> - buf_len = resp->buf_len;
> - }
> + buf_len = resp->buf_len;
> if (!buf_len)
> goto rel_out;
>
> --
> 2.34.1
Thanks Ferruh,
Reviewed-by: Jiawen Wu <jiawenwu@trustnetic.com>
On 11/20/2023 1:51 AM, Jiawen Wu wrote:
> On Friday, November 17, 2023 6:12 PM, Ferruh.Yigit@amd.com wrote:
>> Reported by SuSe CI [1] by GCC [2], possibly false positive. Error:
>>
>> In function 'txgbe_host_interface_command',
>> inlined from 'txgbe_host_interface_command'
>> at ../drivers/net/txgbe/base/txgbe_mng.c:104:1,
>> inlined from 'txgbe_hic_reset'
>> at ../drivers/net/txgbe/base/txgbe_mng.c:345:9:
>> ../drivers/net/txgbe/base/txgbe_mng.c:145:36:
>> error: array subscript 2 is outside array bounds ofr
>> 'struct txgbe_hic_reset[1]' [-Werror=array-bounds=]
>> 145 | buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi);
>> ../drivers/net/txgbe/base/txgbe_mng.c: In function 'txgbe_hic_reset':
>> ../drivers/net/txgbe/base/txgbe_mng.c:331:32:
>> note: at offset 8 into object 'reset_cmd' of size 8
>> 331 | struct txgbe_hic_reset reset_cmd;
>> | ^~~~~~~~~
>>
>> Access to buffer done based on command code, the case complained by
>> FW_RESET_CMD has short buffer but this code path only taken with command
>> 0x30, so this shouldn't be a problem.
>>
>> Command 0x30 no more used, removing this exception check that cause
>> build error.
>>
>> [1]
>> https://build.opensuse.org/public/build/home:bluca:dpdk/openSUSE_Factory_ARM/armv7l/dpdk-20.11/_log
>>
>> [2]
>> gcc 13.2.1 "cc (SUSE Linux) 13.2.1 20230912
>>
>> Fixes: 35c90ecccfd4 ("net/txgbe: add EEPROM functions")
>> Cc: stable@dpdk.org
>>
>> Reported-by: Luca Boccassi <luca.boccassi@microsoft.com>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@amd.com>>
> Reviewed-by: Jiawen Wu <jiawenwu@trustnetic.com>
>
Applied to dpdk-next-net/main, thanks.
@@ -141,21 +141,7 @@ txgbe_host_interface_command(struct txgbe_hw *hw, u32 *buffer,
for (bi = 0; bi < dword_len; bi++)
buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi);
- /*
- * If there is any thing in data position pull it in
- * Read Flash command requires reading buffer length from
- * two byes instead of one byte
- */
- if (resp->cmd == 0x30) {
- for (; bi < dword_len + 2; bi++)
- buffer[bi] = rd32a(hw, TXGBE_MNGMBX, bi);
-
- buf_len = (((u16)(resp->cmd_or_resp.ret_status) << 3)
- & 0xF00) | resp->buf_len;
- hdr_size += (2 << 2);
- } else {
- buf_len = resp->buf_len;
- }
+ buf_len = resp->buf_len;
if (!buf_len)
goto rel_out;