[PATCH v2] drm/virtio: fix deadlock in display_info_cb by removing hotplug from dequeue worker

Ryosuke Yasuoka <[email protected]>
Newsgroups dev.linux.lists.virtualization,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
A probe-time deadlock can occur between the dequeue worker and
drm_client_register(). During probe, drm_client_register() holds
clientlist_mutex and calls the fbdev hotplug callback, which triggers an
atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs()
waiting for virtqueue space. The dequeue worker that would free that
space calls virtio_gpu_cmd_get_display_info_cb(), which invokes
drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting
to acquire the same clientlist_mutex. Since wake_up() is only called
after the resp_cb loop, the probe thread is never woken and both threads
deadlock.

Fix this by removing the hotplug notification from
virtio_gpu_cmd_get_display_info_cb(). The display data (outputs[i].info)
is still updated synchronously in the callback.

For the init path, drm_client_register() already fires an initial
hotplug when the client is registered, which picks up the connector
state updated by display_info_cb.

For the runtime config_changed path, add a wait_event_timeout() in
config_changed_work_func() so that display_info_cb updates the connector
data before the hotplug notification is sent. Also replace
drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event() since
virtio-gpu never calls drm_kms_helper_poll_init() and thus
drm_helper_hpd_irq_event() always returns false without doing anything.

Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client")
Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
Suggested-by: Dmitry Osipenko <[email protected]>
Signed-off-by: Ryosuke Yasuoka <[email protected]>
---
I checked whether drm_helper_hpd_irq_event() is needed in
virtio_gpu_init(), as Dmitry suggested. AFAIS, it is not needed because:

1. drm_helper_hpd_irq_event() is always a no-op in virtio-gpu.
   It returns false immediately probe_helper.c:1088 because
   dev->mode_config.poll_enabled is false — virtio-gpu never calls
   drm_kms_helper_poll_init(). Even if it passed that gate, no
   virtio-gpu connectors set DRM_CONNECTOR_POLL_HPD.

1082 bool drm_helper_hpd_irq_event(struct drm_device *dev)
1083 {
...
1088         if (!dev->mode_config.poll_enabled)
1089                 return false;

2. virtio_gpu_init() runs before drm_dev_register() and
   drm_client_setup(), so no DRM clients are registered yet.
   drm_kms_helper_hotplug_event() would iterate an empty client list.
   The initial hotplug is handled by drm_client_register(), which fires
   a hotplug callback to the newly registered client. By that time,
   display_info_cb has already updated the connector data.

For the same reason, drm_helper_hpd_irq_event() in
config_changed_work_func() was also a no-op. The actual runtime hotplug
notification was always delivered by display_info_cb's call to
drm_kms_helper_hotplug_event(). This patch replaces it with a direct
drm_kms_helper_hotplug_event() call after waiting for the display info
response.
---
Changes in v2:
- Dropped the work_struct approach from v1.
- Instead, removed the hotplug calls from display_info_cb entirely, as
suggested by Dmitry.
- Added wait_event_timeout() in config_changed_work_func() so that the
display info response is received before sending the hotplug
notification.
- Replaced drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event()
in config_changed_work_func() since drm_helper_hpd_irq_event() is
always a no-op in virtio-gpu (poll_enabled is never set).
- No changes to virtio_gpu_init() — drm_client_register() already
handles the initial hotplug and hotplug event does nothing before DRM
device/client has been registered.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
 drivers/gpu/drm/virtio/virtgpu_kms.c | 5 ++++-
 drivers/gpu/drm/virtio/virtgpu_vq.c  | 3 ---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index cfde9f573df6..b4329f28e976 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -49,7 +49,10 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
 				virtio_gpu_cmd_get_edids(vgdev);
 			virtio_gpu_cmd_get_display_info(vgdev);
 			virtio_gpu_notify(vgdev);
-			drm_helper_hpd_irq_event(vgdev->ddev);
+			wait_event_timeout(vgdev->resp_wq,
+					   !vgdev->display_info_pending,
+					   5 * HZ);
+			drm_kms_helper_hotplug_event(vgdev->ddev);
 		}
 		events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
 	}
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index c8b9475a7472..e5e1af8b8e8a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -840,9 +840,6 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
 	vgdev->display_info_pending = false;
 	spin_unlock(&vgdev->display_info_lock);
 	wake_up(&vgdev->resp_wq);
-
-	if (!drm_helper_hpd_irq_event(vgdev->ddev))
-		drm_kms_helper_hotplug_event(vgdev->ddev);
 }
 
 static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,

---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260619-virtiogpu_syzbot-bdab508ffcd5

Best regards,
-- 
Ryosuke Yasuoka <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.