virtio_gpu_gl_device_unrealize() deletes gl->cmdq_resume_bh (and, when present, gl->async_fence_bh). As the comment in that function notes, hostmem memory regions are not guaranteed to be finalized during unrealize(): their finalize may be deferred (e.g. through an RCU / object reference lifetime) until after unrealize() has returned.
virtio_gpu_virgl_hostmem_region_finalize() runs on that deferred path and calls qemu_bh_schedule(gl->cmdq_resume_bh). If the finalize lands after unrealize() has deleted the BH, this schedules a deleted BH (a use-after-free / use-after-delete window on the BH object). NULL out both BHs after deleting them in unrealize(), and guard the two schedule sites in virtio-gpu-virgl.c with a NULL check so a late finalize (or a late async-fence callback) simply skips the wake-up instead of touching a deleted BH. This is an RFC: this area overlaps Akihiko Odaki's "Fix memory region use-after-finalization" discussion series; if that series lands first this patch should be reworked to fit its lifetime model. Signed-off-by: Hongyan Xu <[email protected]> --- hw/display/virtio-gpu-gl.c | 8 ++++++++ hw/display/virtio-gpu-virgl.c | 8 ++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index 2b7a41c466..2fd3f84a91 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -180,10 +180,18 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev) if (gl->renderer_state >= RS_INITED) { #if VIRGL_VERSION_MAJOR >= 1 qemu_bh_delete(gl->cmdq_resume_bh); + /* + * hostmem memory regions can be finalized after unrealize() + * returns (see below); their finalize path schedules + * cmdq_resume_bh, so NULL it out to let them detect that the + * BH is gone instead of touching a deleted BH. + */ + gl->cmdq_resume_bh = NULL; if (gl->async_fence_bh) { virtio_gpu_virgl_reset_async_fences(g); qemu_bh_delete(gl->async_fence_bh); + gl->async_fence_bh = NULL; } #endif if (virtio_gpu_stats_enabled(g->parent_obj.conf)) { diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 9bda572426..d38d8fe995 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -134,7 +134,9 @@ static void virtio_gpu_virgl_hostmem_region_finalize(Object *obj) * context. */ gl = VIRTIO_GPU_GL(vmr->g); - qemu_bh_schedule(gl->cmdq_resume_bh); + if (gl->cmdq_resume_bh) { + qemu_bh_schedule(gl->cmdq_resume_bh); + } } static const TypeInfo virtio_gpu_virgl_hostmem_region_info = { @@ -1294,7 +1296,9 @@ virtio_gpu_virgl_push_async_fence(VirtIOGPU *g, uint32_t ctx_id, QSLIST_INSERT_HEAD_ATOMIC(&gl->async_fenceq, f, next); - qemu_bh_schedule(gl->async_fence_bh); + if (gl->async_fence_bh) { + qemu_bh_schedule(gl->async_fence_bh); + } } static void virgl_write_async_fence(void *opaque, uint32_t fence) -- 2.50.1.windows.1
