[dpdk-dev] vfio: do not use memcmp() to compare PCI address

Message ID 1490975813-6700-1-git-send-email-arybchenko@solarflare.com (mailing list archive)
State Not Applicable, archived
Headers

Checks

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

Commit Message

Andrew Rybchenko March 31, 2017, 3:56 p.m. UTC
  PCI address structure has padding which may have garbage.

Fixes: 2f4adfad0a69 ("vfio: add multiprocess support")
Cc: stable@dpdk.org

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
It is a real bug which I've hit during multi-process debugging.

 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

Comments

Sergio Gonzalez Monroy March 31, 2017, 4:09 p.m. UTC | #1
On 31/03/2017 16:56, Andrew Rybchenko wrote:
> PCI address structure has padding which may have garbage.
>
> Fixes: 2f4adfad0a69 ("vfio: add multiprocess support")
> Cc: stable@dpdk.org
>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> It is a real bug which I've hit during multi-process debugging.
>
>   lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> index 5f478c5..7d8b9fb 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> @@ -355,7 +355,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
>   	} else {
>   		/* if we're in a secondary process, just find our tailq entry */
>   		TAILQ_FOREACH(vfio_res, vfio_res_list, next) {
> -			if (memcmp(&vfio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
> +			if (rte_eal_compare_pci_addr(&vfio_res->pci_addr,
> +						     &dev->addr))
>   				continue;
>   			break;
>   		}

Different commit, same patch :)
http://dpdk.org/dev/patchwork/patch/21828/

Regards,
Sergio
  
Andrew Rybchenko March 31, 2017, 4:18 p.m. UTC | #2
On 03/31/2017 07:09 PM, Sergio Gonzalez Monroy wrote:
> On 31/03/2017 16:56, Andrew Rybchenko wrote:
>> PCI address structure has padding which may have garbage.
>>
>> Fixes: 2f4adfad0a69 ("vfio: add multiprocess support")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>> It is a real bug which I've hit during multi-process debugging.
>>
>>   lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c 
>> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>> index 5f478c5..7d8b9fb 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>> @@ -355,7 +355,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
>>       } else {
>>           /* if we're in a secondary process, just find our tailq 
>> entry */
>>           TAILQ_FOREACH(vfio_res, vfio_res_list, next) {
>> -            if (memcmp(&vfio_res->pci_addr, &dev->addr, 
>> sizeof(dev->addr)))
>> +            if (rte_eal_compare_pci_addr(&vfio_res->pci_addr,
>> +                             &dev->addr))
>>                   continue;
>>               break;
>>           }
>
> Different commit, same patch :)
> https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_dev_patchwork_patch_21828_&d=DQICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=flTOx6Av679My7o_iScb5sOlLD68bpUyE2RUtfW3SWQ&m=uvsF8SJaLsRNp3l01Ji5iD4EB1hkHVPOX_dFKqTu_mU&s=HYxvqVvNsbEzQgMJmwQuH6G7vqOkyg7vJ1PrzsxKr5c&e= 


True, sorry, lost your patch from my view.
I'm not sure which state should be set on my patch in the patchwork. 
Thomas, please, resolve it properly.

Thanks,
Andrew.
  
Sergio Gonzalez Monroy March 31, 2017, 4:24 p.m. UTC | #3
On 31/03/2017 17:18, Andrew Rybchenko wrote:
> On 03/31/2017 07:09 PM, Sergio Gonzalez Monroy wrote:
>> On 31/03/2017 16:56, Andrew Rybchenko wrote:
>>> PCI address structure has padding which may have garbage.
>>>
>>> Fixes: 2f4adfad0a69 ("vfio: add multiprocess support")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> ---
>>> It is a real bug which I've hit during multi-process debugging.
>>>
>>>   lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c 
>>> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>>> index 5f478c5..7d8b9fb 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>>> @@ -355,7 +355,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
>>>       } else {
>>>           /* if we're in a secondary process, just find our tailq 
>>> entry */
>>>           TAILQ_FOREACH(vfio_res, vfio_res_list, next) {
>>> -            if (memcmp(&vfio_res->pci_addr, &dev->addr, 
>>> sizeof(dev->addr)))
>>> +            if (rte_eal_compare_pci_addr(&vfio_res->pci_addr,
>>> +                             &dev->addr))
>>>                   continue;
>>>               break;
>>>           }
>>
>> Different commit, same patch :)
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__dpdk.org_dev_patchwork_patch_21828_&d=DQICaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=flTOx6Av679My7o_iScb5sOlLD68bpUyE2RUtfW3SWQ&m=uvsF8SJaLsRNp3l01Ji5iD4EB1hkHVPOX_dFKqTu_mU&s=HYxvqVvNsbEzQgMJmwQuH6G7vqOkyg7vJ1PrzsxKr5c&e= 
>
>
> True, sorry, lost your patch from my view.
> I'm not sure which state should be set on my patch in the patchwork. 
> Thomas, please, resolve it properly.
>

No worries. Probably the title on the other patch was a bit misleading.

You could just set it to the "Not Applicable" state.

Sergio

> Thanks,
> Andrew.
  

Patch

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 5f478c5..7d8b9fb 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -355,7 +355,8 @@  pci_vfio_map_resource(struct rte_pci_device *dev)
 	} else {
 		/* if we're in a secondary process, just find our tailq entry */
 		TAILQ_FOREACH(vfio_res, vfio_res_list, next) {
-			if (memcmp(&vfio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
+			if (rte_eal_compare_pci_addr(&vfio_res->pci_addr,
+						     &dev->addr))
 				continue;
 			break;
 		}