* [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy
@ 2017-02-17 14:44 Keith Wiles
2017-02-17 14:48 ` Wiles, Keith
2017-02-17 15:02 ` Bruce Richardson
0 siblings, 2 replies; 6+ messages in thread
From: Keith Wiles @ 2017-02-17 14:44 UTC (permalink / raw)
To: dev
Calling strncpy with a maximum size argument of 16 bytes on destination
array "ifr.ifr_ifrn.ifrn_name" of size 16 bytes might leave the
destination string unterminated.
Signed-off-by: Keith Wiles <keith.wiles@intel.com>
---
drivers/net/tap/rte_eth_tap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index efc4426..f9938d7 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -297,7 +297,7 @@ tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
return -1;
}
memset(&ifr, 0, sizeof(ifr));
- strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
+ strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ-1);
err = ioctl(s, SIOCGIFFLAGS, &ifr);
if (err < 0) {
RTE_LOG(WARNING, PMD, "Unable to get %s device flags: %s\n",
--
2.8.0.GIT
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy
2017-02-17 14:44 [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy Keith Wiles
@ 2017-02-17 14:48 ` Wiles, Keith
2017-02-17 15:02 ` Bruce Richardson
1 sibling, 0 replies; 6+ messages in thread
From: Wiles, Keith @ 2017-02-17 14:48 UTC (permalink / raw)
To: dev
> On Feb 17, 2017, at 8:44 AM, Keith Wiles <keith.wiles@intel.com> wrote:
>
> Calling strncpy with a maximum size argument of 16 bytes on destination
> array "ifr.ifr_ifrn.ifrn_name" of size 16 bytes might leave the
> destination string unterminated.
>
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index efc4426..f9938d7 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -297,7 +297,7 @@ tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
> return -1;
> }
> memset(&ifr, 0, sizeof(ifr));
> - strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
> + strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ-1);
> err = ioctl(s, SIOCGIFFLAGS, &ifr);
> if (err < 0) {
> RTE_LOG(WARNING, PMD, "Unable to get %s device flags: %s\n”,
NAK missed the spaces around ‘-‘ :-(
> --
> 2.8.0.GIT
>
Regards,
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy
2017-02-17 14:44 [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy Keith Wiles
2017-02-17 14:48 ` Wiles, Keith
@ 2017-02-17 15:02 ` Bruce Richardson
2017-02-17 15:05 ` Wiles, Keith
1 sibling, 1 reply; 6+ messages in thread
From: Bruce Richardson @ 2017-02-17 15:02 UTC (permalink / raw)
To: Keith Wiles; +Cc: dev
On Fri, Feb 17, 2017 at 08:44:26AM -0600, Keith Wiles wrote:
> Calling strncpy with a maximum size argument of 16 bytes on destination
> array "ifr.ifr_ifrn.ifrn_name" of size 16 bytes might leave the
> destination string unterminated.
>
> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> ---
> drivers/net/tap/rte_eth_tap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> index efc4426..f9938d7 100644
> --- a/drivers/net/tap/rte_eth_tap.c
> +++ b/drivers/net/tap/rte_eth_tap.c
> @@ -297,7 +297,7 @@ tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
> return -1;
> }
> memset(&ifr, 0, sizeof(ifr));
> - strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
> + strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ-1);
This is why I always prefer to use snprintf for copying strings, you
can't avoid null terminating.
snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
/Bruce
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy
2017-02-17 15:02 ` Bruce Richardson
@ 2017-02-17 15:05 ` Wiles, Keith
2017-02-17 15:13 ` Bruce Richardson
2017-02-17 15:15 ` Ferruh Yigit
0 siblings, 2 replies; 6+ messages in thread
From: Wiles, Keith @ 2017-02-17 15:05 UTC (permalink / raw)
To: Richardson, Bruce; +Cc: dev
> On Feb 17, 2017, at 9:02 AM, Richardson, Bruce <bruce.richardson@intel.com> wrote:
>
> On Fri, Feb 17, 2017 at 08:44:26AM -0600, Keith Wiles wrote:
>> Calling strncpy with a maximum size argument of 16 bytes on destination
>> array "ifr.ifr_ifrn.ifrn_name" of size 16 bytes might leave the
>> destination string unterminated.
>>
>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>> ---
>> drivers/net/tap/rte_eth_tap.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>> index efc4426..f9938d7 100644
>> --- a/drivers/net/tap/rte_eth_tap.c
>> +++ b/drivers/net/tap/rte_eth_tap.c
>> @@ -297,7 +297,7 @@ tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
>> return -1;
>> }
>> memset(&ifr, 0, sizeof(ifr));
>> - strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
>> + strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ-1);
> This is why I always prefer to use snprintf for copying strings, you
> can't avoid null terminating.
Normally I use snprintf to not sure why I reverted to strncpy. Maybe leftover from a previous driver I used as the template.
>
> snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
>
> /Bruce
Regards,
Keith
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy
2017-02-17 15:05 ` Wiles, Keith
@ 2017-02-17 15:13 ` Bruce Richardson
2017-02-17 15:15 ` Ferruh Yigit
1 sibling, 0 replies; 6+ messages in thread
From: Bruce Richardson @ 2017-02-17 15:13 UTC (permalink / raw)
To: Wiles, Keith; +Cc: dev
On Fri, Feb 17, 2017 at 03:05:40PM +0000, Wiles, Keith wrote:
>
> > On Feb 17, 2017, at 9:02 AM, Richardson, Bruce <bruce.richardson@intel.com> wrote:
> >
> > On Fri, Feb 17, 2017 at 08:44:26AM -0600, Keith Wiles wrote:
> >> Calling strncpy with a maximum size argument of 16 bytes on destination
> >> array "ifr.ifr_ifrn.ifrn_name" of size 16 bytes might leave the
> >> destination string unterminated.
> >>
> >> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
> >> ---
> >> drivers/net/tap/rte_eth_tap.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
> >> index efc4426..f9938d7 100644
> >> --- a/drivers/net/tap/rte_eth_tap.c
> >> +++ b/drivers/net/tap/rte_eth_tap.c
> >> @@ -297,7 +297,7 @@ tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
> >> return -1;
> >> }
> >> memset(&ifr, 0, sizeof(ifr));
> >> - strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
> >> + strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ-1);
> > This is why I always prefer to use snprintf for copying strings, you
> > can't avoid null terminating.
>
> Normally I use snprintf to not sure why I reverted to strncpy. Maybe leftover from a previous driver I used as the template.
>
Is there a case to be made that DPDK should provide a strlcpy function
in the linuxapp EAL? [Assuming we don't want a dependency on libbsd?]
I find strncpy a horribly-error prone function to use - worse than
strcpy, since it gives a false sense of safety.
/Bruce
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy
2017-02-17 15:05 ` Wiles, Keith
2017-02-17 15:13 ` Bruce Richardson
@ 2017-02-17 15:15 ` Ferruh Yigit
1 sibling, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2017-02-17 15:15 UTC (permalink / raw)
To: Wiles, Keith, Richardson, Bruce; +Cc: dev
On 2/17/2017 3:05 PM, Wiles, Keith wrote:
>
>> On Feb 17, 2017, at 9:02 AM, Richardson, Bruce <bruce.richardson@intel.com> wrote:
>>
>> On Fri, Feb 17, 2017 at 08:44:26AM -0600, Keith Wiles wrote:
>>> Calling strncpy with a maximum size argument of 16 bytes on destination
>>> array "ifr.ifr_ifrn.ifrn_name" of size 16 bytes might leave the
>>> destination string unterminated.
>>>
>>> Signed-off-by: Keith Wiles <keith.wiles@intel.com>
>>> ---
>>> drivers/net/tap/rte_eth_tap.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
>>> index efc4426..f9938d7 100644
>>> --- a/drivers/net/tap/rte_eth_tap.c
>>> +++ b/drivers/net/tap/rte_eth_tap.c
>>> @@ -297,7 +297,7 @@ tap_link_set_flags(struct pmd_internals *pmd, short flags, int add)
>>> return -1;
>>> }
>>> memset(&ifr, 0, sizeof(ifr));
>>> - strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
>>> + strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ-1);
>> This is why I always prefer to use snprintf for copying strings, you
>> can't avoid null terminating.
>
> Normally I use snprintf to not sure why I reverted to strncpy. Maybe leftover from a previous driver I used as the template.
Since you are already updating that line, do you prefer to convert it to
snprintf instead of above modification?
>
>>
>> snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
>>
>> /Bruce
>
> Regards,
> Keith
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-17 15:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-17 14:44 [dpdk-dev] [PATCH] net/tap: fix coverity warning on strncpy Keith Wiles
2017-02-17 14:48 ` Wiles, Keith
2017-02-17 15:02 ` Bruce Richardson
2017-02-17 15:05 ` Wiles, Keith
2017-02-17 15:13 ` Bruce Richardson
2017-02-17 15:15 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).