[dpdk-dev,v4,12/18] net/sfc: solve strncpy size and NUL

Message ID 152600317613.53146.6276891139126316271.stgit@localhost.localdomain (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Andy Green May 11, 2018, 1:46 a.m. UTC
  Signed-off-by: Andy Green <andy@warmcat.com>
---
 drivers/net/sfc/sfc_ethdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Andrew Rybchenko May 11, 2018, 8:13 a.m. UTC | #1
On 05/11/2018 04:46 AM, Andy Green wrote:
> Signed-off-by: Andy Green <andy@warmcat.com>
> ---
>   drivers/net/sfc/sfc_ethdev.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
> index ef5e9ecb2..a8c0f8e19 100644
> --- a/drivers/net/sfc/sfc_ethdev.c
> +++ b/drivers/net/sfc/sfc_ethdev.c
> @@ -664,7 +664,7 @@ sfc_xstats_get_names(struct rte_eth_dev *dev,
>   	for (i = 0; i < EFX_MAC_NSTATS; ++i) {
>   		if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
>   			if (xstats_names != NULL && nstats < xstats_count)
> -				strncpy(xstats_names[nstats].name,
> +				strlcpy(xstats_names[nstats].name,
>   					efx_mac_stat_name(sa->nic, i),
>   					sizeof(xstats_names[0].name));
>   			nstats++;
>

I'd suggest:
net/sfc: make sure that copied stats name is NUL-terminated

Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>
  
De Lara Guarch, Pablo May 11, 2018, 10:55 a.m. UTC | #2
> -----Original Message-----

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green

> Sent: Friday, May 11, 2018 2:46 AM

> To: dev@dpdk.org

> Subject: [dpdk-dev] [PATCH v4 12/18] net/sfc: solve strncpy size and NUL

> 

> Signed-off-by: Andy Green <andy@warmcat.com>

> ---

>  drivers/net/sfc/sfc_ethdev.c |    2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index

> ef5e9ecb2..a8c0f8e19 100644

> --- a/drivers/net/sfc/sfc_ethdev.c

> +++ b/drivers/net/sfc/sfc_ethdev.c

> @@ -664,7 +664,7 @@ sfc_xstats_get_names(struct rte_eth_dev *dev,

>  	for (i = 0; i < EFX_MAC_NSTATS; ++i) {

>  		if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {

>  			if (xstats_names != NULL && nstats < xstats_count)

> -				strncpy(xstats_names[nstats].name,

> +				strlcpy(xstats_names[nstats].name,

>  					efx_mac_stat_name(sa->nic, i),

>  					sizeof(xstats_names[0].name));

>  			nstats++;


I'd  say this patch could be squashed with the previous one, as they are solving the same issue
in the same file.
It also needs an extra fixes line (so the final patch would have two fixes lines) and CC stable too.

Fixes: 7b9891769f4b ("net/sfc: support extended statistics")
Cc: stable@dpdk.org
  
Andy Green May 12, 2018, 1:24 a.m. UTC | #3
On 05/11/2018 06:55 PM, De Lara Guarch, Pablo wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
>> Sent: Friday, May 11, 2018 2:46 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH v4 12/18] net/sfc: solve strncpy size and NUL
>>
>> Signed-off-by: Andy Green <andy@warmcat.com>
>> ---
>>   drivers/net/sfc/sfc_ethdev.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c index
>> ef5e9ecb2..a8c0f8e19 100644
>> --- a/drivers/net/sfc/sfc_ethdev.c
>> +++ b/drivers/net/sfc/sfc_ethdev.c
>> @@ -664,7 +664,7 @@ sfc_xstats_get_names(struct rte_eth_dev *dev,
>>   	for (i = 0; i < EFX_MAC_NSTATS; ++i) {
>>   		if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
>>   			if (xstats_names != NULL && nstats < xstats_count)
>> -				strncpy(xstats_names[nstats].name,
>> +				strlcpy(xstats_names[nstats].name,
>>   					efx_mac_stat_name(sa->nic, i),
>>   					sizeof(xstats_names[0].name));
>>   			nstats++;
> 
> I'd  say this patch could be squashed with the previous one, as they are solving the same issue
> in the same file.
> It also needs an extra fixes line (so the final patch would have two fixes lines) and CC stable too.

OK it's adapted accordingly.

-Andy

> Fixes: 7b9891769f4b ("net/sfc: support extended statistics")
> Cc: stable@dpdk.org
>
  

Patch

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index ef5e9ecb2..a8c0f8e19 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -664,7 +664,7 @@  sfc_xstats_get_names(struct rte_eth_dev *dev,
 	for (i = 0; i < EFX_MAC_NSTATS; ++i) {
 		if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
 			if (xstats_names != NULL && nstats < xstats_count)
-				strncpy(xstats_names[nstats].name,
+				strlcpy(xstats_names[nstats].name,
 					efx_mac_stat_name(sa->nic, i),
 					sizeof(xstats_names[0].name));
 			nstats++;