qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/2] Block patches
@ 2019-08-22 15:53 Stefan Hajnoczi
  2019-08-22 15:53 ` [Qemu-devel] [PULL 1/2] util/async: hold AioContext ref to prevent use-after-free Stefan Hajnoczi
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2019-08-22 15:53 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Peter Maydell, qemu-block, Michael S. Tsirkin,
	Stefan Hajnoczi, Paolo Bonzini

The following changes since commit 33f18cf7dca7741d3647d514040904ce83edd73d:

  Merge remote-tracking branch 'remotes/kraxel/tags/audio-20190821-pull-request' into staging (2019-08-21 15:18:50 +0100)

are available in the Git repository at:

  https://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 5d4c1ed3d46d7e2010b389fe5f3376f605182ab0:

  vhost-user-scsi: prevent using uninitialized vqs (2019-08-22 16:52:23 +0100)

----------------------------------------------------------------
Pull request

----------------------------------------------------------------

Raphael Norwitz (1):
  vhost-user-scsi: prevent using uninitialized vqs

Stefan Hajnoczi (1):
  util/async: hold AioContext ref to prevent use-after-free

 hw/scsi/vhost-user-scsi.c | 2 +-
 util/async.c              | 8 ++++++++
 2 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.21.0



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

* [Qemu-devel] [PULL 1/2] util/async: hold AioContext ref to prevent use-after-free
  2019-08-22 15:53 [Qemu-devel] [PULL 0/2] Block patches Stefan Hajnoczi
@ 2019-08-22 15:53 ` Stefan Hajnoczi
  2019-08-22 15:53 ` [Qemu-devel] [PULL 2/2] vhost-user-scsi: prevent using uninitialized vqs Stefan Hajnoczi
  2019-08-27  9:00 ` [Qemu-devel] [PULL 0/2] Block patches Peter Maydell
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2019-08-22 15:53 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Peter Maydell, qemu-block, Michael S. Tsirkin,
	Stefan Hajnoczi, Paolo Bonzini

The tests/test-bdrv-drain /bdrv-drain/iothread/drain test case does the
following:

1. The preadv coroutine calls aio_bh_schedule_oneshot() and then yields.
2. The one-shot BH executes in another AioContext.  All it does is call
   aio_co_wakeup(preadv_co).
3. The preadv coroutine is re-entered and returns.

There is a race condition in aio_co_wake() where the preadv coroutine
returns and the test case destroys the preadv IOThread.  aio_co_wake()
can still be running in the other AioContext and it performs an access
to the freed IOThread AioContext.

Here is the race in aio_co_schedule():

  QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines,
                            co, co_scheduled_next);
  <-- race: co may execute before we invoke qemu_bh_schedule()!
  qemu_bh_schedule(ctx->co_schedule_bh);

So if co causes ctx to be freed then we're in trouble.  Fix this problem
by holding a reference to ctx.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20190723190623.21537-1-stefanha@redhat.com
Message-Id: <20190723190623.21537-1-stefanha@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 util/async.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/util/async.c b/util/async.c
index 8d2105729c..4e4c7af51e 100644
--- a/util/async.c
+++ b/util/async.c
@@ -459,9 +459,17 @@ void aio_co_schedule(AioContext *ctx, Coroutine *co)
         abort();
     }
 
+    /* The coroutine might run and release the last ctx reference before we
+     * invoke qemu_bh_schedule().  Take a reference to keep ctx alive until
+     * we're done.
+     */
+    aio_context_ref(ctx);
+
     QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines,
                               co, co_scheduled_next);
     qemu_bh_schedule(ctx->co_schedule_bh);
+
+    aio_context_unref(ctx);
 }
 
 void aio_co_wake(struct Coroutine *co)
-- 
2.21.0



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

* [Qemu-devel] [PULL 2/2] vhost-user-scsi: prevent using uninitialized vqs
  2019-08-22 15:53 [Qemu-devel] [PULL 0/2] Block patches Stefan Hajnoczi
  2019-08-22 15:53 ` [Qemu-devel] [PULL 1/2] util/async: hold AioContext ref to prevent use-after-free Stefan Hajnoczi
@ 2019-08-22 15:53 ` Stefan Hajnoczi
  2019-08-27  9:00 ` [Qemu-devel] [PULL 0/2] Block patches Peter Maydell
  2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2019-08-22 15:53 UTC (permalink / raw)
  To: qemu-devel
  Cc: Fam Zheng, Peter Maydell, qemu-block, Michael S. Tsirkin,
	Raphael Norwitz, Stefan Hajnoczi, Paolo Bonzini

From: Raphael Norwitz <raphael.norwitz@nutanix.com>

Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
and event without a physical address. This can cause
vhost_verify_ring_part_mapping to return ENOMEM, causing
the following logs:

qemu-system-x86_64: Unable to map available ring for ring 0
qemu-system-x86_64: Verify ring failure on region 0

The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
has already resolved the issue for vhost scsi devices but
the fix was never applied to vhost-user scsi devices.

Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1560299717-177734-1-git-send-email-raphael.norwitz@nutanix.com
Message-Id: <1560299717-177734-1-git-send-email-raphael.norwitz@nutanix.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/scsi/vhost-user-scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 31c9d34637..6a6c15dd32 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -93,7 +93,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
     }
 
     vsc->dev.nvqs = 2 + vs->conf.num_queues;
-    vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs);
+    vsc->dev.vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
     vsc->dev.vq_index = 0;
     vsc->dev.backend_features = 0;
     vqs = vsc->dev.vqs;
-- 
2.21.0



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

* Re: [Qemu-devel] [PULL 0/2] Block patches
  2019-08-22 15:53 [Qemu-devel] [PULL 0/2] Block patches Stefan Hajnoczi
  2019-08-22 15:53 ` [Qemu-devel] [PULL 1/2] util/async: hold AioContext ref to prevent use-after-free Stefan Hajnoczi
  2019-08-22 15:53 ` [Qemu-devel] [PULL 2/2] vhost-user-scsi: prevent using uninitialized vqs Stefan Hajnoczi
@ 2019-08-27  9:00 ` Peter Maydell
  2 siblings, 0 replies; 4+ messages in thread
From: Peter Maydell @ 2019-08-27  9:00 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Fam Zheng, Paolo Bonzini, QEMU Developers, Qemu-block,
	Michael S. Tsirkin

On Thu, 22 Aug 2019 at 16:53, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> The following changes since commit 33f18cf7dca7741d3647d514040904ce83edd73d:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/audio-20190821-pull-request' into staging (2019-08-21 15:18:50 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 5d4c1ed3d46d7e2010b389fe5f3376f605182ab0:
>
>   vhost-user-scsi: prevent using uninitialized vqs (2019-08-22 16:52:23 +0100)
>
> ----------------------------------------------------------------
> Pull request
>
> ----------------------------------------------------------------


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.

-- PMM


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

end of thread, other threads:[~2019-08-27  9:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-22 15:53 [Qemu-devel] [PULL 0/2] Block patches Stefan Hajnoczi
2019-08-22 15:53 ` [Qemu-devel] [PULL 1/2] util/async: hold AioContext ref to prevent use-after-free Stefan Hajnoczi
2019-08-22 15:53 ` [Qemu-devel] [PULL 2/2] vhost-user-scsi: prevent using uninitialized vqs Stefan Hajnoczi
2019-08-27  9:00 ` [Qemu-devel] [PULL 0/2] Block patches Peter Maydell

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