patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
@ 2019-11-19  8:22 Andrew Rybchenko
  2019-11-19 12:24 ` Ferruh Yigit
  2019-11-19 14:57 ` Ferruh Yigit
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Rybchenko @ 2019-11-19  8:22 UTC (permalink / raw)
  To: Thomas Monjalon, Ferruh Yigit; +Cc: dev, stable

memcpy() source and destination areas must not overlap and equal
pointers is the case which is really met, so handle it.

Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
Cc: stable@dpdk.org

Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
slave_configure() in drivers/net/bonding calls rte_eth_dev_configure()
with &slave_eth_dev->data->dev_conf.

Alternative solution is to fix bonding and return error if dev_conf is
equal to &dev->data->dev_conf since usecase is unclear and callers
should not use dev->data.

 lib/librte_ethdev/rte_ethdev.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 8f48e8d659..8d2ce31a81 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -1245,7 +1245,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
 	 * Copy the dev_conf parameter into the dev structure.
 	 * rte_eth_dev_info_get() requires dev_conf, copy it before dev_info get
 	 */
-	memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
+	if (dev_conf != &dev->data->dev_conf)
+		memcpy(&dev->data->dev_conf, dev_conf,
+		       sizeof(dev->data->dev_conf));
 
 	ret = rte_eth_dev_info_get(port_id, &dev_info);
 	if (ret != 0)
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
  2019-11-19  8:22 [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying Andrew Rybchenko
@ 2019-11-19 12:24 ` Ferruh Yigit
  2019-11-19 12:36   ` Andrew Rybchenko
  2019-11-19 14:57 ` Ferruh Yigit
  1 sibling, 1 reply; 9+ messages in thread
From: Ferruh Yigit @ 2019-11-19 12:24 UTC (permalink / raw)
  To: Andrew Rybchenko, Thomas Monjalon; +Cc: dev, stable

On 11/19/2019 8:22 AM, Andrew Rybchenko wrote:
> memcpy() source and destination areas must not overlap and equal
> pointers is the case which is really met, so handle it.

Agree providing same config as input can cause problem with current
implementation, but it is the limitation of the memcpy, the API doesn't request
this.

We can fix as you suggested, in this case we should document this in API
documentation I think,
we can also solve this by updating the implementation to let this, using an
interim buffer in the simplest measure, not sure which one is better.
Any practical reason to prevent this other than 'memcpy' limitation?

> 
> Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
> slave_configure() in drivers/net/bonding calls rte_eth_dev_configure()
> with &slave_eth_dev->data->dev_conf.
> 
> Alternative solution is to fix bonding and return error if dev_conf is
> equal to &dev->data->dev_conf since usecase is unclear and callers
> should not use dev->data.
> 
>  lib/librte_ethdev/rte_ethdev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index 8f48e8d659..8d2ce31a81 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -1245,7 +1245,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>  	 * Copy the dev_conf parameter into the dev structure.
>  	 * rte_eth_dev_info_get() requires dev_conf, copy it before dev_info get
>  	 */
> -	memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
> +	if (dev_conf != &dev->data->dev_conf)
> +		memcpy(&dev->data->dev_conf, dev_conf,
> +		       sizeof(dev->data->dev_conf));
>  
>  	ret = rte_eth_dev_info_get(port_id, &dev_info);
>  	if (ret != 0)
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
  2019-11-19 12:24 ` Ferruh Yigit
@ 2019-11-19 12:36   ` Andrew Rybchenko
  2019-11-19 13:19     ` Ferruh Yigit
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Rybchenko @ 2019-11-19 12:36 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon; +Cc: dev, stable

On 11/19/19 3:24 PM, Ferruh Yigit wrote:
> On 11/19/2019 8:22 AM, Andrew Rybchenko wrote:
>> memcpy() source and destination areas must not overlap and equal
>> pointers is the case which is really met, so handle it.
> Agree providing same config as input can cause problem with current
> implementation, but it is the limitation of the memcpy, the API doesn't request
> this.
>
> We can fix as you suggested, in this case we should document this in API
> documentation I think,

Basically the patch solves it and there is nothing to document.
If pointers are equal there is nothing to do, no copying required.

> we can also solve this by updating the implementation to let this, using an
> interim buffer in the simplest measure, not sure which one is better.

I don't think that interim buffer is required, 'if' perfectly does the job.

> Any practical reason to prevent this other than 'memcpy' limitation?

Nothing except application should not play with dev->data,
but I'm not sure if it is the right place to forbid it.

Alternative solution is to fix bonding and return error if dev_conf is
equal to &dev->data->dev_conf since usecase is unclear and callers
should not use dev->data.

>> Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>> ---
>> slave_configure() in drivers/net/bonding calls rte_eth_dev_configure()
>> with &slave_eth_dev->data->dev_conf.
>>
>> Alternative solution is to fix bonding and return error if dev_conf is
>> equal to &dev->data->dev_conf since usecase is unclear and callers
>> should not use dev->data.
>>
>>  lib/librte_ethdev/rte_ethdev.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index 8f48e8d659..8d2ce31a81 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -1245,7 +1245,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>>  	 * Copy the dev_conf parameter into the dev structure.
>>  	 * rte_eth_dev_info_get() requires dev_conf, copy it before dev_info get
>>  	 */
>> -	memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
>> +	if (dev_conf != &dev->data->dev_conf)
>> +		memcpy(&dev->data->dev_conf, dev_conf,
>> +		       sizeof(dev->data->dev_conf));
>>  
>>  	ret = rte_eth_dev_info_get(port_id, &dev_info);
>>  	if (ret != 0)
>>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
  2019-11-19 12:36   ` Andrew Rybchenko
@ 2019-11-19 13:19     ` Ferruh Yigit
  2019-11-19 13:25       ` Andrew Rybchenko
  2019-11-19 13:37       ` Ferruh Yigit
  0 siblings, 2 replies; 9+ messages in thread
From: Ferruh Yigit @ 2019-11-19 13:19 UTC (permalink / raw)
  To: Andrew Rybchenko, Thomas Monjalon; +Cc: dev, stable

On 11/19/2019 12:36 PM, Andrew Rybchenko wrote:
> On 11/19/19 3:24 PM, Ferruh Yigit wrote:
>> On 11/19/2019 8:22 AM, Andrew Rybchenko wrote:
>>> memcpy() source and destination areas must not overlap and equal
>>> pointers is the case which is really met, so handle it.
>> Agree providing same config as input can cause problem with current
>> implementation, but it is the limitation of the memcpy, the API doesn't request
>> this.
>>
>> We can fix as you suggested, in this case we should document this in API
>> documentation I think,
> 
> Basically the patch solves it and there is nothing to document.
> If pointers are equal there is nothing to do, no copying required.

You are right, scratch my comment. I overlooked as just overlapping memory issue.

> 
>> we can also solve this by updating the implementation to let this, using an
>> interim buffer in the simplest measure, not sure which one is better.
> 
> I don't think that interim buffer is required, 'if' perfectly does the job.
> 
>> Any practical reason to prevent this other than 'memcpy' limitation?
> 
> Nothing except application should not play with dev->data,

+1.
Bonding PMD though not exactly an application, not sure to let or not it to
update 'dev->data'

> but I'm not sure if it is the right place to forbid it.
> 
> Alternative solution is to fix bonding and return error if dev_conf is
> equal to &dev->data->dev_conf since usecase is unclear and callers
> should not use dev->data.
> 
>>> Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>> ---
>>> slave_configure() in drivers/net/bonding calls rte_eth_dev_configure()
>>> with &slave_eth_dev->data->dev_conf.
>>>
>>> Alternative solution is to fix bonding and return error if dev_conf is
>>> equal to &dev->data->dev_conf since usecase is unclear and callers
>>> should not use dev->data.
>>>
>>>  lib/librte_ethdev/rte_ethdev.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>> index 8f48e8d659..8d2ce31a81 100644
>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>> @@ -1245,7 +1245,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>>>  	 * Copy the dev_conf parameter into the dev structure.
>>>  	 * rte_eth_dev_info_get() requires dev_conf, copy it before dev_info get
>>>  	 */
>>> -	memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
>>> +	if (dev_conf != &dev->data->dev_conf)
>>> +		memcpy(&dev->data->dev_conf, dev_conf,
>>> +		       sizeof(dev->data->dev_conf));
>>>  
>>>  	ret = rte_eth_dev_info_get(port_id, &dev_info);
>>>  	if (ret != 0)
>>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
  2019-11-19 13:19     ` Ferruh Yigit
@ 2019-11-19 13:25       ` Andrew Rybchenko
  2019-11-19 13:37       ` Ferruh Yigit
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Rybchenko @ 2019-11-19 13:25 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon; +Cc: dev, stable

On 11/19/19 4:19 PM, Ferruh Yigit wrote:
> On 11/19/2019 12:36 PM, Andrew Rybchenko wrote:
>> On 11/19/19 3:24 PM, Ferruh Yigit wrote:
>>> On 11/19/2019 8:22 AM, Andrew Rybchenko wrote:
>>>> memcpy() source and destination areas must not overlap and equal
>>>> pointers is the case which is really met, so handle it.
>>> Agree providing same config as input can cause problem with current
>>> implementation, but it is the limitation of the memcpy, the API doesn't request
>>> this.
>>>
>>> We can fix as you suggested, in this case we should document this in API
>>> documentation I think,
>>
>> Basically the patch solves it and there is nothing to document.
>> If pointers are equal there is nothing to do, no copying required.
> 
> You are right, scratch my comment. I overlooked as just overlapping memory issue.
> 
>>
>>> we can also solve this by updating the implementation to let this, using an
>>> interim buffer in the simplest measure, not sure which one is better.
>>
>> I don't think that interim buffer is required, 'if' perfectly does the job.
>>
>>> Any practical reason to prevent this other than 'memcpy' limitation?
>>
>> Nothing except application should not play with dev->data,
> 
> +1.
> Bonding PMD though not exactly an application, not sure to let or not it to
> update 'dev->data'

I think it is an application for its slaves, so it is better to have
no exceptions.

>> but I'm not sure if it is the right place to forbid it.
>>
>> Alternative solution is to fix bonding and return error if dev_conf is
>> equal to &dev->data->dev_conf since usecase is unclear and callers
>> should not use dev->data.
>>
>>>> Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> ---
>>>> slave_configure() in drivers/net/bonding calls rte_eth_dev_configure()
>>>> with &slave_eth_dev->data->dev_conf.
>>>>
>>>> Alternative solution is to fix bonding and return error if dev_conf is
>>>> equal to &dev->data->dev_conf since usecase is unclear and callers
>>>> should not use dev->data.
>>>>
>>>>  lib/librte_ethdev/rte_ethdev.c | 4 +++-
>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>>> index 8f48e8d659..8d2ce31a81 100644
>>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>>> @@ -1245,7 +1245,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>>>>  	 * Copy the dev_conf parameter into the dev structure.
>>>>  	 * rte_eth_dev_info_get() requires dev_conf, copy it before dev_info get
>>>>  	 */
>>>> -	memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
>>>> +	if (dev_conf != &dev->data->dev_conf)
>>>> +		memcpy(&dev->data->dev_conf, dev_conf,
>>>> +		       sizeof(dev->data->dev_conf));
>>>>  
>>>>  	ret = rte_eth_dev_info_get(port_id, &dev_info);
>>>>  	if (ret != 0)
>>>>
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
  2019-11-19 13:19     ` Ferruh Yigit
  2019-11-19 13:25       ` Andrew Rybchenko
@ 2019-11-19 13:37       ` Ferruh Yigit
  2019-11-19 13:59         ` Andrew Rybchenko
  1 sibling, 1 reply; 9+ messages in thread
From: Ferruh Yigit @ 2019-11-19 13:37 UTC (permalink / raw)
  To: Andrew Rybchenko, Thomas Monjalon; +Cc: dev, stable

On 11/19/2019 1:19 PM, Ferruh Yigit wrote:
> On 11/19/2019 12:36 PM, Andrew Rybchenko wrote:
>> On 11/19/19 3:24 PM, Ferruh Yigit wrote:
>>> On 11/19/2019 8:22 AM, Andrew Rybchenko wrote:
>>>> memcpy() source and destination areas must not overlap and equal
>>>> pointers is the case which is really met, so handle it.
>>> Agree providing same config as input can cause problem with current
>>> implementation, but it is the limitation of the memcpy, the API doesn't request
>>> this.
>>>
>>> We can fix as you suggested, in this case we should document this in API
>>> documentation I think,
>>
>> Basically the patch solves it and there is nothing to document.
>> If pointers are equal there is nothing to do, no copying required.
> 
> You are right, scratch my comment. I overlooked as just overlapping memory issue.
> 
>>
>>> we can also solve this by updating the implementation to let this, using an
>>> interim buffer in the simplest measure, not sure which one is better.
>>
>> I don't think that interim buffer is required, 'if' perfectly does the job.
>>
>>> Any practical reason to prevent this other than 'memcpy' limitation?
>>
>> Nothing except application should not play with dev->data,
> 
> +1.
> Bonding PMD though not exactly an application, not sure to let or not it to
> update 'dev->data'
> 
>> but I'm not sure if it is the right place to forbid it.
>>
>> Alternative solution is to fix bonding and return error if dev_conf is
>> equal to &dev->data->dev_conf since usecase is unclear and callers
>> should not use dev->data.
>>
>>>> Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> ---
>>>> slave_configure() in drivers/net/bonding calls rte_eth_dev_configure()
>>>> with &slave_eth_dev->data->dev_conf.
>>>>
>>>> Alternative solution is to fix bonding and return error if dev_conf is
>>>> equal to &dev->data->dev_conf since usecase is unclear and callers
>>>> should not use dev->data.

Right now each application should have a copy of each port config in
application, and as we discussed before there is a chance that the application
copy and the PMD copy can diverge here or there.

What do you think having an API to get a copy/clone of the config from the PMD,
later we can introduce the check you mentioned above, use case becomes:
- Get the config from PMD
- Update it
- Configure PMD back with it

This can prevent both application keeping copy and application updating config
directly. Does it make sense?

>>>>
>>>>  lib/librte_ethdev/rte_ethdev.c | 4 +++-
>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>>> index 8f48e8d659..8d2ce31a81 100644
>>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>>> @@ -1245,7 +1245,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>>>>  	 * Copy the dev_conf parameter into the dev structure.
>>>>  	 * rte_eth_dev_info_get() requires dev_conf, copy it before dev_info get
>>>>  	 */
>>>> -	memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
>>>> +	if (dev_conf != &dev->data->dev_conf)
>>>> +		memcpy(&dev->data->dev_conf, dev_conf,
>>>> +		       sizeof(dev->data->dev_conf));
>>>>  
>>>>  	ret = rte_eth_dev_info_get(port_id, &dev_info);
>>>>  	if (ret != 0)
>>>>
>>
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
  2019-11-19 13:37       ` Ferruh Yigit
@ 2019-11-19 13:59         ` Andrew Rybchenko
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Rybchenko @ 2019-11-19 13:59 UTC (permalink / raw)
  To: Ferruh Yigit, Thomas Monjalon; +Cc: dev, stable

On 11/19/19 4:37 PM, Ferruh Yigit wrote:
> On 11/19/2019 1:19 PM, Ferruh Yigit wrote:
>> On 11/19/2019 12:36 PM, Andrew Rybchenko wrote:
>>> On 11/19/19 3:24 PM, Ferruh Yigit wrote:
>>>> On 11/19/2019 8:22 AM, Andrew Rybchenko wrote:
>>>>> memcpy() source and destination areas must not overlap and equal
>>>>> pointers is the case which is really met, so handle it.
>>>> Agree providing same config as input can cause problem with current
>>>> implementation, but it is the limitation of the memcpy, the API doesn't request
>>>> this.
>>>>
>>>> We can fix as you suggested, in this case we should document this in API
>>>> documentation I think,
>>> Basically the patch solves it and there is nothing to document.
>>> If pointers are equal there is nothing to do, no copying required.
>> You are right, scratch my comment. I overlooked as just overlapping memory issue.
>>
>>>> we can also solve this by updating the implementation to let this, using an
>>>> interim buffer in the simplest measure, not sure which one is better.
>>> I don't think that interim buffer is required, 'if' perfectly does the job.
>>>
>>>> Any practical reason to prevent this other than 'memcpy' limitation?
>>> Nothing except application should not play with dev->data,
>> +1.
>> Bonding PMD though not exactly an application, not sure to let or not it to
>> update 'dev->data'
>>
>>> but I'm not sure if it is the right place to forbid it.
>>>
>>> Alternative solution is to fix bonding and return error if dev_conf is
>>> equal to &dev->data->dev_conf since usecase is unclear and callers
>>> should not use dev->data.
>>>
>>>>> Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
>>>>> ---
>>>>> slave_configure() in drivers/net/bonding calls rte_eth_dev_configure()
>>>>> with &slave_eth_dev->data->dev_conf.
>>>>>
>>>>> Alternative solution is to fix bonding and return error if dev_conf is
>>>>> equal to &dev->data->dev_conf since usecase is unclear and callers
>>>>> should not use dev->data.
> Right now each application should have a copy of each port config in
> application, and as we discussed before there is a chance that the application
> copy and the PMD copy can diverge here or there.
>
> What do you think having an API to get a copy/clone of the config from the PMD,
> later we can introduce the check you mentioned above, use case becomes:
> - Get the config from PMD
> - Update it
> - Configure PMD back with it
>
> This can prevent both application keeping copy and application updating config
> directly. Does it make sense?

Yes, it makes sense for me. It will allow testpmd to show actual
configuration from PMD point of view.

Update and configure back approach has obvious problems with
configuration items interdependencies, but it should be the
application headache if it chooses the way since nothing forces
application to do so.

>>>>>  lib/librte_ethdev/rte_ethdev.c | 4 +++-
>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>>>>> index 8f48e8d659..8d2ce31a81 100644
>>>>> --- a/lib/librte_ethdev/rte_ethdev.c
>>>>> +++ b/lib/librte_ethdev/rte_ethdev.c
>>>>> @@ -1245,7 +1245,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
>>>>>  	 * Copy the dev_conf parameter into the dev structure.
>>>>>  	 * rte_eth_dev_info_get() requires dev_conf, copy it before dev_info get
>>>>>  	 */
>>>>> -	memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf));
>>>>> +	if (dev_conf != &dev->data->dev_conf)
>>>>> +		memcpy(&dev->data->dev_conf, dev_conf,
>>>>> +		       sizeof(dev->data->dev_conf));
>>>>>  
>>>>>  	ret = rte_eth_dev_info_get(port_id, &dev_info);
>>>>>  	if (ret != 0)
>>>>>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
  2019-11-19  8:22 [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying Andrew Rybchenko
  2019-11-19 12:24 ` Ferruh Yigit
@ 2019-11-19 14:57 ` Ferruh Yigit
  2019-11-19 15:47   ` Ferruh Yigit
  1 sibling, 1 reply; 9+ messages in thread
From: Ferruh Yigit @ 2019-11-19 14:57 UTC (permalink / raw)
  To: Andrew Rybchenko, Thomas Monjalon; +Cc: dev, stable

On 11/19/2019 8:22 AM, Andrew Rybchenko wrote:
> memcpy() source and destination areas must not overlap and equal
> pointers is the case which is really met, so handle it.
> 
> Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying
  2019-11-19 14:57 ` Ferruh Yigit
@ 2019-11-19 15:47   ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2019-11-19 15:47 UTC (permalink / raw)
  To: Andrew Rybchenko, Thomas Monjalon; +Cc: dev, stable

On 11/19/2019 2:57 PM, Ferruh Yigit wrote:
> On 11/19/2019 8:22 AM, Andrew Rybchenko wrote:
>> memcpy() source and destination areas must not overlap and equal
>> pointers is the case which is really met, so handle it.
>>
>> Fixes: 68b931bff287 ("ethdev: eliminate interim variable")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-11-19 15:47 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-19  8:22 [dpdk-stable] [PATCH] ethdev: avoid undefined behaviour on configuration copying Andrew Rybchenko
2019-11-19 12:24 ` Ferruh Yigit
2019-11-19 12:36   ` Andrew Rybchenko
2019-11-19 13:19     ` Ferruh Yigit
2019-11-19 13:25       ` Andrew Rybchenko
2019-11-19 13:37       ` Ferruh Yigit
2019-11-19 13:59         ` Andrew Rybchenko
2019-11-19 14:57 ` Ferruh Yigit
2019-11-19 15:47   ` 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).