[dpdk-dev,v5,03/16] bus/pci: replace strncpy dangerous code

Message ID 152608967931.121204.3086768884047081432.stgit@localhost.localdomain (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

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

Commit Message

Andy Green May 12, 2018, 1:47 a.m. UTC
  In function ‘pci_get_kernel_driver_by_path’,
    inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/
	drivers/bus/pci/linux/pci.c:317:8:
/home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:
‘strncpy’ specified bound depends on the length of the source argument
[-Werror=stringop-overflow=]
   strncpy(dri_name, name + 1, strlen(name + 1) + 1);

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Fixes: d9a8cd9595f2 ("pci: add kernel driver type")
Cc: stable@dpdk.org
---
 drivers/bus/pci/linux/pci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Yao, Lei A May 15, 2018, 6:12 a.m. UTC | #1
Hi, Andy

This patch will break the vfio-pci driver on my server. 
I can't launch NIC with vfio-pci using testpmd.  Could you have 
a check on this? Thanks a lot!

My server info:
OS: Ubuntu 16.04 LTS
gcc: 5.4.0
kernel: 4.4.0
CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
NIC: Ethernet Controller X710 for 10GbE SFP+ 

My Step:
1. Bind NIC to vfio-pci driver
modprobe vfio-pci
dpdk-devbind.py -b vfio-pci [PCI address of NIC]

2. Launch testpmd;
./x86_64-native-linuxapp-gcc/app/testpmd -c 0x03 -n 4 -- -i

> -----Original Message-----

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

> Sent: Saturday, May 12, 2018 9:48 AM

> To: dev@dpdk.org

> Subject: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous

> code

> 

> In function ‘pci_get_kernel_driver_by_path’,

>     inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/

> 	drivers/bus/pci/linux/pci.c:317:8:

> /home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:

> ‘strncpy’ specified bound depends on the length of the source argument

> [-Werror=stringop-overflow=]

>    strncpy(dri_name, name + 1, strlen(name + 1) + 1);

> 

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

> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

> Fixes: d9a8cd9595f2 ("pci: add kernel driver type")

> Cc: stable@dpdk.org

> ---

>  drivers/bus/pci/linux/pci.c |    2 +-

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

> 

> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c

> index 4630a8057..a73ee49c2 100644

> --- a/drivers/bus/pci/linux/pci.c

> +++ b/drivers/bus/pci/linux/pci.c

> @@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char *filename,

> char *dri_name)

> 

>  	name = strrchr(path, '/');

>  	if (name) {

> -		strncpy(dri_name, name + 1, strlen(name + 1) + 1);

> +		strlcpy(dri_name, name + 1, sizeof(dri_name));

>  		return 0;

>  	}

>
  
Andy Green May 15, 2018, 7:32 a.m. UTC | #2
On 05/15/2018 02:12 PM, Yao, Lei A wrote:
> Hi, Andy
> 
> This patch will break the vfio-pci driver on my server.
> I can't launch NIC with vfio-pci using testpmd.  Could you have
> a check on this? Thanks a lot!
> 
> My server info:
> OS: Ubuntu 16.04 LTS
> gcc: 5.4.0
> kernel: 4.4.0
> CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
> NIC: Ethernet Controller X710 for 10GbE SFP+
> 
> My Step:
> 1. Bind NIC to vfio-pci driver
> modprobe vfio-pci
> dpdk-devbind.py -b vfio-pci [PCI address of NIC]
> 
> 2. Launch testpmd;
> ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x03 -n 4 -- -i

I don't have any nic to test with.

But it doesn't matter the patch is indeed wrong... I just sent you and 
the list a fix on top of the incomplete patch.  "bus/pci: correct the 
earlier strlcpy conversion"

Sorry...

-Andy

>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
>> Sent: Saturday, May 12, 2018 9:48 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous
>> code
>>
>> In function ‘pci_get_kernel_driver_by_path’,
>>      inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/
>> 	drivers/bus/pci/linux/pci.c:317:8:
>> /home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:
>> ‘strncpy’ specified bound depends on the length of the source argument
>> [-Werror=stringop-overflow=]
>>     strncpy(dri_name, name + 1, strlen(name + 1) + 1);
>>
>> Signed-off-by: Andy Green <andy@warmcat.com>
>> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>> Fixes: d9a8cd9595f2 ("pci: add kernel driver type")
>> Cc: stable@dpdk.org
>> ---
>>   drivers/bus/pci/linux/pci.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
>> index 4630a8057..a73ee49c2 100644
>> --- a/drivers/bus/pci/linux/pci.c
>> +++ b/drivers/bus/pci/linux/pci.c
>> @@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char *filename,
>> char *dri_name)
>>
>>   	name = strrchr(path, '/');
>>   	if (name) {
>> -		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
>> +		strlcpy(dri_name, name + 1, sizeof(dri_name));
>>   		return 0;
>>   	}
>>
>
  
Yao, Lei A May 15, 2018, 8:18 a.m. UTC | #3
> -----Original Message-----

> From: Andy Green [mailto:andy@warmcat.com]

> Sent: Tuesday, May 15, 2018 3:33 PM

> To: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org

> Subject: Re: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous

> code

> 

> 

> 

> On 05/15/2018 02:12 PM, Yao, Lei A wrote:

> > Hi, Andy

> >

> > This patch will break the vfio-pci driver on my server.

> > I can't launch NIC with vfio-pci using testpmd.  Could you have

> > a check on this? Thanks a lot!

> >

> > My server info:

> > OS: Ubuntu 16.04 LTS

> > gcc: 5.4.0

> > kernel: 4.4.0

> > CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz

> > NIC: Ethernet Controller X710 for 10GbE SFP+

> >

> > My Step:

> > 1. Bind NIC to vfio-pci driver

> > modprobe vfio-pci

> > dpdk-devbind.py -b vfio-pci [PCI address of NIC]

> >

> > 2. Launch testpmd;

> > ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x03 -n 4 -- -i

> 

> I don't have any nic to test with.

> 

> But it doesn't matter the patch is indeed wrong... I just sent you and

> the list a fix on top of the incomplete patch.  "bus/pci: correct the

> earlier strlcpy conversion"

> 

> Sorry...

> 

> -Andy

> 

Hi, Andy

Thanks a lot for your quick fix. It can work on my server now. 

BRs
Lei

> >> -----Original Message-----

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

> >> Sent: Saturday, May 12, 2018 9:48 AM

> >> To: dev@dpdk.org

> >> Subject: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous

> >> code

> >>

> >> In function ‘pci_get_kernel_driver_by_path’,

> >>      inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/

> >> 	drivers/bus/pci/linux/pci.c:317:8:

> >> /home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:

> >> ‘strncpy’ specified bound depends on the length of the source argument

> >> [-Werror=stringop-overflow=]

> >>     strncpy(dri_name, name + 1, strlen(name + 1) + 1);

> >>

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

> >> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

> >> Fixes: d9a8cd9595f2 ("pci: add kernel driver type")

> >> Cc: stable@dpdk.org

> >> ---

> >>   drivers/bus/pci/linux/pci.c |    2 +-

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

> >>

> >> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c

> >> index 4630a8057..a73ee49c2 100644

> >> --- a/drivers/bus/pci/linux/pci.c

> >> +++ b/drivers/bus/pci/linux/pci.c

> >> @@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char

> *filename,

> >> char *dri_name)

> >>

> >>   	name = strrchr(path, '/');

> >>   	if (name) {

> >> -		strncpy(dri_name, name + 1, strlen(name + 1) + 1);

> >> +		strlcpy(dri_name, name + 1, sizeof(dri_name));

> >>   		return 0;

> >>   	}

> >>

> >
  

Patch

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 4630a8057..a73ee49c2 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -54,7 +54,7 @@  pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
 
 	name = strrchr(path, '/');
 	if (name) {
-		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
+		strlcpy(dri_name, name + 1, sizeof(dri_name));
 		return 0;
 	}