[dpdk-dev,05/18] drivers: net: nfp: nfpcore: fix strncpy misuse

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

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail apply issues

Commit Message

Andy Green May 8, 2018, 4:29 a.m. UTC
  ---
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Bruce Richardson May 8, 2018, 8:58 a.m. UTC | #1
On Tue, May 08, 2018 at 12:29:53PM +0800, Andy Green wrote:
> 
> ---
>  drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> index 4e6c66624..9f6704a7f 100644
> --- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> +++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> @@ -846,7 +846,8 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
>  
>  
>  	memset(desc->busdev, 0, BUSDEV_SZ);
> -	strncpy(desc->busdev, devname, strlen(devname));
> +	strncpy(desc->busdev, devname, sizeof(desc->busdev) - 1);
> +	desc->busdev[sizeof(desc->busdev) - 1] = '\0';
>  
>  	ret = nfp_acquire_process_lock(desc);
>  	if (ret)
As with previous patch, a better fix is to use strlcpy. This would apply to
just about all uses of strncpy in the code.

/Bruce
  
Andy Green May 8, 2018, 9 a.m. UTC | #2
On 05/08/2018 04:58 PM, Bruce Richardson wrote:
> On Tue, May 08, 2018 at 12:29:53PM +0800, Andy Green wrote:
>>
>> ---
>>   drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>> index 4e6c66624..9f6704a7f 100644
>> --- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>> +++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>> @@ -846,7 +846,8 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
>>   
>>   
>>   	memset(desc->busdev, 0, BUSDEV_SZ);
>> -	strncpy(desc->busdev, devname, strlen(devname));
>> +	strncpy(desc->busdev, devname, sizeof(desc->busdev) - 1);
>> +	desc->busdev[sizeof(desc->busdev) - 1] = '\0';
>>   
>>   	ret = nfp_acquire_process_lock(desc);
>>   	if (ret)
> As with previous patch, a better fix is to use strlcpy. This would apply to
> just about all uses of strncpy in the code.

OK.

But the strncpy() was already there, it's not introduced by the patch.

I agree just doing it in one hit with strlcpy() is nicer.

-Andy

> /Bruce
>
  
Bruce Richardson May 8, 2018, 9:03 a.m. UTC | #3
On Tue, May 08, 2018 at 05:00:21PM +0800, Andy Green wrote:
> 
> 
> On 05/08/2018 04:58 PM, Bruce Richardson wrote:
> > On Tue, May 08, 2018 at 12:29:53PM +0800, Andy Green wrote:
> > > 
> > > ---
> > >   drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> > > index 4e6c66624..9f6704a7f 100644
> > > --- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> > > +++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> > > @@ -846,7 +846,8 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
> > >   	memset(desc->busdev, 0, BUSDEV_SZ);
> > > -	strncpy(desc->busdev, devname, strlen(devname));
> > > +	strncpy(desc->busdev, devname, sizeof(desc->busdev) - 1);
> > > +	desc->busdev[sizeof(desc->busdev) - 1] = '\0';
> > >   	ret = nfp_acquire_process_lock(desc);
> > >   	if (ret)
> > As with previous patch, a better fix is to use strlcpy. This would apply to
> > just about all uses of strncpy in the code.
> 
> OK.
> 
> But the strncpy() was already there, it's not introduced by the patch.
> 
> I agree just doing it in one hit with strlcpy() is nicer.
> 
> -Andy
> 
Thanks for the fixes!
  

Patch

diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index 4e6c66624..9f6704a7f 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -846,7 +846,8 @@  nfp6000_init(struct nfp_cpp *cpp, const char *devname)
 
 
 	memset(desc->busdev, 0, BUSDEV_SZ);
-	strncpy(desc->busdev, devname, strlen(devname));
+	strncpy(desc->busdev, devname, sizeof(desc->busdev) - 1);
+	desc->busdev[sizeof(desc->busdev) - 1] = '\0';
 
 	ret = nfp_acquire_process_lock(desc);
 	if (ret)