Skip to content

Commit 456d8aa

Browse files
cdkeybjorn-helgaas
authored andcommitted
PCI/ASPM: Disable ASPM on MFD function removal to avoid use-after-free
Struct pcie_link_state->downstream is a pointer to the pci_dev of function 0. Previously we retained that pointer when removing function 0, and subsequent ASPM policy changes dereferenced it, resulting in a use-after-free warning from KASAN, e.g.: # echo 1 > /sys/bus/pci/devices/0000:03:00.0/remove # echo powersave > /sys/module/pcie_aspm/parameters/policy BUG: KASAN: slab-use-after-free in pcie_config_aspm_link+0x42d/0x500 Call Trace: kasan_report+0xae/0xe0 pcie_config_aspm_link+0x42d/0x500 pcie_aspm_set_policy+0x8e/0x1a0 param_attr_store+0x162/0x2c0 module_attr_store+0x3e/0x80 PCIe spec r6.0, sec 7.5.3.7, recommends that software program the same ASPM Control value in all functions of multi-function devices. Disable ASPM and free the pcie_link_state when any child function is removed so we can discard the dangling pcie_link_state->downstream pointer and maintain the same ASPM Control configuration for all functions. [bhelgaas: commit log and comment] Debugged-by: Zongquan Qin <[email protected]> Suggested-by: Bjorn Helgaas <[email protected]> Fixes: b5a0a9b ("PCI/ASPM: Read and set up L1 substate capabilities") Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Ding Hui <[email protected]> Signed-off-by: Bjorn Helgaas <[email protected]>
1 parent ac9a786 commit 456d8aa

File tree

1 file changed

+12
-9
lines changed

1 file changed

+12
-9
lines changed

drivers/pci/pcie/aspm.c

+12-9
Original file line numberDiff line numberDiff line change
@@ -1010,29 +1010,32 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
10101010

10111011
down_read(&pci_bus_sem);
10121012
mutex_lock(&aspm_lock);
1013-
/*
1014-
* All PCIe functions are in one slot, remove one function will remove
1015-
* the whole slot, so just wait until we are the last function left.
1016-
*/
1017-
if (!list_empty(&parent->subordinate->devices))
1018-
goto out;
10191013

10201014
link = parent->link_state;
10211015
root = link->root;
10221016
parent_link = link->parent;
10231017

1024-
/* All functions are removed, so just disable ASPM for the link */
1018+
/*
1019+
* link->downstream is a pointer to the pci_dev of function 0. If
1020+
* we remove that function, the pci_dev is about to be deallocated,
1021+
* so we can't use link->downstream again. Free the link state to
1022+
* avoid this.
1023+
*
1024+
* If we're removing a non-0 function, it's possible we could
1025+
* retain the link state, but PCIe r6.0, sec 7.5.3.7, recommends
1026+
* programming the same ASPM Control value for all functions of
1027+
* multi-function devices, so disable ASPM for all of them.
1028+
*/
10251029
pcie_config_aspm_link(link, 0);
10261030
list_del(&link->sibling);
1027-
/* Clock PM is for endpoint device */
10281031
free_link_state(link);
10291032

10301033
/* Recheck latencies and configure upstream links */
10311034
if (parent_link) {
10321035
pcie_update_aspm_capable(root);
10331036
pcie_config_aspm_path(parent_link);
10341037
}
1035-
out:
1038+
10361039
mutex_unlock(&aspm_lock);
10371040
up_read(&pci_bus_sem);
10381041
}

0 commit comments

Comments
 (0)