Re: [PATCH] PCI/ASPM: Update ASPM sysfs on MFD function removal to avoid use-after-free

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2024/7/30 9:16, Jay Fang wrote:
 From 'commit 456d8aa37d0f ("PCI/ASPM: Disable ASPM on MFD function removal
to avoid use-after-free")' we know that PCIe spec r6.0, sec 7.5.3.7,
recommends that software program the same ASPM Control(pcie_link_state)
value in all functions of multi-function devices, and free the
pcie_link_state when any child function is removed.

However, ASPM Control sysfs is still visible to other children even if it
has been removed by any child function, and careless use it will
trigger use-after-free error, e.g.:

   # lspci -tv
     -[0000:16]---00.0-[17]--+-00.0  Device 19e5:0222
                             \-00.1  Device 19e5:0222
   # echo 1 > /sys/bus/pci/devices/0000:17:00.0/remove       // pcie_link_state will be released
   # echo 1 > /sys/bus/pci/devices/0000:17:00.1/link/l1_aspm // will trigger error

   Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030
   Call trace:
    aspm_attr_store_common.constprop.0+0x10c/0x154
    l1_aspm_store+0x24/0x30
    dev_attr_store+0x20/0x34
    sysfs_kf_write+0x4c/0x5c

We can solve this problem by updating the ASPM Control sysfs of all
children immediately after ASPM Control have been freed.

Fixes: 456d8aa37d0f ("PCI/ASPM: Disable ASPM on MFD function removal to avoid use-after-free")
Signed-off-by: Jay Fang <f.fangjian@xxxxxxxxxx>
---
  drivers/pci/pcie/aspm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index cee2365e54b8..eee9e6739924 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1262,6 +1262,8 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev)
  		pcie_config_aspm_path(parent_link);
  	}
+ pcie_aspm_update_sysfs_visibility(parent);
+

To be more rigorous, is there still a race window in aspm_attr_{show,store}_common or
clkpm_{show,store} before updating the visibility, we can get an old or NULL pointer
by pcie_aspm_get_link()?

  	mutex_unlock(&aspm_lock);
  	up_read(&pci_bus_sem);
  }

--
Thanks,
- Ding Hui





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux