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
