patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
       [not found] ` <20220725203206.427083-1-david.marchand@redhat.com>
@ 2022-07-25 20:32   ` David Marchand
  2022-07-26  7:55     ` Maxime Coquelin
  0 siblings, 1 reply; 9+ messages in thread
From: David Marchand @ 2022-07-25 20:32 UTC (permalink / raw)
  To: dev; +Cc: stable, Maxime Coquelin, Chenbo Xia

translate_ring_addresses (via numa_realloc) may change a virtio device and
virtio queue.
The virtqueue object must be refreshed before accessing the lock.

Fixes: 04c27cb673b9 ("vhost: fix unsafe vring addresses modifications")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 lib/vhost/vhost_user.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 4ad28bac45..91d40e32fc 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -2596,6 +2596,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev,
 			if (is_vring_iotlb(dev, vq, imsg)) {
 				rte_spinlock_lock(&vq->access_lock);
 				*pdev = dev = translate_ring_addresses(dev, i);
+				vq = dev->virtqueue[i];
 				rte_spinlock_unlock(&vq->access_lock);
 			}
 		}
-- 
2.36.1


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

* Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
  2022-07-25 20:32   ` [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation David Marchand
@ 2022-07-26  7:55     ` Maxime Coquelin
  2022-09-13 15:02       ` Maxime Coquelin
  0 siblings, 1 reply; 9+ messages in thread
From: Maxime Coquelin @ 2022-07-26  7:55 UTC (permalink / raw)
  To: David Marchand, dev; +Cc: stable, Chenbo Xia



On 7/25/22 22:32, David Marchand wrote:
> translate_ring_addresses (via numa_realloc) may change a virtio device and
> virtio queue.
> The virtqueue object must be refreshed before accessing the lock.
> 
> Fixes: 04c27cb673b9 ("vhost: fix unsafe vring addresses modifications")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>   lib/vhost/vhost_user.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> index 4ad28bac45..91d40e32fc 100644
> --- a/lib/vhost/vhost_user.c
> +++ b/lib/vhost/vhost_user.c
> @@ -2596,6 +2596,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev,
>   			if (is_vring_iotlb(dev, vq, imsg)) {
>   				rte_spinlock_lock(&vq->access_lock);
>   				*pdev = dev = translate_ring_addresses(dev, i);
> +				vq = dev->virtqueue[i];
>   				rte_spinlock_unlock(&vq->access_lock);
>   			}
>   		}

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime


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

* Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
  2022-07-26  7:55     ` Maxime Coquelin
@ 2022-09-13 15:02       ` Maxime Coquelin
  2022-09-14  1:05         ` Xia, Chenbo
  0 siblings, 1 reply; 9+ messages in thread
From: Maxime Coquelin @ 2022-09-13 15:02 UTC (permalink / raw)
  To: David Marchand, Chenbo Xia, Thomas Monjalon; +Cc: stable, dev

Hi,

On 7/26/22 09:55, Maxime Coquelin wrote:
> 
> 
> On 7/25/22 22:32, David Marchand wrote:
>> translate_ring_addresses (via numa_realloc) may change a virtio device 
>> and
>> virtio queue.
>> The virtqueue object must be refreshed before accessing the lock.
>>
>> Fixes: 04c27cb673b9 ("vhost: fix unsafe vring addresses modifications")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>> ---
>>   lib/vhost/vhost_user.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
>> index 4ad28bac45..91d40e32fc 100644
>> --- a/lib/vhost/vhost_user.c
>> +++ b/lib/vhost/vhost_user.c
>> @@ -2596,6 +2596,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev,
>>               if (is_vring_iotlb(dev, vq, imsg)) {
>>                   rte_spinlock_lock(&vq->access_lock);
>>                   *pdev = dev = translate_ring_addresses(dev, i);
>> +                vq = dev->virtqueue[i];
>>                   rte_spinlock_unlock(&vq->access_lock);
>>               }
>>           }
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks,
> Maxime

The bug this patch is fixing is being reproduced downstream.
It would be great it gets merged in main branch rapidly so that we can
perform the backport.

Chenbo, are you planning a pull request for vhost/virtio in the next few
days? If not, should the main branch maintainer pick this single patch
directly and let the rest of the series more time for reviews?

Thanks,
Maxime


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

* RE: [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
  2022-09-13 15:02       ` Maxime Coquelin
@ 2022-09-14  1:05         ` Xia, Chenbo
  2022-09-14  7:14           ` Maxime Coquelin
  0 siblings, 1 reply; 9+ messages in thread
From: Xia, Chenbo @ 2022-09-14  1:05 UTC (permalink / raw)
  To: Maxime Coquelin, David Marchand, Thomas Monjalon; +Cc: stable, dev

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Tuesday, September 13, 2022 11:03 PM
> To: David Marchand <david.marchand@redhat.com>; Xia, Chenbo
> <chenbo.xia@intel.com>; Thomas Monjalon <thomas@monjalon.net>
> Cc: stable@dpdk.org; dev@dpdk.org
> Subject: Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA
> reallocation
> 
> Hi,
> 
> On 7/26/22 09:55, Maxime Coquelin wrote:
> >
> >
> > On 7/25/22 22:32, David Marchand wrote:
> >> translate_ring_addresses (via numa_realloc) may change a virtio device
> >> and
> >> virtio queue.
> >> The virtqueue object must be refreshed before accessing the lock.
> >>
> >> Fixes: 04c27cb673b9 ("vhost: fix unsafe vring addresses modifications")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: David Marchand <david.marchand@redhat.com>
> >> ---
> >>   lib/vhost/vhost_user.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
> >> index 4ad28bac45..91d40e32fc 100644
> >> --- a/lib/vhost/vhost_user.c
> >> +++ b/lib/vhost/vhost_user.c
> >> @@ -2596,6 +2596,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev,
> >>               if (is_vring_iotlb(dev, vq, imsg)) {
> >>                   rte_spinlock_lock(&vq->access_lock);
> >>                   *pdev = dev = translate_ring_addresses(dev, i);
> >> +                vq = dev->virtqueue[i];
> >>                   rte_spinlock_unlock(&vq->access_lock);
> >>               }
> >>           }
> >
> > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> >
> > Thanks,
> > Maxime
> 
> The bug this patch is fixing is being reproduced downstream.
> It would be great it gets merged in main branch rapidly so that we can
> perform the backport.
> 
> Chenbo, are you planning a pull request for vhost/virtio in the next few
> days? If not, should the main branch maintainer pick this single patch
> directly and let the rest of the series more time for reviews?

Based on the status of all patches in the list, I guess PR will not happen
this week. So it will be good if David/Thomas can directly pick up this.

Thanks,
Chenbo

> 
> Thanks,
> Maxime


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

* Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
  2022-09-14  1:05         ` Xia, Chenbo
@ 2022-09-14  7:14           ` Maxime Coquelin
  2022-09-14  9:15             ` Thomas Monjalon
  0 siblings, 1 reply; 9+ messages in thread
From: Maxime Coquelin @ 2022-09-14  7:14 UTC (permalink / raw)
  To: Xia, Chenbo, David Marchand, Thomas Monjalon; +Cc: stable, dev

Hi Chenbo,

On 9/14/22 03:05, Xia, Chenbo wrote:
> Hi Maxime,
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Tuesday, September 13, 2022 11:03 PM
>> To: David Marchand <david.marchand@redhat.com>; Xia, Chenbo
>> <chenbo.xia@intel.com>; Thomas Monjalon <thomas@monjalon.net>
>> Cc: stable@dpdk.org; dev@dpdk.org
>> Subject: Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA
>> reallocation
>>
>> Hi,
>>
>> On 7/26/22 09:55, Maxime Coquelin wrote:
>>>
>>>
>>> On 7/25/22 22:32, David Marchand wrote:
>>>> translate_ring_addresses (via numa_realloc) may change a virtio device
>>>> and
>>>> virtio queue.
>>>> The virtqueue object must be refreshed before accessing the lock.
>>>>
>>>> Fixes: 04c27cb673b9 ("vhost: fix unsafe vring addresses modifications")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>>>> ---
>>>>    lib/vhost/vhost_user.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
>>>> index 4ad28bac45..91d40e32fc 100644
>>>> --- a/lib/vhost/vhost_user.c
>>>> +++ b/lib/vhost/vhost_user.c
>>>> @@ -2596,6 +2596,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev,
>>>>                if (is_vring_iotlb(dev, vq, imsg)) {
>>>>                    rte_spinlock_lock(&vq->access_lock);
>>>>                    *pdev = dev = translate_ring_addresses(dev, i);
>>>> +                vq = dev->virtqueue[i];
>>>>                    rte_spinlock_unlock(&vq->access_lock);
>>>>                }
>>>>            }
>>>
>>> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>
>>> Thanks,
>>> Maxime
>>
>> The bug this patch is fixing is being reproduced downstream.
>> It would be great it gets merged in main branch rapidly so that we can
>> perform the backport.
>>
>> Chenbo, are you planning a pull request for vhost/virtio in the next few
>> days? If not, should the main branch maintainer pick this single patch
>> directly and let the rest of the series more time for reviews?
> 
> Based on the status of all patches in the list, I guess PR will not happen
> this week. So it will be good if David/Thomas can directly pick up this.

OK, sounds good to me.

Thomas/David, is that good on your side?

Thanks,
Maxime

> Thanks,
> Chenbo
> 
>>
>> Thanks,
>> Maxime
> 


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

* Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
  2022-09-14  7:14           ` Maxime Coquelin
@ 2022-09-14  9:15             ` Thomas Monjalon
  2022-09-14  9:34               ` Maxime Coquelin
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2022-09-14  9:15 UTC (permalink / raw)
  To: Xia, Chenbo, David Marchand, Maxime Coquelin; +Cc: stable, dev

14/09/2022 09:14, Maxime Coquelin:
> On 9/14/22 03:05, Xia, Chenbo wrote:
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> On 7/26/22 09:55, Maxime Coquelin wrote:
> >> The bug this patch is fixing is being reproduced downstream.
> >> It would be great it gets merged in main branch rapidly so that we can
> >> perform the backport.
> >>
> >> Chenbo, are you planning a pull request for vhost/virtio in the next few
> >> days? If not, should the main branch maintainer pick this single patch
> >> directly and let the rest of the series more time for reviews?
> > 
> > Based on the status of all patches in the list, I guess PR will not happen
> > this week. So it will be good if David/Thomas can directly pick up this.
> 
> OK, sounds good to me.
> 
> Thomas/David, is that good on your side?

Is there a blocker to merge the whole series?





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

* Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
  2022-09-14  9:15             ` Thomas Monjalon
@ 2022-09-14  9:34               ` Maxime Coquelin
  2022-09-14  9:45                 ` Thomas Monjalon
  0 siblings, 1 reply; 9+ messages in thread
From: Maxime Coquelin @ 2022-09-14  9:34 UTC (permalink / raw)
  To: Thomas Monjalon, Xia, Chenbo, David Marchand; +Cc: stable, dev



On 9/14/22 11:15, Thomas Monjalon wrote:
> 14/09/2022 09:14, Maxime Coquelin:
>> On 9/14/22 03:05, Xia, Chenbo wrote:
>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>> On 7/26/22 09:55, Maxime Coquelin wrote:
>>>> The bug this patch is fixing is being reproduced downstream.
>>>> It would be great it gets merged in main branch rapidly so that we can
>>>> perform the backport.
>>>>
>>>> Chenbo, are you planning a pull request for vhost/virtio in the next few
>>>> days? If not, should the main branch maintainer pick this single patch
>>>> directly and let the rest of the series more time for reviews?
>>>
>>> Based on the status of all patches in the list, I guess PR will not happen
>>> this week. So it will be good if David/Thomas can directly pick up this.
>>
>> OK, sounds good to me.
>>
>> Thomas/David, is that good on your side?
> 
> Is there a blocker to merge the whole series?

No, I reviewed the whole series.

But other patches are an enhancement, so it would not hurt to have more
reviews.

I am fine either way.

Thanks,
Maxime


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

* Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
  2022-09-14  9:34               ` Maxime Coquelin
@ 2022-09-14  9:45                 ` Thomas Monjalon
  2022-09-14 11:48                   ` Maxime Coquelin
  0 siblings, 1 reply; 9+ messages in thread
From: Thomas Monjalon @ 2022-09-14  9:45 UTC (permalink / raw)
  To: Xia, Chenbo, David Marchand, Maxime Coquelin; +Cc: stable, dev

14/09/2022 11:34, Maxime Coquelin:
> 
> On 9/14/22 11:15, Thomas Monjalon wrote:
> > 14/09/2022 09:14, Maxime Coquelin:
> >> On 9/14/22 03:05, Xia, Chenbo wrote:
> >>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >>>> On 7/26/22 09:55, Maxime Coquelin wrote:
> >>>> The bug this patch is fixing is being reproduced downstream.
> >>>> It would be great it gets merged in main branch rapidly so that we can
> >>>> perform the backport.
> >>>>
> >>>> Chenbo, are you planning a pull request for vhost/virtio in the next few
> >>>> days? If not, should the main branch maintainer pick this single patch
> >>>> directly and let the rest of the series more time for reviews?
> >>>
> >>> Based on the status of all patches in the list, I guess PR will not happen
> >>> this week. So it will be good if David/Thomas can directly pick up this.
> >>
> >> OK, sounds good to me.
> >>
> >> Thomas/David, is that good on your side?
> > 
> > Is there a blocker to merge the whole series?
> 
> No, I reviewed the whole series.
> 
> But other patches are an enhancement, so it would not hurt to have more
> reviews.
> 
> I am fine either way.

I prefer not breaking series.
How urgent is it? Should I merge it today?



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

* Re: [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation
  2022-09-14  9:45                 ` Thomas Monjalon
@ 2022-09-14 11:48                   ` Maxime Coquelin
  0 siblings, 0 replies; 9+ messages in thread
From: Maxime Coquelin @ 2022-09-14 11:48 UTC (permalink / raw)
  To: Thomas Monjalon, Xia, Chenbo, David Marchand; +Cc: stable, dev



On 9/14/22 11:45, Thomas Monjalon wrote:
> 14/09/2022 11:34, Maxime Coquelin:
>>
>> On 9/14/22 11:15, Thomas Monjalon wrote:
>>> 14/09/2022 09:14, Maxime Coquelin:
>>>> On 9/14/22 03:05, Xia, Chenbo wrote:
>>>>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>>>>>> On 7/26/22 09:55, Maxime Coquelin wrote:
>>>>>> The bug this patch is fixing is being reproduced downstream.
>>>>>> It would be great it gets merged in main branch rapidly so that we can
>>>>>> perform the backport.
>>>>>>
>>>>>> Chenbo, are you planning a pull request for vhost/virtio in the next few
>>>>>> days? If not, should the main branch maintainer pick this single patch
>>>>>> directly and let the rest of the series more time for reviews?
>>>>>
>>>>> Based on the status of all patches in the list, I guess PR will not happen
>>>>> this week. So it will be good if David/Thomas can directly pick up this.
>>>>
>>>> OK, sounds good to me.
>>>>
>>>> Thomas/David, is that good on your side?
>>>
>>> Is there a blocker to merge the whole series?
>>
>> No, I reviewed the whole series.
>>
>> But other patches are an enhancement, so it would not hurt to have more
>> reviews.
>>
>> I am fine either way.
> 
> I prefer not breaking series.
> How urgent is it? Should I merge it today?
> 
> 

It causes failures in some of our QE tests, so it is urgent for us.
If you prefer not breaking the series, I am fine if you take it all.

Thanks,
Maxime


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

end of thread, other threads:[~2022-09-14 11:48 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220722135320.109269-1-david.marchand@redhat.com>
     [not found] ` <20220725203206.427083-1-david.marchand@redhat.com>
2022-07-25 20:32   ` [PATCH v3 1/4] vhost: fix vq use after free on NUMA reallocation David Marchand
2022-07-26  7:55     ` Maxime Coquelin
2022-09-13 15:02       ` Maxime Coquelin
2022-09-14  1:05         ` Xia, Chenbo
2022-09-14  7:14           ` Maxime Coquelin
2022-09-14  9:15             ` Thomas Monjalon
2022-09-14  9:34               ` Maxime Coquelin
2022-09-14  9:45                 ` Thomas Monjalon
2022-09-14 11:48                   ` Maxime Coquelin

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).