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