Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Flawed driver teardown order allows work items (including the newly added `hotplug_work`) to be re-queued and executed after the device structure and virtqueues are freed.
--

commit 81a0fe4ee345c5c5f16f0048bb1284a83aa82e58
Author: Ryosuke Yasuoka <ryasuoka@redhat.com>

drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
    
A probe-time deadlock can occur between the dequeue worker and
drm_client_register(). This patch defers the hotplug notification from
virtio_gpu_cmd_get_display_info_cb() to a separate work item to prevent
this deadlock.

Link: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
Link: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e

> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index cfde9f573df6e..cfb532ba43a47 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
[ ... ]
> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
>  	flush_work(&vgdev->obj_free_work);
>  	flush_work(&vgdev->ctrlq.dequeue_work);
>  	flush_work(&vgdev->cursorq.dequeue_work);
> +	flush_work(&vgdev->hotplug_work);
>  	flush_work(&vgdev->config_changed_work);
>  	virtio_reset_device(vgdev->vdev);
>  	vgdev->vdev->config->del_vqs(vgdev->vdev);

[Severity: High]
This is a pre-existing issue, but does this teardown sequence allow work items
to be re-queued after they are flushed?

Here in virtio_gpu_deinit(), the work items are flushed before the device is
reset via virtio_reset_device() and the virtqueues are deleted via del_vqs().

Because the device is still active during the flush, if responses arrive for
shutdown commands (like those queued by drm_atomic_helper_shutdown()), the
interrupt handler can re-queue the work:

virtio_gpu_ctrl_ack()
  schedule_work(&vgdev->ctrlq.dequeue_work);

This re-queued dequeue_work can then execute after del_vqs() has freed the
virtqueues, leading to a use-after-free. Furthermore, it could schedule the
newly added hotplug_work:

virtio_gpu_cmd_get_display_info_cb()
  schedule_work(&vgdev->hotplug_work);

This would cause further use-after-free access on the freed vgdev structure.
Should the flush_work() calls be moved after virtio_reset_device() to ensure
no new interrupts can re-queue these work items?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-virtiogpu_syzbot-v1-1-0aa06630750e@redhat.com?part=1