[PATCH 0/3] PCI: Fix ECAM config window use-after-free in five drivers

[PATCH 0/3] PCI: Fix ECAM config window use-after-free in five drivers

[Hongyan Xu]

Five PCI host controller drivers use pci_host_common_probe() or
pci_host_common_init() but install no ->remove, so unbinding frees the ECAM
config window through devres while the bus tree is still registered, and
the next config space access is a use-after-free.

Patch 1/3 is the apple driver.  CONFIG_PCIE_APPLE=m plus
module_platform_driver() means one root command, "rmmod apple_pcie", is
enough to trigger it.

This series fixes the lifetime of cfg only.  It does not make these drivers
safe to unload: apple's IRQ handlers and microchip's PLDA IRQ domains are
still not released, both pre-existing and out of scope here.

The defect was found by code review; the trigger steps are in each patch,
but no oops or KASAN report is attached, as none was measured.

Patch 3/3 is one commit for the three remaining drivers and therefore
carries three Fixes: tags; it can be split apart if you prefer.

Hongyan Xu (3):
  PCI: apple: Fix ECAM config window use-after-free
  PCI: microchip: Fix ECAM config window use-after-free
  PCI: thunder-pem, hisi, thunder-ecam: Fix ECAM config window
    use-after-free

 drivers/pci/controller/dwc/pcie-hisi.c            | 1 +
 drivers/pci/controller/pci-thunder-ecam.c         | 1 +
 drivers/pci/controller/pci-thunder-pem.c          | 1 +
 drivers/pci/controller/pcie-apple.c               | 1 +
 drivers/pci/controller/plda/pcie-microchip-host.c | 1 +
 5 files changed, 5 insertions(+)

-- 
2.50.1.windows.1

[PATCH 1/3] PCI: apple: Fix ECAM config window use-after-free

[Hongyan Xu]

pcie-apple.c has no ->remove for pci_host_common_init(), so unbinding only
runs devres_release_all(): gen_pci_unmap_cfg() -> pci_ecam_free() frees the
ECAM config window while the bus tree stays registered - only
pci_remove_bus() unlinks a bus, and only pci_host_common_remove() reaches
it for a root bus.  The next config access is a use-after-free in
pci_ecam_map_bus(), which reads cfg->ops->bus_shift.
Trigger: CONFIG_PCIE_APPLE=m; as root, one command: # rmmod apple_pcie

Fix: add .remove = pci_host_common_remove, as pci-host-generic.c does since
commit 01fcb7f777a9 ("PCI: Add support for unbinding the generic PCI host
controller"), so pci_remove_root_bus() unlinks the bus before devres frees
cfg.  Scope: cfg lifetime only; its IRQ handlers stay unreleased, so the
module is still unsafe to unload.

Fixes: 1e33888fbe44 ("PCI: apple: Add initial hardware bring-up")
Cc: stable@vger.kernel.org
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
 drivers/pci/controller/pcie-apple.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index 2d92fc79f6dd..805171551e01 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -884,6 +884,7 @@ MODULE_DEVICE_TABLE(of, apple_pcie_of_match);
 
 static struct platform_driver apple_pcie_driver = {
 	.probe	= apple_pcie_probe,
+	.remove	= pci_host_common_remove,
 	.driver	= {
 		.name			= "pcie-apple",
 		.of_match_table		= apple_pcie_of_match,
-- 
2.50.1.windows.1

[PATCH 2/3] PCI: microchip: Fix ECAM config window use-after-free

[Hongyan Xu]

mc_host_probe() calls pci_host_common_probe() but has no ->remove, so an
unbind only runs devres_release_all(): gen_pci_unmap_cfg() ->
pci_ecam_free() frees the ECAM config window while the bus tree stays
registered - only pci_remove_bus() unlinks a bus, and only
pci_host_common_remove() reaches it for a root bus.  The next config access
is a use-after-free in pci_ecam_map_bus(), which reads cfg->ops->bus_shift.
Trigger: overlay rmdir in configfs, not rmmod (builtin_platform_driver()).

Fix: add .remove = pci_host_common_remove, as pci-host-generic.c does since
commit 01fcb7f777a9 ("PCI: Add support for unbinding the generic PCI host
controller"), so pci_remove_root_bus() unlinks the bus before devres frees
cfg.  Scope: the PLDA IRQ domains it creates are not devres managed and
stay unreleased - a separate, pre-existing issue.

Fixes: 6f15a9c9f941 ("PCI: microchip: Add Microchip PolarFire PCIe controller driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
 drivers/pci/controller/plda/pcie-microchip-host.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/plda/pcie-microchip-host.c b/drivers/pci/controller/plda/pcie-microchip-host.c
index 24bbf93b8051..f7b75013d9fc 100644
--- a/drivers/pci/controller/plda/pcie-microchip-host.c
+++ b/drivers/pci/controller/plda/pcie-microchip-host.c
@@ -821,6 +821,7 @@ MODULE_DEVICE_TABLE(of, mc_pcie_of_match);
 
 static struct platform_driver mc_pcie_driver = {
 	.probe = mc_host_probe,
+	.remove = pci_host_common_remove,
 	.driver = {
 		.name = "microchip-pcie",
 		.of_match_table = mc_pcie_of_match,
-- 
2.50.1.windows.1

[PATCH 3/3] PCI: thunder-pem, hisi, thunder-ecam: Fix ECAM config window use-after-free

[Hongyan Xu]

thunder-pem, hisi and thunder-ecam set .probe = pci_host_common_probe but
have no ->remove, so unbinding only runs devres_release_all():
gen_pci_unmap_cfg() -> pci_ecam_free() frees the ECAM config window while
the bus tree stays registered - only pci_remove_bus() unlinks a bus, and
only pci_host_common_remove() reaches it for a root bus.  The next config
access in pci_ecam_map_bus() reads cfg->ops->bus_shift.
Trigger: DT overlay removal of the node that supplied or enabled it.

Fix: give each driver the same .remove = pci_host_common_remove that
pci-host-generic.c got in commit 01fcb7f777a9 ("PCI: Add support for
unbinding the generic PCI host controller"), so the unlink comes first.

Fixes: f12b76e56ada ("PCI: thunder: Add PCIe host driver for ThunderX processors")
Fixes: a2ec1996098c ("PCI: hisi: Add DT almost-ECAM support for Hip06/Hip07 host controllers")
Fixes: 7b6e7ba8e81a ("PCI: thunder: Add driver for ThunderX-pass{1,2} on-chip devices")
Cc: stable@vger.kernel.org
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
 drivers/pci/controller/dwc/pcie-hisi.c    | 1 +
 drivers/pci/controller/pci-thunder-ecam.c | 1 +
 drivers/pci/controller/pci-thunder-pem.c  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-hisi.c b/drivers/pci/controller/dwc/pcie-hisi.c
index 3c17897e56fc..5ed58e5db797 100644
--- a/drivers/pci/controller/dwc/pcie-hisi.c
+++ b/drivers/pci/controller/dwc/pcie-hisi.c
@@ -169,6 +169,7 @@ static const struct of_device_id hisi_pcie_almost_ecam_of_match[] = {
 
 static struct platform_driver hisi_pcie_almost_ecam_driver = {
 	.probe  = pci_host_common_probe,
+	.remove = pci_host_common_remove,
 	.driver = {
 		   .name = "hisi-pcie-almost-ecam",
 		   .of_match_table = hisi_pcie_almost_ecam_of_match,
diff --git a/drivers/pci/controller/pci-thunder-ecam.c b/drivers/pci/controller/pci-thunder-ecam.c
index b5b4a958e6a2..4745f5d00b5a 100644
--- a/drivers/pci/controller/pci-thunder-ecam.c
+++ b/drivers/pci/controller/pci-thunder-ecam.c
@@ -358,6 +358,7 @@ static struct platform_driver thunder_ecam_driver = {
 		.suppress_bind_attrs = true,
 	},
 	.probe = pci_host_common_probe,
+	.remove = pci_host_common_remove,
 };
 builtin_platform_driver(thunder_ecam_driver);
 
diff --git a/drivers/pci/controller/pci-thunder-pem.c b/drivers/pci/controller/pci-thunder-pem.c
index 5fa037fb61dc..80cabbf263be 100644
--- a/drivers/pci/controller/pci-thunder-pem.c
+++ b/drivers/pci/controller/pci-thunder-pem.c
@@ -474,6 +474,7 @@ static struct platform_driver thunder_pem_driver = {
 		.suppress_bind_attrs = true,
 	},
 	.probe = pci_host_common_probe,
+	.remove = pci_host_common_remove,
 };
 builtin_platform_driver(thunder_pem_driver);
 
-- 
2.50.1.windows.1