summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLukas Wunner <lukas@wunner.de>2024-05-29 16:32:09 +0200
committerBjorn Helgaas <bhelgaas@google.com>2024-05-30 19:17:34 +0200
commit9d573d19547b3fae0c1d4e5fce52bdad3fda3664 (patch)
tree7c207b8e0e7de7a51bc6295daa4f9ae008783dd0
parentLinux 6.10-rc1 (diff)
downloadlinux-9d573d19547b3fae0c1d4e5fce52bdad3fda3664.tar.xz
linux-9d573d19547b3fae0c1d4e5fce52bdad3fda3664.zip
PCI: pciehp: Detect device replacement during system sleep
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>
-rw-r--r--drivers/pci/hotplug/pciehp.h4
-rw-r--r--drivers/pci/hotplug/pciehp_core.c42
-rw-r--r--drivers/pci/hotplug/pciehp_hpc.c5
-rw-r--r--drivers/pci/hotplug/pciehp_pci.c4
4 files changed, 54 insertions, 1 deletions
diff --git a/drivers/pci/hotplug/pciehp.h b/drivers/pci/hotplug/pciehp.h
index e0a614acee05..273dd8c66f4e 100644
--- a/drivers/pci/hotplug/pciehp.h
+++ b/drivers/pci/hotplug/pciehp.h
@@ -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;
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c
index ddd55ad97a58..ff458e692fed 100644
--- a/drivers/pci/hotplug/pciehp_core.c
+++ b/drivers/pci/hotplug/pciehp_core.c
@@ -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
diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index b1d0a1b3917d..061f01f60db4 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -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;
}
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index ad12515a4a12..65e50bee1a8c 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -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;