Re: [PATCH v4] drm/qxl: fix use-after-free in qxl_irq_handler on PCI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/qxl: incorrect global disablement of shared PCI interrupt
- [High] drm/qxl: hardware teardown times out due to premature free_irq()
--

commit 4dcfdbcd12c8dcfe24f833ddd26240d1479406f0
Author: Óscar Megía López <megia.oscar@gmail.com>

drm/qxl: fix use-after-free in qxl_irq_handler on PCI

This commit adds disable_irq() and free_irq() to qxl_pci_remove() to
prevent the IRQ handler from firing after device teardown begins, fixing
a reported KASAN use-after-free.

> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -160,6 +160,9 @@ qxl_pci_remove(struct pci_dev *pdev)
>  {
>  	struct drm_device *dev = pci_get_drvdata(pdev);
>  
> +	disable_irq(pdev->irq);

[Severity: High]
Does this disable a shared interrupt line globally?

Looking at qxl_irq_init(), the interrupt is requested with IRQF_SHARED:
request_irq(pdev->irq, qxl_irq_handler, IRQF_SHARED, ...);

If this interrupt line is shared with other PCI devices, calling
disable_irq() here will mask the interrupt at the hardware controller
level (like the IO-APIC). Could this prevent other devices sharing the
same line from receiving interrupts, causing them to hang?

To safely disable shared interrupts, should the driver mask interrupts via
its own device registers instead?

> +	free_irq(pdev->irq, dev);
> +

[Severity: High]
Will freeing the IRQ this early cause hardware teardown commands to time out?

After this free_irq(), drm_atomic_helper_shutdown() is called.
Furthermore, when the final reference is dropped, qxl_device_fini()
executes the following cleanup sequence:

qxl_kms.c:qxl_device_fini() {
    ...
    qxl_io_notify_oom(qdev);
    wait_event_timeout(qdev->release_event,
                       atomic_read(&qdev->release_count) == 0, HZ);
}

Since qxl_io_notify_oom() requires the interrupt handler to fire to process
the completion and signal release_event, will this wait now unconditionally
time out because the IRQ handler was already removed?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927101026.45411-1-megia.oscar@gmail.com?part=1