Am 07.07.26 um 16:18 schrieb [email protected]:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:

New issues:
- [High] New AB-BA deadlock introduced between vgasr_mutex and clientlist_mutex.

Pre-existing issues:
- [High] Preexisting AB-BA deadlock between vgasr_mutex and console_lock.
--

--- Patch [7]: [PATCH 07/10] drm/nouveau: Implement 
vga_switcheroo_client_ops.pre_switch ---
commit a0ca13a7165f71c977582a84acf58b2f2681a805
Author: Thomas Zimmermann <[email protected]>

drm/nouveau: Implement vga_switcheroo_client_ops.pre_switch

This patch implements the vga_switcheroo_client_ops.pre_switch callback in
the nouveau driver. It calls drm_client_dev_acquire_outputs() from the
pre_switch callback to push fbcon updates from vga_switcheroo into DRM's
fbdev emulation.

diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c 
b/drivers/gpu/drm/nouveau/nouveau_vga.c
index a6c375a241542..2d2d08be8fbe2 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -76,11 +76,20 @@ nouveau_switcheroo_can_switch(struct pci_dev *pdev)
        return atomic_read(&drm->dev->open_count) == 0;
  }
+static void
+nouveau_switcheroo_pre_switch(struct pci_dev *pdev)
+{
+       struct nouveau_drm *drm = pci_get_drvdata(pdev);
+
+       drm_client_dev_acquire_outputs(drm->dev);
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does this introduce an AB-BA deadlock between vgasr_mutex and
clientlist_mutex?

When drm_client_dev_hotplug() is called, it acquires dev->clientlist_mutex
and eventually calls register_framebuffer(). This takes the console_lock()
and then calls vga_switcheroo_client_fb_set(), taking vgasr_mutex. This
establishes the lock order:
clientlist_mutex -> console_lock -> vgasr_mutex

However, vga_switcheroo_debugfs_write() acquires vgasr_mutex and calls
vga_switchto_stage2(), which invokes this new pre_switch callback.
Calling drm_client_dev_acquire_outputs() here attempts to acquire
dev->clientlist_mutex, establishing the reverse lock order:
vgasr_mutex -> clientlist_mutex

Patch 10 removes calls to vga_switcheroo_client_fb_set() entirely. I could try to reorganize the series to avoid the vulnerable intermediate states.


[Severity: High]
This is a pre-existing issue, but does this path also trigger a deadlock
between vgasr_mutex and console_lock?

When register_framebuffer() is called (e.g., during driver probe or
hotplug), it calls fbcon_fb_registered(), which acquires console_lock()
and then calls vga_switcheroo_client_fb_set(), taking vgasr_mutex.

Conversely, vga_switcheroo_debugfs_write() acquires vgasr_mutex and calls
vga_switchto_stage2(). This invokes pre_switch(), which leads to
drm_client_dev_acquire_outputs() and then fb_switch_outputs(), attempting
to acquire console_lock(). This creates an exact inversion:
vgasr_mutex -> console_lock

This would also be resolved by the early removal of vga_switcheroo_client_fb_set().


+}
+
  static const struct vga_switcheroo_client_ops
  nouveau_switcheroo_ops = {
        .set_gpu_state = nouveau_switcheroo_set_state,
        .reprobe = nouveau_switcheroo_reprobe,
        .can_switch = nouveau_switcheroo_can_switch,
+       .pre_switch = nouveau_switcheroo_pre_switch,
  };
void

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)


Reply via email to