[Intel-wired-lan] [PATCH] ixgbe: add array bounds check

[Tantilov, Emil S]

>-----Original Message-----
>From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
>Behalf Of Alexander Duyck
>Sent: Tuesday, October 24, 2017 9:20 AM
>To: Greenwalt, Paul <paul.greenwalt at intel.com>
>Cc: intel-wired-lan <intel-wired-lan at lists.osuosl.org>
>Subject: Re: [Intel-wired-lan] [PATCH] ixgbe: add array bounds check
>
>On Mon, Oct 23, 2017 at 8:21 AM, Paul Greenwalt
><paul.greenwalt at intel.com> wrote:
>> Add bounds check for index fcoe_i of rx_ring array.
>>
>> Signed-off-by: Paul Greenwalt <paul.greenwalt at intel.com>
>> ---
>>  drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
>> index a23c2b5..5772fe2 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_fcoe.c
>> @@ -698,6 +698,8 @@ void ixgbe_configure_fcoe(struct ixgbe_adapter
>*adapter)
>>                 }
>>
>>                 fcoe_i = fcoe->offset + (i % fcoe->indices);
>> +               if (fcoe_i >= MAX_RX_QUEUES)
>> +                       fcoe_i = MAX_RX_QUEUES - 1;
>
>Instead of using MAX_RX_QUEUES why use fcoe->limit? If I am not
>mistake the upper limit is either 8 or 32 for the FCoE redirection
>table anyway so odds are you can't even use 64 queues if you wanted
>to. From what I can tell the only advantage of the approach you are
>using now is that you don't potentially zero out the value when it
>gets hit with the mask below.
>
>>                 fcoe_i &= IXGBE_FCRETA_ENTRY_MASK;

Actually the check is added to protect from accessing rx_ring[fcoe_i] beyond 
it's boundaries. The above line was supposed to be removed. This is because
IXGBE_FCRETA_ENTRY_MASK = 0x7f, and static checkers report possible out of
bounds issue.

Thanks,
Emil