linux-kernel - [PATCH] PCI/ASPM: fix UAF by disable ASPM for link when child function is removed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230504123418.4438-1-dinghui@sangfor.com.cn>
Date:   Thu,  4 May 2023 20:34:18 +0800
From:   Ding Hui <dinghui@...gfor.com.cn>
To:     bhelgaas@...gle.com
Cc:     sathyanarayanan.kuppuswamy@...ux.intel.com, vidyas@...dia.com,
        david.e.box@...ux.intel.com, kai.heng.feng@...onical.com,
        michael.a.bottini@...ux.intel.com, rajatja@...gle.com,
        qinzongquan@...gfor.com.cn, dinghui@...gfor.com.cn,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH] PCI/ASPM: fix UAF by disable ASPM for link when child function is removed

If the Function 0 of a Multi-Function device is software removed,
a freed downstream pointer will be left in struct pcie_link_state,
and then when pcie_config_aspm_link() be invoked from any path,
we will trigger use-after-free.

Based on the PCIe spec about ASPM Control (PCIe r6.0, sec 7.5.3.7),
for Multi-Function Devices (including ARI Devices), it is recommended
that software program the same value in all Functions. For ARI
Devices, ASPM Control is determined solely by the setting in Function 0.

So we can just disable ASPM of the whole component if any child
function is removed, the downstream pointer will be avoided from
use-after-free, that will also avoid other potential corner cases.

Fixes: b5a0a9b59c81 ("PCI/ASPM: Read and set up L1 substate capabilities")
Debugged-by: Zongquan Qin <qinzongquan@...gfor.com.cn>
Suggestion-by: Bjorn Helgaas <bhelgaas@...gle.com>
Signed-off-by: Ding Hui <dinghui@...gfor.com.cn>
---
 drivers/pci/pcie/aspm.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 66d7514ca111..1bf8306141aa 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1010,18 +1010,17 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 
 	down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
-	/*
-	 * All PCIe functions are in one slot, remove one function will remove
-	 * the whole slot, so just wait until we are the last function left.
-	 */
-	if (!list_empty(&parent->subordinate->devices))
-		goto out;
 
 	link = parent->link_state;
 	root = link->root;
 	parent_link = link->parent;
 
-	/* All functions are removed, so just disable ASPM for the link */
+	/*
+	 * Any function is removed (including software removing), just
+	 * disable ASPM for the link, in case we can not configure the same
+	 * setting for all functions.
+	 * See PCIe r6.0, sec 7.5.3.7.
+	 */
 	pcie_config_aspm_link(link, 0);
 	list_del(&link->sibling);
 	/* Clock PM is for endpoint device */
@@ -1032,7 +1031,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
 		pcie_update_aspm_capable(root);
 		pcie_config_aspm_path(parent_link);
 	}
-out:
+
 	mutex_unlock(&aspm_lock);
 	up_read(&pci_bus_sem);
 }
-- 
2.17.1