[dpdk-dev] [PATCH v4] net/tap: fix coverity warning on strncpy

[dpdk-dev] [PATCH v4] net/tap: fix coverity warning on strncpy

[Keith Wiles]

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>
---
v4 - Forgot to finish rebase
v3 - convert strncpy to use snprintf instead.
v2 - fix the checkpatch warning no spaces around '-'
v1 - fix coverity warning on strncpy using invalid length.

 drivers/net/tap/rte_eth_tap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c
index efc4426..47a7060 100644
--- a/drivers/net/tap/rte_eth_tap.c
+++ b/drivers/net/tap/rte_eth_tap.c
@@ -129,7 +129,7 @@ tun_alloc(struct pmd_internals *pmd, uint16_t qid)
 	memset(&ifr, 0, sizeof(struct ifreq));
 
 	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
-	strncpy(ifr.ifr_name, pmd->name, IFNAMSIZ);
+	snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
 
 	RTE_LOG(DEBUG, PMD, "ifr_name '%s'\n", ifr.ifr_name);
 
@@ -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);
+	snprintf(ifr.ifr_name, IFNAMSIZ, "%s", pmd->name);
 	err = ioctl(s, SIOCGIFFLAGS, &ifr);
 	if (err < 0) {
 		RTE_LOG(WARNING, PMD, "Unable to get %s device flags: %s\n",
-- 
2.8.0.GIT

Re: [dpdk-dev] [PATCH v4] net/tap: fix coverity warning on strncpy

[Ferruh Yigit]

On 2/17/2017 3:43 PM, 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>

    net/tap: fix possibly unterminated string

    Coverity issue: 1407499
    Fixes: 6b38b2725cdb ("net/tap: fix multi-queue support")
    Cc: stable@dpdk.org

Applied to dpdk-next-net/master, thanks.


(Updates:
- patch title:
It is preferred to mention from problem solved instead of the tool that
found it.

- Added coverity tag:
This helps to trace coverity issues, defined syntax is:

    Coverity issue: xxx
    Fixes: yyyy

- Added Cc: tag for stable tree:
In case stable tree wants get this patch, to make patch visible.
)

Re: [dpdk-dev] [PATCH v4] net/tap: fix coverity warning on strncpy

[Wiles, Keith]

> On Feb 17, 2017, at 10:21 AM, Yigit, Ferruh <ferruh.yigit@intel.com> wrote:
> 
> On 2/17/2017 3:43 PM, 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>
> 
>    net/tap: fix possibly unterminated string
> 
>    Coverity issue: 1407499
>    Fixes: 6b38b2725cdb ("net/tap: fix multi-queue support")
>    Cc: stable@dpdk.org
> 
> Applied to dpdk-next-net/master, thanks.
> 
> 
> (Updates:
> - patch title:
> It is preferred to mention from problem solved instead of the tool that
> found it.
> 
> - Added coverity tag:
> This helps to trace coverity issues, defined syntax is:
> 
>    Coverity issue: xxx
>    Fixes: yyyy
> 
> - Added Cc: tag for stable tree:
> In case stable tree wants get this patch, to make patch visible.

I agree this is good, but to many rules not listed or checked in the tools. We need a much easier method to submit patches in the format that is defined and checked.

Today it is way to hard to know every little internal format for every type of patch. We need to fix this problem to make it easier to submit patches to dpdk.org, we can not continue like this as we grow it will become way to much work for the repo maintainers and the submitter.

Using a better tool then submitting via email seems like a better solution as long as we can add the given checks to the tool. Using a tools should also reduce the email traffic for most everyone, but we need to allow anyone to ask for all of the commits to the repo or pull requests like patches.

How can we handle these types of issues in the future?

> )

Regards,
Keith

Re: [dpdk-dev] [PATCH v4] net/tap: fix coverity warning on strncpy

[Ferruh Yigit]

On 2/17/2017 4:34 PM, Wiles, Keith wrote:
> 
>> On Feb 17, 2017, at 10:21 AM, Yigit, Ferruh <ferruh.yigit@intel.com> wrote:
>>
>> On 2/17/2017 3:43 PM, 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>
>>
>>    net/tap: fix possibly unterminated string
>>
>>    Coverity issue: 1407499
>>    Fixes: 6b38b2725cdb ("net/tap: fix multi-queue support")
>>    Cc: stable@dpdk.org
>>
>> Applied to dpdk-next-net/master, thanks.
>>
>>
>> (Updates:
>> - patch title:
>> It is preferred to mention from problem solved instead of the tool that
>> found it.
>>
>> - Added coverity tag:
>> This helps to trace coverity issues, defined syntax is:
>>
>>    Coverity issue: xxx
>>    Fixes: yyyy
>>
>> - Added Cc: tag for stable tree:
>> In case stable tree wants get this patch, to make patch visible.
> 
> I agree this is good, but to many rules not listed or checked in the tools. We need a much easier method to submit patches in the format that is defined and checked.
> 
> Today it is way to hard to know every little internal format for every type of patch. We need to fix this problem to make it easier to submit patches to dpdk.org, we can not continue like this as we grow it will become way to much work for the repo maintainers and the submitter.

That is why I am documenting what has been changed and the reasoning in
the mail, so I am hoping this is helping others that following the mail
list sync about rules. Also gives a discussion medium about rules..

> 
> Using a better tool then submitting via email seems like a better solution as long as we can add the given checks to the tool. Using a tools should also reduce the email traffic for most everyone, but we need to allow anyone to ask for all of the commits to the repo or pull requests like patches.
> 
> How can we handle these types of issues in the future?
> 
>> )
> 
> Regards,
> Keith
> 

Re: [dpdk-dev] [PATCH v4] net/tap: fix coverity warning on strncpy

[Wiles, Keith]

> On Feb 17, 2017, at 10:48 AM, Yigit, Ferruh <ferruh.yigit@intel.com> wrote:
> 
> On 2/17/2017 4:34 PM, Wiles, Keith wrote:
>> 
>>> On Feb 17, 2017, at 10:21 AM, Yigit, Ferruh <ferruh.yigit@intel.com> wrote:
>>> 
>>> On 2/17/2017 3:43 PM, 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>
>>> 
>>>   net/tap: fix possibly unterminated string
>>> 
>>>   Coverity issue: 1407499
>>>   Fixes: 6b38b2725cdb ("net/tap: fix multi-queue support")
>>>   Cc: stable@dpdk.org
>>> 
>>> Applied to dpdk-next-net/master, thanks.
>>> 
>>> 
>>> (Updates:
>>> - patch title:
>>> It is preferred to mention from problem solved instead of the tool that
>>> found it.
>>> 
>>> - Added coverity tag:
>>> This helps to trace coverity issues, defined syntax is:
>>> 
>>>   Coverity issue: xxx
>>>   Fixes: yyyy
>>> 
>>> - Added Cc: tag for stable tree:
>>> In case stable tree wants get this patch, to make patch visible.
>> 
>> I agree this is good, but to many rules not listed or checked in the tools. We need a much easier method to submit patches in the format that is defined and checked.
>> 
>> Today it is way to hard to know every little internal format for every type of patch. We need to fix this problem to make it easier to submit patches to dpdk.org, we can not continue like this as we grow it will become way to much work for the repo maintainers and the submitter.
> 
> That is why I am documenting what has been changed and the reasoning in
> the mail, so I am hoping this is helping others that following the mail
> list sync about rules. Also gives a discussion medium about rules..

Great, but we need to document this on the web site not in email. We can not expect someone (or a new person) to read millions of emails to find these hidden gems, right?

> 
>> 
>> Using a better tool then submitting via email seems like a better solution as long as we can add the given checks to the tool. Using a tools should also reduce the email traffic for most everyone, but we need to allow anyone to ask for all of the commits to the repo or pull requests like patches.
>> 
>> How can we handle these types of issues in the future?
>> 
>>> )
>> 
>> Regards,
>> Keith

Regards,
Keith