* [PATCH] Revert "vhost-blk: set features before setting inflight feature"
@ 2020-11-02 16:57 Stefan Hajnoczi
2020-11-03 5:11 ` Yu, Jin
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Hajnoczi @ 2020-11-02 16:57 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, Jin Yu, Max Reitz,
Stefan Hajnoczi, Raphael Norwitz
This reverts commit adb29c027341ba095a3ef4beef6aaef86d3a520e.
The commit broke -device vhost-user-blk-pci because the
vhost_dev_prepare_inflight() function it introduced segfaults in
vhost_dev_set_features() when attempting to access struct vhost_dev's
vdev pointer before it has been assigned.
To reproduce the segfault simply launch a vhost-user-blk device with the
contrib vhost-user-blk device backend:
$ build/contrib/vhost-user-blk/vhost-user-blk -s /tmp/vhost-user-blk.sock -r -b /var/tmp/foo.img
$ build/qemu-system-x86_64 \
-device vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 \
-object memory-backend-memfd,id=mem,size=1G,share=on \
-M memory-backend=mem,accel=kvm \
-chardev socket,id=char1,path=/tmp/vhost-user-blk.sock
Segmentation fault (core dumped)
Cc: Jin Yu <jin.yu@intel.com>
Cc: Raphael Norwitz <raphael.norwitz@nutanix.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/hw/virtio/vhost.h | 1 -
hw/block/vhost-user-blk.c | 6 ------
hw/virtio/vhost.c | 18 ------------------
3 files changed, 25 deletions(-)
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 839bfb153c..94585067f7 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -141,7 +141,6 @@ void vhost_dev_reset_inflight(struct vhost_inflight *inflight);
void vhost_dev_free_inflight(struct vhost_inflight *inflight);
void vhost_dev_save_inflight(struct vhost_inflight *inflight, QEMUFile *f);
int vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f);
-int vhost_dev_prepare_inflight(struct vhost_dev *hdev);
int vhost_dev_set_inflight(struct vhost_dev *dev,
struct vhost_inflight *inflight);
int vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size,
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index f67b29bbf3..a076b1e54d 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -131,12 +131,6 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
s->dev.acked_features = vdev->guest_features;
- ret = vhost_dev_prepare_inflight(&s->dev);
- if (ret < 0) {
- error_report("Error set inflight format: %d", -ret);
- goto err_guest_notifiers;
- }
-
if (!s->inflight->addr) {
ret = vhost_dev_get_inflight(&s->dev, s->queue_size, s->inflight);
if (ret < 0) {
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f2482378c6..79b2be20df 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1645,24 +1645,6 @@ int vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f)
return 0;
}
-int vhost_dev_prepare_inflight(struct vhost_dev *hdev)
-{
- int r;
-
- if (hdev->vhost_ops->vhost_get_inflight_fd == NULL ||
- hdev->vhost_ops->vhost_set_inflight_fd == NULL) {
- return 0;
- }
-
- r = vhost_dev_set_features(hdev, hdev->log_enabled);
- if (r < 0) {
- VHOST_OPS_DEBUG("vhost_dev_prepare_inflight failed");
- return r;
- }
-
- return 0;
-}
-
int vhost_dev_set_inflight(struct vhost_dev *dev,
struct vhost_inflight *inflight)
{
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH] Revert "vhost-blk: set features before setting inflight feature"
2020-11-02 16:57 [PATCH] Revert "vhost-blk: set features before setting inflight feature" Stefan Hajnoczi
@ 2020-11-03 5:11 ` Yu, Jin
2020-11-03 14:41 ` Stefan Hajnoczi
0 siblings, 1 reply; 5+ messages in thread
From: Yu, Jin @ 2020-11-03 5:11 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel
Cc: Kevin Wolf, Max Reitz, Michael S. Tsirkin, qemu-block, Raphael Norwitz
Hi Stefan,
I have sent a version 2 and it should fix this issue.
I also test it several times and I did not meet this issue again.
Thanks for your report.
Best Regards
Jin
> -----Original Message-----
> From: Stefan Hajnoczi <stefanha@redhat.com>
> Sent: Tuesday, November 3, 2020 12:57 AM
> To: qemu-devel@nongnu.org
> Cc: qemu-block@nongnu.org; Raphael Norwitz
> <raphael.norwitz@nutanix.com>; Max Reitz <mreitz@redhat.com>; Kevin
> Wolf <kwolf@redhat.com>; Michael S. Tsirkin <mst@redhat.com>; Stefan
> Hajnoczi <stefanha@redhat.com>; Yu, Jin <jin.yu@intel.com>
> Subject: [PATCH] Revert "vhost-blk: set features before setting inflight
> feature"
>
> This reverts commit adb29c027341ba095a3ef4beef6aaef86d3a520e.
>
> The commit broke -device vhost-user-blk-pci because the
> vhost_dev_prepare_inflight() function it introduced segfaults in
> vhost_dev_set_features() when attempting to access struct vhost_dev's vdev
> pointer before it has been assigned.
>
> To reproduce the segfault simply launch a vhost-user-blk device with the
> contrib vhost-user-blk device backend:
>
> $ build/contrib/vhost-user-blk/vhost-user-blk -s /tmp/vhost-user-blk.sock -r
> -b /var/tmp/foo.img
> $ build/qemu-system-x86_64 \
> -device vhost-user-blk-pci,id=drv0,chardev=char1,addr=4.0 \
> -object memory-backend-memfd,id=mem,size=1G,share=on \
> -M memory-backend=mem,accel=kvm \
> -chardev socket,id=char1,path=/tmp/vhost-user-blk.sock
> Segmentation fault (core dumped)
>
> Cc: Jin Yu <jin.yu@intel.com>
> Cc: Raphael Norwitz <raphael.norwitz@nutanix.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> include/hw/virtio/vhost.h | 1 -
> hw/block/vhost-user-blk.c | 6 ------
> hw/virtio/vhost.c | 18 ------------------
> 3 files changed, 25 deletions(-)
>
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index
> 839bfb153c..94585067f7 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -141,7 +141,6 @@ void vhost_dev_reset_inflight(struct vhost_inflight
> *inflight); void vhost_dev_free_inflight(struct vhost_inflight *inflight); void
> vhost_dev_save_inflight(struct vhost_inflight *inflight, QEMUFile *f); int
> vhost_dev_load_inflight(struct vhost_inflight *inflight, QEMUFile *f); -int
> vhost_dev_prepare_inflight(struct vhost_dev *hdev); int
> vhost_dev_set_inflight(struct vhost_dev *dev,
> struct vhost_inflight *inflight); int
> vhost_dev_get_inflight(struct vhost_dev *dev, uint16_t queue_size, diff --git
> a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index
> f67b29bbf3..a076b1e54d 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -131,12 +131,6 @@ static int vhost_user_blk_start(VirtIODevice *vdev)
>
> s->dev.acked_features = vdev->guest_features;
>
> - ret = vhost_dev_prepare_inflight(&s->dev);
> - if (ret < 0) {
> - error_report("Error set inflight format: %d", -ret);
> - goto err_guest_notifiers;
> - }
> -
> if (!s->inflight->addr) {
> ret = vhost_dev_get_inflight(&s->dev, s->queue_size, s->inflight);
> if (ret < 0) {
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index
> f2482378c6..79b2be20df 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1645,24 +1645,6 @@ int vhost_dev_load_inflight(struct vhost_inflight
> *inflight, QEMUFile *f)
> return 0;
> }
>
> -int vhost_dev_prepare_inflight(struct vhost_dev *hdev) -{
> - int r;
> -
> - if (hdev->vhost_ops->vhost_get_inflight_fd == NULL ||
> - hdev->vhost_ops->vhost_set_inflight_fd == NULL) {
> - return 0;
> - }
> -
> - r = vhost_dev_set_features(hdev, hdev->log_enabled);
> - if (r < 0) {
> - VHOST_OPS_DEBUG("vhost_dev_prepare_inflight failed");
> - return r;
> - }
> -
> - return 0;
> -}
> -
> int vhost_dev_set_inflight(struct vhost_dev *dev,
> struct vhost_inflight *inflight) {
> --
> 2.28.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "vhost-blk: set features before setting inflight feature"
2020-11-03 5:11 ` Yu, Jin
@ 2020-11-03 14:41 ` Stefan Hajnoczi
2020-11-03 14:45 ` Michael S. Tsirkin
0 siblings, 1 reply; 5+ messages in thread
From: Stefan Hajnoczi @ 2020-11-03 14:41 UTC (permalink / raw)
To: Yu, Jin
Cc: Kevin Wolf, qemu-block, Michael S. Tsirkin, qemu-devel,
Max Reitz, Raphael Norwitz
[-- Attachment #1: Type: text/plain, Size: 206 bytes --]
On Tue, Nov 03, 2020 at 05:11:18AM +0000, Yu, Jin wrote:
> I have sent a version 2 and it should fix this issue.
Great, thanks! Michael can review and consider it for merge instead of
this patch.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "vhost-blk: set features before setting inflight feature"
2020-11-03 14:41 ` Stefan Hajnoczi
@ 2020-11-03 14:45 ` Michael S. Tsirkin
2020-11-03 17:38 ` Stefan Hajnoczi
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2020-11-03 14:45 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: Kevin Wolf, qemu-block, Yu, Jin, qemu-devel, Max Reitz, Raphael Norwitz
On Tue, Nov 03, 2020 at 02:41:30PM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 03, 2020 at 05:11:18AM +0000, Yu, Jin wrote:
> > I have sent a version 2 and it should fix this issue.
>
> Great, thanks! Michael can review and consider it for merge instead of
> this patch.
>
> Stefan
It's not instead, it's on top of your revert.
--
MST
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Revert "vhost-blk: set features before setting inflight feature"
2020-11-03 14:45 ` Michael S. Tsirkin
@ 2020-11-03 17:38 ` Stefan Hajnoczi
0 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2020-11-03 17:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Kevin Wolf, qemu-block, Yu, Jin, qemu-devel, Max Reitz, Raphael Norwitz
[-- Attachment #1: Type: text/plain, Size: 450 bytes --]
On Tue, Nov 03, 2020 at 09:45:27AM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 03, 2020 at 02:41:30PM +0000, Stefan Hajnoczi wrote:
> > On Tue, Nov 03, 2020 at 05:11:18AM +0000, Yu, Jin wrote:
> > > I have sent a version 2 and it should fix this issue.
> >
> > Great, thanks! Michael can review and consider it for merge instead of
> > this patch.
> >
> > Stefan
>
> It's not instead, it's on top of your revert.
Okay.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-03 17:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-02 16:57 [PATCH] Revert "vhost-blk: set features before setting inflight feature" Stefan Hajnoczi
2020-11-03 5:11 ` Yu, Jin
2020-11-03 14:41 ` Stefan Hajnoczi
2020-11-03 14:45 ` Michael S. Tsirkin
2020-11-03 17:38 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.