On Wed, May 16, 2018 at 01:21:54PM +0200, Greg Kurz wrote: > Removing a drive with drive_del while it is being used to run an I/O > intensive workload can cause QEMU to crash. > > An AIO flush can yield at some point: > > blk_aio_flush_entry() > blk_co_flush(blk) > bdrv_co_flush(blk->root->bs) > ... > qemu_coroutine_yield()
I'm surprised you didn't hit another crash later on with this patch applied. What happens to this completion after you've set blk->root = NULL? > and let the HMP command to run, free blk->root and give control > back to the AIO flush: > > hmp_drive_del() > blk_remove_bs() > bdrv_root_unref_child(blk->root) > child_bs = blk->root->bs > bdrv_detach_child(blk->root) > bdrv_replace_child(blk->root, NULL) > blk->root->bs = NULL > g_free(blk->root) <============== blk->root becomes stale > bdrv_unref(child_bs) > bdrv_delete(child_bs) > bdrv_close() > bdrv_drained_begin() > bdrv_do_drained_begin() > bdrv_drain_recurse() > aio_poll() > ... > qemu_coroutine_switch() > > and the AIO flush completion ends up dereferencing blk->root: > > blk_aio_complete() > scsi_aio_complete() > blk_get_aio_context(blk) > bs = blk_bs(blk) > ie, bs = blk->root ? blk->root->bs : NULL > ^^^^^ > stale > > The solution to this user-after-free situation is is to clear > blk->root before calling bdrv_unref() in bdrv_detach_child(), > and let blk_get_aio_context() fall back to the main loop context > since the BDS has been removed. QEMU should drain I/O requests before making block driver graph changes. I think the drained region in blk_remove_bs() needs to begin earlier so that requests are completed before we begin to change things. Stefan
signature.asc
Description: PGP signature