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 <[email protected]> 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/[email protected]?part=1
