• File: sys/dev/video/vga/vga_switcheroo.c
• Lines: 557, 558, 559, 560, 561
• Severity: Low
• CVSS: CVSS:3.1/AV:L/AC:H/PR:H/UI:N/S:U:C:N/I:N/A:L
• CWE: CWE-669 Incorrect Resource Transfer or Closure
• Confidence: certain

Summary

vga_switcheroo_lock_ddc() acquires mux_hw_lk, then on the early-return path where handler is NULL or handler->switch_ddc is NULL it returns -ENODEV without calling mutex_unlock.

This is a lock leak that will permanently block every subsequent mux_hw_lk acquisition (force_migd, switchto_stage2, other lock_ddc callers) on a different thread, hanging the system's GPU switching and EDID probing.

Root cause

sys/dev/video/vga/vga_switcheroo.c:557 mutex_lock(&vgasr_priv->mux_hw_lk); then immediately at 558–561:

if (!vgasr_priv->handler || !vgasr_priv->handler->switch_ddc) {
    vgasr_priv->old_ddc_owner = -ENODEV;
    return -ENODEV;
}

The return -ENODEV leaves mux_hw_lk held.

The in-tree indexed gmux handler (gmux_handler_indexed at sys/gnu/dev/misc/apple_gmux/apple-gmux.c:600-604) is registered with NO switch_ddc callback, so on indexed (retina) MacBook Pros this exact branch is taken every time drm_get_edid_switcheroo() (sys/dev/drm/drm_edid.c:1770) calls lock_ddc.

The only thing preventing an immediate system-wide hang is that drm_get_edid_switcheroo unconditionally calls vga_switcheroo_unlock_ddc() immediately afterward (drm_edid.c:1772), which (because the lock was held by the same thread and LK_CANRECURSE was set at vga_switcheroo.c:1018) releases it.

But the API contract in the doc at lines 569–583 ("must be called even if vga_switcheroo_lock_ddc() returned an error") is fragile: any future caller that returns early on the -ENODEV error without calling unlock_ddc will deadlock the entire subsystem.

Threat

Local privileged user (root, or any code path inside the kernel that can drive a DRM connector EDID probe on an indexed gmux machine) can trigger the leak; the practical impact today is bounded because the sole caller (drm_get_edid_switcheroo) compensates, but the lock discipline is broken and any new caller (or a regression in drm_edid.c that returns between lock and unlock on error) will hang the GPU subsystem.

Defense-in-depth / kernel robustness issue.

Exploit / PoC

Direct demonstration requires adding a new caller (or modifying drm_edid.c) to take the -ENODEV path and skip unlock.

Concretely: write a small kld module on a machine whose handler is gmux_handler_indexed (no switch_ddc) that calls vga_switcheroo_lock_ddc(some_pdev), checks the -ENODEV return, and deliberately (or via a bug) returns without calling unlock_ddc.

After that, any other thread taking mux_hw_lk (e.g. write 'MIGD' to /dev/vga_switcheroo as root, which hits lines 1184–1187, or i915's vga_switcheroo_force_migd at i915_drv.c:742) blocks forever in mutex_lock → lockmgr(LK_EXCLUSIVE) → tsleep.

Success criterion: the write/MIGD command never returns and the system becomes unresponsive to GPU switching.

Recommended fix

Release the lock before returning -ENODEV.

--- a/sys/dev/video/vga/vga_switcheroo.c
+++ b/sys/dev/video/vga/vga_switcheroo.c
@@ -556,7 +556,9 @@ vga_switcheroo_lock_ddc(struct pci_dev *pdev)
    mutex_lock(&vgasr_priv->mux_hw_lk);
    if (!vgasr_priv->handler || !vgasr_priv->handler->switch_ddc) {
        vgasr_priv->old_ddc_owner = -ENODEV;
+       mutex_unlock(&vgasr_priv->mux_hw_lk);
        return -ENODEV;
    }

This matches unlock_ddc's expectation (lines 594–601) and removes the requirement that callers always pair the call.

Related findings

• DF-1511 (sibling): NULL-deref in force_migd in same file.
• DF-1513 (sibling): negative errno convention in same file.