]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
PCI: pciehp: Detect device replacement during system sleep
authorLukas Wunner <lukas@wunner.de>
Wed, 29 May 2024 14:32:09 +0000 (16:32 +0200)
committerBjorn Helgaas <bhelgaas@google.com>
Thu, 30 May 2024 17:17:34 +0000 (12:17 -0500)
Ricky reports that replacing a device in a hotplug slot during ACPI sleep
state S3 does not cause re-enumeration on resume, as one would expect.
Instead, the new device is treated as if it was the old one.

There is no bulletproof way to detect device replacement, but as a
heuristic, check whether the device identity in config space matches cached
data in struct pci_dev (Vendor ID, Device ID, Class Code, Revision ID,
Subsystem Vendor ID, Subsystem ID).  Additionally, cache and compare the
Device Serial Number (PCIe r6.2 sec 7.9.3).  If a mismatch is detected,
mark the old device disconnected (to prevent its driver from accessing the
new device) and synthesize a Presence Detect Changed event.

The device identity in config space which is compared here is the same as
the one included in the signed Subject Alternative Name per PCIe r6.1 sec
6.31.3.  Thus, the present commit prevents attacks where a valid device is
replaced with a malicious device during system sleep and the valid device's
driver obliviously accesses the malicious device.

This is about as much as can be done at the PCI layer.  Drivers may have
additional ways to identify devices (such as reading a WWID from some
register) and may trigger re-enumeration when detecting an identity change
on resume.

Link: https://lore.kernel.org/r/a1afaa12f341d146ecbea27c1743661c71683833.1716992815.git.lukas@wunner.de
Reported-by: Ricky Wu <ricky_wu@realtek.com>
Closes: https://lore.kernel.org/r/a608b5930d0a48f092f717c0e137454b@realtek.com
Tested-by: Ricky Wu <ricky_wu@realtek.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
drivers/pci/hotplug/pciehp.h
drivers/pci/hotplug/pciehp_core.c
drivers/pci/hotplug/pciehp_hpc.c
drivers/pci/hotplug/pciehp_pci.c

index e0a614acee059b87e578e08716c6496d9ed1f80b..273dd8c66f4eff8b62ab065cebf97db3c343977d 100644 (file)
@@ -46,6 +46,9 @@ extern int pciehp_poll_time;
 /**
  * struct controller - PCIe hotplug controller
  * @pcie: pointer to the controller's PCIe port service device
+ * @dsn: cached copy of Device Serial Number of Function 0 in the hotplug slot
+ *     (PCIe r6.2 sec 7.9.3); used to determine whether a hotplugged device
+ *     was replaced with a different one during system sleep
  * @slot_cap: cached copy of the Slot Capabilities register
  * @inband_presence_disabled: In-Band Presence Detect Disable supported by
  *     controller and disabled per spec recommendation (PCIe r5.0, appendix I
@@ -87,6 +90,7 @@ extern int pciehp_poll_time;
  */
 struct controller {
        struct pcie_device *pcie;
+       u64 dsn;
 
        u32 slot_cap;                           /* capabilities and quirks */
        unsigned int inband_presence_disabled:1;
index ddd55ad97a58983ec11d9996e30d92bcdefdd6e0..ff458e692fedb3a9e05fb6b80f5eb9997fc8c6af 100644 (file)
@@ -284,6 +284,32 @@ static int pciehp_suspend(struct pcie_device *dev)
        return 0;
 }
 
+static bool pciehp_device_replaced(struct controller *ctrl)
+{
+       struct pci_dev *pdev __free(pci_dev_put);
+       u32 reg;
+
+       pdev = pci_get_slot(ctrl->pcie->port->subordinate, PCI_DEVFN(0, 0));
+       if (!pdev)
+               return true;
+
+       if (pci_read_config_dword(pdev, PCI_VENDOR_ID, &reg) ||
+           reg != (pdev->vendor | (pdev->device << 16)) ||
+           pci_read_config_dword(pdev, PCI_CLASS_REVISION, &reg) ||
+           reg != (pdev->revision | (pdev->class << 8)))
+               return true;
+
+       if (pdev->hdr_type == PCI_HEADER_TYPE_NORMAL &&
+           (pci_read_config_dword(pdev, PCI_SUBSYSTEM_VENDOR_ID, &reg) ||
+            reg != (pdev->subsystem_vendor | (pdev->subsystem_device << 16))))
+               return true;
+
+       if (pci_get_dsn(pdev) != ctrl->dsn)
+               return true;
+
+       return false;
+}
+
 static int pciehp_resume_noirq(struct pcie_device *dev)
 {
        struct controller *ctrl = get_service_data(dev);
@@ -293,9 +319,23 @@ static int pciehp_resume_noirq(struct pcie_device *dev)
        ctrl->cmd_busy = true;
 
        /* clear spurious events from rediscovery of inserted card */
-       if (ctrl->state == ON_STATE || ctrl->state == BLINKINGOFF_STATE)
+       if (ctrl->state == ON_STATE || ctrl->state == BLINKINGOFF_STATE) {
                pcie_clear_hotplug_events(ctrl);
 
+               /*
+                * If hotplugged device was replaced with a different one
+                * during system sleep, mark the old device disconnected
+                * (to prevent its driver from accessing the new device)
+                * and synthesize a Presence Detect Changed event.
+                */
+               if (pciehp_device_replaced(ctrl)) {
+                       ctrl_dbg(ctrl, "device replaced during system sleep\n");
+                       pci_walk_bus(ctrl->pcie->port->subordinate,
+                                    pci_dev_set_disconnected, NULL);
+                       pciehp_request(ctrl, PCI_EXP_SLTSTA_PDC);
+               }
+       }
+
        return 0;
 }
 #endif
index b1d0a1b3917d44b34985e89c541d63816605b703..061f01f60db4b9978284a77e6503a043972cc712 100644 (file)
@@ -1055,6 +1055,11 @@ struct controller *pcie_init(struct pcie_device *dev)
                }
        }
 
+       pdev = pci_get_slot(subordinate, PCI_DEVFN(0, 0));
+       if (pdev)
+               ctrl->dsn = pci_get_dsn(pdev);
+       pci_dev_put(pdev);
+
        return ctrl;
 }
 
index ad12515a4a121fe08a1d84f8c03d67676a207c2a..65e50bee1a8c08452575d239328cf7af219b8705 100644 (file)
@@ -72,6 +72,10 @@ int pciehp_configure_device(struct controller *ctrl)
        pci_bus_add_devices(parent);
        down_read_nested(&ctrl->reset_lock, ctrl->depth);
 
+       dev = pci_get_slot(parent, PCI_DEVFN(0, 0));
+       ctrl->dsn = pci_get_dsn(dev);
+       pci_dev_put(dev);
+
  out:
        pci_unlock_rescan_remove();
        return ret;