[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] block: Fix qemu crash when using scsi-block
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-block] [PATCH] block: Fix qemu crash when using scsi-block |
Date: |
Thu, 23 Nov 2017 10:23:25 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
On Wed, Nov 22, 2017 at 07:04:26PM +0100, Kevin Wolf wrote:
> Am 22.11.2017 um 18:06 hat Stefan Hajnoczi geschrieben:
> > On Wed, Nov 22, 2017 at 07:33:28AM -0800, Deepa Srinivasan wrote:
> > > Starting qemu with the following arguments causes qemu to segfault:
> > > ... -device lsi,id=lsi0 -drive
> > > file=iscsi:<...>,format=raw,if=none,node-name=
> > > iscsi1 -device scsi-block,bus=lsi0.0,id=<...>,drive=iscsi1
> > >
> > > This patch fixes blk_aio_ioctl() so it does not pass stack addresses to
> > > blk_aio_ioctl_entry() which may be invoked after blk_aio_ioctl() returns.
> > > More
> > > details about the bug follow.
> > >
> > > blk_aio_ioctl() invokes blk_aio_prwv() with blk_aio_ioctl_entry as the
> > > coroutine parameter. blk_aio_prwv() ultimately calls aio_co_enter().
> > >
> > > When blk_aio_ioctl() is executed from within a coroutine context (e.g.
> > > iscsi_bh_cb()), aio_co_enter() adds the coroutine (blk_aio_ioctl_entry) to
> > > the current coroutine's wakeup queue. blk_aio_ioctl() then returns.
> > >
> > > When blk_aio_ioctl_entry() executes later, it accesses an invalid pointer:
> > > ....
> > > BlkRwCo *rwco = &acb->rwco;
> > >
> > > rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
> > > rwco->qiov->iov[0].iov_base); <--- qiov is
> > > invalid
> > > here
> > > ...
> > >
> > > In the case when blk_aio_ioctl() is called from a non-coroutine context,
> > > blk_aio_ioctl_entry() executes immediately. But if bdrv_co_ioctl() calls
> > > qemu_coroutine_yield(), blk_aio_ioctl() will return. When the coroutine
> > > execution is complete, control returns to blk_aio_ioctl_entry() after the
> > > call
> > > to blk_co_ioctl(). There is no invalid reference after this point, but the
> > > function is still holding on to invalid pointers.
> > >
> > > The fix is to allocate memory for the QEMUIOVector and struct iovec as
> > > part of
> > > the request struct which the IO buffer is part of. The memory for this
> > > struct is
> > > guaranteed to be valid till the AIO is completed.
> >
> > Thanks for the patch!
> >
> > AIO APIs currently don't require the caller to match qiov's lifetime to
> > the I/O request lifetime. This patch changes that for blk_aio_ioctl()
> > only. If we want to do this consistently then all aio callers need to
> > be audited and fixed.
> >
> > The alternative is to make the API copy qiov when necessary. That is
> > less efficient but avoids modifying all callers.
> >
> > Either way, the lifetime of qiov must be consistent across all aio APIs,
> > not just blk_aio_ioctl().
>
> Don't all blk_aio_*() APIs that take a qiov pointer require that it
> remains valid until the request completes? I don't think they are copied
> anywhere for blk_aio_preadv/pwritev() before being passed to the block
> driver.
>
> So this does look consistent with the existing functions to me.
You are right. I audited the blk_aio_preadv() callers and they all keep
qiov around until the request is complete.
Actually this makes sense because even in the simple non-coroutine case
with aio=threads the qiov hasn't necessarily been read yet when the
function returns. The aio_worker() function executes later and only
then is qiov handed to the host kernel.
So this is a one-off bug in blk_aio_ioctl() callers.
Stefan
signature.asc
Description: PGP signature
- Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block, (continued)
- Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block, Deepa Srinivasan, 2017/11/22
- Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block, Paolo Bonzini, 2017/11/23
- [Qemu-block] [PATCH] block: Fix qemu crash when using scsi-block, Deepa Srinivasan, 2017/11/23
- Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block, Deepa Srinivasan, 2017/11/23
- Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block, Paolo Bonzini, 2017/11/23
- Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block, Kevin Wolf, 2017/11/23
- Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block, Paolo Bonzini, 2017/11/23
- Re: [Qemu-block] [Qemu-devel] [PATCH] block: Fix qemu crash when using scsi-block, Kevin Wolf, 2017/11/23
Re: [Qemu-block] [PATCH] block: Fix qemu crash when using scsi-block, Stefan Hajnoczi, 2017/11/22