This series fixes potential vulnerabilities in cxgb4 via additional
checks to make sure that we don't corrupt memory.
Maksim Davydov (2):
net/ethernet/chelsio: fix cxgb4_getpgtccfg wrong memory access
net/ethernet/chelsio: t4_handle_fw_rpl fix NULL
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 5 ++++-
drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 3 ++-
2 files changed, 6 insertions(+), 2 deletions(-)
--
2.25.1
*pgid can be in range 0 to 0xF (bitmask 0xF) but valid values for PGID
are between 0 and 7. Also the size of pgrate is 8. Thus, we are needed
additional check to make sure that this code doesn't have access to tsa.
Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.
Fixes: 76bcb31efc06 ("cxgb4 : Add DCBx support codebase and dcbnl_ops")
Signed-off-by: Maksim Davydov <[email protected]>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
index 7d5204834ee2..3aa65f0f335e 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
@@ -471,7 +471,10 @@ static void cxgb4_getpgtccfg(struct net_device *dev, int tc,
return;
}
- *bw_per = pcmd.u.dcb.pgrate.pgrate[*pgid];
+ /* Valid values are: 0-7 */
+ if (*pgid <= 7)
+ *bw_per = pcmd.u.dcb.pgrate.pgrate[*pgid];
+
*up_tc_map = (1 << tc);
/* prio_type is link strict */
--
2.25.1
On Mon, Jan 16, 2023 at 06:20:59PM +0300, Maksim Davydov wrote:
> *pgid can be in range 0 to 0xF (bitmask 0xF) but valid values for PGID
> are between 0 and 7. Also the size of pgrate is 8. Thus, we are needed
> additional check to make sure that this code doesn't have access to tsa.
>
> Found by Linux Verification Center (linuxtesting.org) with the SVACE
> static analysis tool.
>
> Fixes: 76bcb31efc06 ("cxgb4 : Add DCBx support codebase and dcbnl_ops")
> Signed-off-by: Maksim Davydov <[email protected]>
> ---
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
> index 7d5204834ee2..3aa65f0f335e 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
> @@ -471,7 +471,10 @@ static void cxgb4_getpgtccfg(struct net_device *dev, int tc,
> return;
> }
>
> - *bw_per = pcmd.u.dcb.pgrate.pgrate[*pgid];
> + /* Valid values are: 0-7 */
How do you see it?
There are lines below that assume something different.
477 /* prio_type is link strict */
478 if (*pgid != 0xF)
479 *prio_type = 0x2;
> + if (*pgid <= 7)
> + *bw_per = pcmd.u.dcb.pgrate.pgrate[*pgid];
Why do you think that it is valid simply do not set *bw_per?
Thanks
On 1/17/23 12:21, Leon Romanovsky wrote:
> On Mon, Jan 16, 2023 at 06:20:59PM +0300, Maksim Davydov wrote:
>> *pgid can be in range 0 to 0xF (bitmask 0xF) but valid values for PGID
>> are between 0 and 7. Also the size of pgrate is 8. Thus, we are needed
>> additional check to make sure that this code doesn't have access to tsa.
>>
>> Found by Linux Verification Center (linuxtesting.org) with the SVACE
>> static analysis tool.
>>
>> Fixes: 76bcb31efc06 ("cxgb4 : Add DCBx support codebase and dcbnl_ops")
>> Signed-off-by: Maksim Davydov <[email protected]>
>> ---
>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
>> index 7d5204834ee2..3aa65f0f335e 100644
>> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
>> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
>> @@ -471,7 +471,10 @@ static void cxgb4_getpgtccfg(struct net_device *dev, int tc,
>> return;
>> }
>>
>> - *bw_per = pcmd.u.dcb.pgrate.pgrate[*pgid];
>> + /* Valid values are: 0-7 */
> How do you see it?
>
> There are lines below that assume something different.
> 477 /* prio_type is link strict */
> 478 if (*pgid != 0xF)
> 479 *prio_type = 0x2;
>
But if *pgid == 0xF we get value for *bw_per from pgrate.tsa, it seems
not correct
Thanks for reviewing,
Maksim Davydov
>> + if (*pgid <= 7)
>> + *bw_per = pcmd.u.dcb.pgrate.pgrate[*pgid];
> Why do you think that it is valid simply do not set *bw_per?
>
> Thanks
Ping
To summarize:
*pgid can be 0xF, but valid values are 0-7. Whether *bw_per is set or
not set inside if-block, it will have valid value (it was set to zero
before)
On 1/18/23 11:29, Maksim Davydov wrote:
>
> On 1/17/23 12:21, Leon Romanovsky wrote:
>> On Mon, Jan 16, 2023 at 06:20:59PM +0300, Maksim Davydov wrote:
>>> *pgid can be in range 0 to 0xF (bitmask 0xF) but valid values for PGID
>>> are between 0 and 7. Also the size of pgrate is 8. Thus, we are needed
>>> additional check to make sure that this code doesn't have access to
>>> tsa.
>>>
>>> Found by Linux Verification Center (linuxtesting.org) with the SVACE
>>> static analysis tool.
>>>
>>> Fixes: 76bcb31efc06 ("cxgb4 : Add DCBx support codebase and dcbnl_ops")
>>> Signed-off-by: Maksim Davydov <[email protected]>
>>> ---
>>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
>>> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
>>> index 7d5204834ee2..3aa65f0f335e 100644
>>> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
>>> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c
>>> @@ -471,7 +471,10 @@ static void cxgb4_getpgtccfg(struct net_device
>>> *dev, int tc,
>>> return;
>>> }
>>> - *bw_per = pcmd.u.dcb.pgrate.pgrate[*pgid];
>>> + /* Valid values are: 0-7 */
>> How do you see it?
>>
>> There are lines below that assume something different.
>> 477 /* prio_type is link strict */
>> 478 if (*pgid != 0xF)
>> 479 *prio_type = 0x2;
>>
> But if *pgid == 0xF we get value for *bw_per from pgrate.tsa, it seems
> not correct
>
> Thanks for reviewing,
> Maksim Davydov
>>> + if (*pgid <= 7)
>>> + *bw_per = pcmd.u.dcb.pgrate.pgrate[*pgid];
>> Why do you think that it is valid simply do not set *bw_per?
>>
>> Thanks
--
Best regards,
Maksim Davydov