Commit fc8c818e authored by Philipp Stanner's avatar Philipp Stanner Committed by Bjorn Helgaas

PCI: Fix potential deadlock in pcim_intx()

25216afc ("PCI: Add managed pcim_intx()") moved the allocation step for
pci_intx()'s device resource from pcim_enable_device() to pcim_intx(). As
before, pcim_enable_device() sets pci_dev.is_managed to true; and it is
never set to false again.

Due to the lifecycle of a struct pci_dev, it can happen that a second
driver obtains the same pci_dev after a first driver ran.  If one driver
uses pcim_enable_device() and the other doesn't, this causes the other
driver to run into managed pcim_intx(), which will try to allocate when
called for the first time.

Allocations might sleep, so calling pci_intx() while holding spinlocks
becomes then invalid, which causes lockdep warnings and could cause
deadlocks:

  ========================================================
  WARNING: possible irq lock inversion dependency detected
  6.11.0-rc6+ #59 Tainted: G        W
  --------------------------------------------------------
  CPU 0/KVM/1537 just changed the state of lock:
  ffffa0f0cff965f0 (&vdev->irqlock){-...}-{2:2}, at:
  vfio_intx_handler+0x21/0xd0 [vfio_pci_core] but this lock took another,
  HARDIRQ-unsafe lock in the past: (fs_reclaim){+.+.}-{0:0}

and interrupts could create inverse lock ordering between them.

other info that might help us debug this:

  Possible interrupt unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(fs_reclaim);
			       local_irq_disable();
			       lock(&vdev->irqlock);
			       lock(fs_reclaim);
  <Interrupt>
    lock(&vdev->irqlock);

  *** DEADLOCK ***

Have pcim_enable_device()'s release function, pcim_disable_device(), set
pci_dev.is_managed to false so that subsequent drivers using the same
struct pci_dev do not implicitly run into managed code.

Link: https://lore.kernel.org/r/20240905072556.11375-2-pstanner@redhat.com
Fixes: 25216afc ("PCI: Add managed pcim_intx()")
Reported-by: default avatarAlex Williamson <alex.williamson@redhat.com>
Closes: https://lore.kernel.org/all/20240903094431.63551744.alex.williamson@redhat.com/Suggested-by: default avatarAlex Williamson <alex.williamson@redhat.com>
Signed-off-by: default avatarPhilipp Stanner <pstanner@redhat.com>
Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
Tested-by: default avatarAlex Williamson <alex.williamson@redhat.com>
Reviewed-by: default avatarDamien Le Moal <dlemoal@kernel.org>
parent 8400291e
...@@ -483,6 +483,8 @@ static void pcim_disable_device(void *pdev_raw) ...@@ -483,6 +483,8 @@ static void pcim_disable_device(void *pdev_raw)
if (!pdev->pinned) if (!pdev->pinned)
pci_disable_device(pdev); pci_disable_device(pdev);
pdev->is_managed = false;
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment