[PATCH net 0/2] cxgb4: fix memory vulnerabilities

[PATCH net 0/2] cxgb4: fix memory vulnerabilities

[Maksim Davydov]

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

[PATCH net 1/2] net/ethernet/chelsio: fix cxgb4_getpgtccfg wrong memory access

[Maksim Davydov]

*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 <davydov-max@yandex-team.ru>
---
 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

[PATCH net 2/2] net/ethernet/chelsio: t4_handle_fw_rpl fix NULL

[Maksim Davydov]

After t4_hw.c:t4_prep_adapter() that is called in cxgb4_main.c:init_one()
adapter has it least 1 port for debug. Thus, for_each_port() usually has
at least 1 iteration, but this function can be called with wrong
configured adapter

Found by Linux Verification Center (linuxtesting.org) with the SVACE
static analysis tool.

Fixes: 23853a0a9a76 ("cxgb4: Don't assume FW_PORT_CMD reply is always
port info msg")

Signed-off-by: Maksim Davydov <davydov-max@yandex-team.ru>
---
 drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index 8d719f82854a..2f7b49473f52 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -8864,7 +8864,8 @@ int t4_handle_fw_rpl(struct adapter *adap, const __be64 *rpl)
 				break;
 		}
 
-		t4_handle_get_port_info(pi, rpl);
+		if (pi)
+			t4_handle_get_port_info(pi, rpl);
 	} else {
 		dev_warn(adap->pdev_dev, "Unknown firmware reply %d\n",
 			 opcode);
-- 
2.25.1

Re: [PATCH net 1/2] net/ethernet/chelsio: fix cxgb4_getpgtccfg wrong memory access

[Leon Romanovsky]

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 <davydov-max@yandex-team.ru>
> ---
>  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

Re: [PATCH net 2/2] net/ethernet/chelsio: t4_handle_fw_rpl fix NULL

[Leon Romanovsky]

On Mon, Jan 16, 2023 at 06:21:00PM +0300, Maksim Davydov wrote:
> After t4_hw.c:t4_prep_adapter() that is called in cxgb4_main.c:init_one()
> adapter has it least 1 port for debug. 

IMHO it is wrong to keep this interface and the logic for this debug
code should be deleted.

Thanks

Re: [PATCH net 1/2] net/ethernet/chelsio: fix cxgb4_getpgtccfg wrong memory access

[Maksim Davydov]

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 <davydov-max@yandex-team.ru>
>> ---
>>   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

Re: [PATCH net 1/2] net/ethernet/chelsio: fix cxgb4_getpgtccfg wrong memory access

[Maksim Davydov]

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 <davydov-max@yandex-team.ru>
>>> ---
>>>   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