[dpdk-dev,05/18] drivers: net: nfp: nfpcore: fix strncpy misuse
Checks
Commit Message
---
drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
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
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
>
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!
@@ -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)