2021-04-13 23:12:14

by Gustavo A. R. Silva

[permalink] [raw]
Subject: [PATCH][next] ixgbe: Fix out-bounds warning in ixgbe_host_interface_command()

Replace union with a couple of pointers in order to fix the following
out-of-bounds warning:

CC [M] drivers/net/ethernet/intel/ixgbe/ixgbe_common.o
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c: In function ‘ixgbe_host_interface_command’:
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c:3729:13: warning: array subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-bounds]
3729 | bp->u32arr[bi] = IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, bi);
| ~~~~~~~~~~^~~~
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c:3682:7: note: while referencing ‘u32arr’
3682 | u32 u32arr[1];
| ^~~~~~

This helps with the ongoing efforts to globally enable -Warray-bounds.

Link: https://github.com/KSPP/linux/issues/109
Co-developed-by: Kees Cook <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 03ccbe6b66d2..e90b5047e695 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -3678,10 +3678,8 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *buffer,
bool return_data)
{
u32 hdr_size = sizeof(struct ixgbe_hic_hdr);
- union {
- struct ixgbe_hic_hdr hdr;
- u32 u32arr[1];
- } *bp = buffer;
+ struct ixgbe_hic_hdr *hdr = buffer;
+ u32 *u32arr = buffer;
u16 buf_len, dword_len;
s32 status;
u32 bi;
@@ -3707,12 +3705,12 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *buffer,

/* first pull in the header so we know the buffer length */
for (bi = 0; bi < dword_len; bi++) {
- bp->u32arr[bi] = IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, bi);
- le32_to_cpus(&bp->u32arr[bi]);
+ u32arr[bi] = IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, bi);
+ le32_to_cpus(&u32arr[bi]);
}

/* If there is any thing in data position pull it in */
- buf_len = bp->hdr.buf_len;
+ buf_len = hdr->buf_len;
if (!buf_len)
goto rel_out;

@@ -3727,8 +3725,8 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *buffer,

/* Pull in the rest of the buffer (bi is where we left off) */
for (; bi <= dword_len; bi++) {
- bp->u32arr[bi] = IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, bi);
- le32_to_cpus(&bp->u32arr[bi]);
+ u32arr[bi] = IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, bi);
+ le32_to_cpus(&u32arr[bi]);
}

rel_out:
--
2.27.0


2021-05-06 07:27:36

by Switzer, David

[permalink] [raw]
Subject: RE: [Intel-wired-lan] [PATCH][next] ixgbe: Fix out-bounds warning in ixgbe_host_interface_command()


>-----Original Message-----
>From: Intel-wired-lan <[email protected]> On Behalf Of
>Gustavo A. R. Silva
>Sent: Tuesday, April 13, 2021 12:04 PM
>To: Brandeburg, Jesse <[email protected]>; Nguyen, Anthony L
><[email protected]>; David S. Miller <[email protected]>; Jakub
>Kicinski <[email protected]>
>Cc: Kees Cook <[email protected]>; [email protected]; linux-
>[email protected]; Gustavo A. R. Silva <[email protected]>; intel-
>[email protected]; [email protected]
>Subject: [Intel-wired-lan] [PATCH][next] ixgbe: Fix out-bounds warning in
>ixgbe_host_interface_command()
>
>Replace union with a couple of pointers in order to fix the following out-of-
>bounds warning:
>
> CC [M] drivers/net/ethernet/intel/ixgbe/ixgbe_common.o
>drivers/net/ethernet/intel/ixgbe/ixgbe_common.c: In function
>‘ixgbe_host_interface_command’:
>drivers/net/ethernet/intel/ixgbe/ixgbe_common.c:3729:13: warning: array
>subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-
>bounds]
> 3729 | bp->u32arr[bi] = IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, bi);
> | ~~~~~~~~~~^~~~
>drivers/net/ethernet/intel/ixgbe/ixgbe_common.c:3682:7: note: while
>referencing ‘u32arr’
> 3682 | u32 u32arr[1];
> | ^~~~~~
>
>This helps with the ongoing efforts to globally enable -Warray-bounds.
>
>Link: https://github.com/KSPP/linux/issues/109
>Co-developed-by: Kees Cook <[email protected]>
>Signed-off-by: Kees Cook <[email protected]>
>Signed-off-by: Gustavo A. R. Silva <[email protected]>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
Tested-by: Dave Switzer <[email protected]>

2021-05-11 15:51:13

by Gustavo A. R. Silva

[permalink] [raw]
Subject: Re: [Intel-wired-lan] [PATCH][next] ixgbe: Fix out-bounds warning in ixgbe_host_interface_command()

Hi all,

On 5/6/21 02:25, Switzer, David wrote:
>
>> -----Original Message-----
>> From: Intel-wired-lan <[email protected]> On Behalf Of
>> Gustavo A. R. Silva
>> Sent: Tuesday, April 13, 2021 12:04 PM
>> To: Brandeburg, Jesse <[email protected]>; Nguyen, Anthony L
>> <[email protected]>; David S. Miller <[email protected]>; Jakub
>> Kicinski <[email protected]>
>> Cc: Kees Cook <[email protected]>; [email protected]; linux-
>> [email protected]; Gustavo A. R. Silva <[email protected]>; intel-
>> [email protected]; [email protected]
>> Subject: [Intel-wired-lan] [PATCH][next] ixgbe: Fix out-bounds warning in
>> ixgbe_host_interface_command()
>>
>> Replace union with a couple of pointers in order to fix the following out-of-
>> bounds warning:
>>
>> CC [M] drivers/net/ethernet/intel/ixgbe/ixgbe_common.o
>> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c: In function
>> ‘ixgbe_host_interface_command’:
>> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c:3729:13: warning: array
>> subscript 1 is above array bounds of ‘u32[1]’ {aka ‘unsigned int[1]’} [-Warray-
>> bounds]
>> 3729 | bp->u32arr[bi] = IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG, bi);
>> | ~~~~~~~~~~^~~~
>> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c:3682:7: note: while
>> referencing ‘u32arr’
>> 3682 | u32 u32arr[1];
>> | ^~~~~~
>>
>> This helps with the ongoing efforts to globally enable -Warray-bounds.
>>
>> Link: https://github.com/KSPP/linux/issues/109
>> Co-developed-by: Kees Cook <[email protected]>
>> Signed-off-by: Kees Cook <[email protected]>
>> Signed-off-by: Gustavo A. R. Silva <[email protected]>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 16 +++++++---------
>> 1 file changed, 7 insertions(+), 9 deletions(-)
>>
> Tested-by: Dave Switzer <[email protected]>

Thanks for this, Dave. :)

By the way, we are about to be able to globally enable -Warray-bounds and,
this is one of the last out-of-bounds warnings in linux-next.

Thanks
--
Gustavo