Commit 3bd7db63 authored by Yinghai Lu's avatar Yinghai Lu Committed by Bjorn Helgaas

PCI/ASPM: Always set link->downstream to avoid NULL dereference on remove

We call pcie_aspm_exit_link_state() when we remove a device.  If the device
is the last PCIe function to be removed below a bridge and the bridge has
an ASPM link_state struct, we disable ASPM on the link.  Disabling ASPM
requires link->downstream (used in pcie_config_aspm_link()).

We previously set link->downstream in pcie_aspm_cap_init(), but only if the
device was not blacklisted.  Removing the blacklisted device caused a NULL
pointer dereference in the pcie_aspm_exit_link_state() ->
pcie_config_aspm_link() path:

  # echo 1 > /sys/bus/pci/devices/0000\:0b\:00.0/remove
  ...
   BUG: unable to handle kernel NULL pointer dereference at 0000000000000080
   IP: pcie_config_aspm_link+0x5d/0x2b0
   Call Trace:
    pcie_aspm_exit_link_state+0x75/0x130
    pci_stop_bus_device+0xa4/0xb0
    pci_stop_and_remove_bus_device_locked+0x1a/0x30
    remove_store+0x50/0x70
    dev_attr_store+0x18/0x30
    sysfs_kf_write+0x44/0x60
    kernfs_fop_write+0x10e/0x190
    __vfs_write+0x28/0x110
    ? rcu_read_lock_sched_held+0x5d/0x80
    ? rcu_sync_lockdep_assert+0x2c/0x60
    ? __sb_start_write+0x173/0x1a0
    ? vfs_write+0xb3/0x180
    vfs_write+0xc4/0x180
    SyS_write+0x49/0xa0
    do_syscall_64+0xa6/0x1c0
    entry_SYSCALL64_slow_path+0x25/0x25
   ---[ end trace bd187ee0267df5d9 ]---

To avoid this, set link->downstream in alloc_pcie_link_state(), so every
pcie_link_state structure has a valid link->downstream pointer.

[bhelgaas: changelog]
Signed-off-by: default avatarYinghai Lu <yinghai@kernel.org>
Signed-off-by: default avatarBjorn Helgaas <bhelgaas@google.com>
Acked-by: default avatarRajat Jain <rajatja@google.com>
CC: stable@vger.kernel.org
parent 0d5370d1
...@@ -478,7 +478,7 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link, ...@@ -478,7 +478,7 @@ static void aspm_calc_l1ss_info(struct pcie_link_state *link,
static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
{ {
struct pci_dev *child, *parent = link->pdev; struct pci_dev *child = link->downstream, *parent = link->pdev;
struct pci_bus *linkbus = parent->subordinate; struct pci_bus *linkbus = parent->subordinate;
struct aspm_register_info upreg, dwreg; struct aspm_register_info upreg, dwreg;
...@@ -491,9 +491,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) ...@@ -491,9 +491,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
/* Get upstream/downstream components' register state */ /* Get upstream/downstream components' register state */
pcie_get_aspm_reg(parent, &upreg); pcie_get_aspm_reg(parent, &upreg);
child = pci_function_0(linkbus);
pcie_get_aspm_reg(child, &dwreg); pcie_get_aspm_reg(child, &dwreg);
link->downstream = child;
/* /*
* If ASPM not supported, don't mess with the clocks and link, * If ASPM not supported, don't mess with the clocks and link,
...@@ -800,6 +798,7 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev) ...@@ -800,6 +798,7 @@ static struct pcie_link_state *alloc_pcie_link_state(struct pci_dev *pdev)
INIT_LIST_HEAD(&link->children); INIT_LIST_HEAD(&link->children);
INIT_LIST_HEAD(&link->link); INIT_LIST_HEAD(&link->link);
link->pdev = pdev; link->pdev = pdev;
link->downstream = pci_function_0(pdev->subordinate);
/* /*
* Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe * Root Ports and PCI/PCI-X to PCIe Bridges are roots of PCIe
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment