Re: [PATCH v2] block-backend: prevent dangling BDS pointers across aio_poll()

[Stefan Hajnoczi]

On Mon, Jan 10, 2022 at 07:57:05PM +0100, Hanna Reitz wrote:
> On 14.12.21 15:35, Stefan Hajnoczi wrote:
> > The BlockBackend root child can change when aio_poll() is invoked. This
> > happens when a temporary filter node is removed upon blockjob
> > completion, for example.
> > 
> > Functions in block/block-backend.c must be aware of this when using a
> > blk_bs() pointer across aio_poll() because the BlockDriverState refcnt
> > may reach 0, resulting in a stale pointer.
> > 
> > One example is scsi_device_purge_requests(), which calls blk_drain() to
> > wait for in-flight requests to cancel. If the backup blockjob is active,
> > then the BlockBackend root child is a temporary filter BDS owned by the
> > blockjob. The blockjob can complete during bdrv_drained_begin() and the
> > last reference to the BDS is released when the temporary filter node is
> > removed. This results in a use-after-free when blk_drain() calls
> > bdrv_drained_end(bs) on the dangling pointer.
> 
> By the way, I have a BZ for this, though it’s about block-stream instead of
> backup (https://bugzilla.redhat.com/show_bug.cgi?id=2036178).  But I’m happy
> to report your patch seems* to fix that problem, too!  (Thanks for fixing my
> BZs! :))
> 
> *I’ve written a reproducer in iotest form 
> (https://gitlab.com/hreitz/qemu/-/blob/stefans-fix-and-a-test/tests/qemu-iotests/tests/stream-error-on-reset),
> and so far I can only assume it indeed reproduces the report, but I found
> that iotest to indeed be fixed by this patch.  (Which made me very happy.)

Great, I have merged your test case and sent a v3.

Thanks,
Stefan

signature.asc
Description: PGP signature