PCI/bwctrl: Fix NULL pointer deref on unbind and bind

JIRA: https://issues.redhat.com/browse/RHEL-81906
Upstream Status: 15b8968

Conflict(s):
  Patching file drivers/pci/pcie/bwctrl.c; Hunk #2 FAILED at 334.

  This is due to RHEL not having incorporated the "PCIe cooling" driver
  (see upstream commit d278b09 "thermal: Add PCIe cooling driver").


commit 15b8968
Author: Lukas Wunner <[email protected]>
Date:   Mon Jan 6 12:26:35 2025 +0100

    PCI/bwctrl: Fix NULL pointer deref on unbind and bind

    The interrupt handler for bandwidth notifications, pcie_bwnotif_irq(),
    dereferences a "data" pointer.

    On unbind, that pointer is set to NULL by pcie_bwnotif_remove().  However
    the interrupt handler may still be invoked afterwards and will dereference
    that NULL pointer.

    That's because the interrupt is requested using a devm_*() helper and the
    driver core releases devm_*() resources *after* calling ->remove().

    pcie_bwnotif_remove() does clear the Link Bandwidth Management Interrupt
    Enable and Link Autonomous Bandwidth Interrupt Enable bits in the Link
    Control Register, but that won't prevent execution of pcie_bwnotif_irq():
    The interrupt for bandwidth notifications may be shared with AER, DPC,
    PME, and hotplug.  So pcie_bwnotif_irq() may be executed as long as the
    interrupt is requested.

    There's a similar race on bind:  pcie_bwnotif_probe() requests the
    interrupt when the "data" pointer still points to NULL.  A NULL pointer
    deref may thus likewise occur if AER, DPC, PME or hotplug raise an
    interrupt in-between the bandwidth controller's call to devm_request_irq()
    and assignment of the "data" pointer.

    Drop the devm_*() usage and reorder requesting of the interrupt to fix the
    issue.

    While at it, drop a stray but harmless no_free_ptr() invocation when
    assigning the "data" pointer in pcie_bwnotif_probe().

    Ilpo points out that the locking on unbind and bind needs to be symmetric,
    so move the call to pcie_bwnotif_disable() inside the critical section
    protected by pcie_bwctrl_setspeed_rwsem and pcie_bwctrl_lbms_rwsem.

    Evert reports a hang on shutdown of an ASUS ROG Strix SCAR 17 G733PYV.
    The issue is no longer reproducible with the present commit.

    Evert found that attaching a USB-C monitor prevented the hang.  The
    machine contains an ASMedia USB 3.2 controller below a hotplug-capable
    Root Port.  So one possible explanation is that the controller gets
    hot-removed on shutdown unless something is connected.  And the ensuing
    hotplug interrupt occurs exactly when the bandwidth controller is
    unregistering.  The precise cause could not be determined because the
    screen had already turned black when the hang occurred.

    Link: https://lore.kernel.org/r/ae2b02c9cfbefff475b6e132b0aa962aaccbd7b2.1736162539.git.lukas@wunner.de
    Fixes: 665745f ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller")
    Reported-by: Evert Vorster <[email protected]>
    Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219629
    Signed-off-by: Lukas Wunner <[email protected]>
    Signed-off-by: Bjorn Helgaas <[email protected]>
    Tested-by: Evert Vorster <[email protected]>
    Reviewed-by: Ilpo Järvinen <[email protected]>

Signed-off-by: Myron Stowe <[email protected]>

Author: Myron Stowe

drivers/pci/pcie/bwctrl.c

@@ -300,14 +300,17 @@ static int pcie_bwnotif_probe(struct pcie_device *srv)
 	if (ret)
 		return ret;
 
-	ret = devm_request_irq(&srv->device, srv->irq, pcie_bwnotif_irq,
-			       IRQF_SHARED, "PCIe bwctrl", srv);
-	if (ret)
-		return ret;
-
 	scoped_guard(rwsem_write, &pcie_bwctrl_setspeed_rwsem) {
 		scoped_guard(rwsem_write, &pcie_bwctrl_lbms_rwsem) {
-			port->link_bwctrl = no_free_ptr(data);
+			port->link_bwctrl = data;
+
+			ret = request_irq(srv->irq, pcie_bwnotif_irq,
+					  IRQF_SHARED, "PCIe bwctrl", srv);
+			if (ret) {
+				port->link_bwctrl = NULL;
+				return ret;
+			}
+
 			pcie_bwnotif_enable(srv);
 		}
 	}
@@ -319,11 +322,15 @@ static int pcie_bwnotif_probe(struct pcie_device *srv)
 
 static void pcie_bwnotif_remove(struct pcie_device *srv)
 {
-	pcie_bwnotif_disable(srv->port);
+	scoped_guard(rwsem_write, &pcie_bwctrl_setspeed_rwsem) {
+		scoped_guard(rwsem_write, &pcie_bwctrl_lbms_rwsem) {
+			pcie_bwnotif_disable(srv->port);
+
+			free_irq(srv->irq, srv);
 
-	scoped_guard(rwsem_write, &pcie_bwctrl_setspeed_rwsem)
-		scoped_guard(rwsem_write, &pcie_bwctrl_lbms_rwsem)
 			srv->port->link_bwctrl = NULL;
+		}
+	}
 }
 
 static int pcie_bwnotif_suspend(struct pcie_device *srv)