[PATCH] PCI/ASPM: Fix NULL pointer dereference in sysfs show/store callbacks

[PATCH] PCI/ASPM: Fix NULL pointer dereference in sysfs show/store callbacks

[Shawn Lin]

Reading or writing a link state sysfs attribute after the upstream
bridge's pcie_link_state has been freed triggers a NULL pointer
dereference.

This was first seen on a 6.12 based kernel by userspace stress testing a
PCIe endpoint on an RK3588 EVB1 (Intel I350 dual-port NIC behind the
root port). The same NULL pointer dereference exists in upstream and can
be reproduced reliably on:

  # lspci
  00:00.0 PCI bridge: Rockchip Electronics Co., Ltd RK3588 (rev 01)
  01:00.0 Ethernet controller: Intel Corporation I350 Gigabit Network Connection (rev 01)
  01:00.1 Ethernet controller: Intel Corporation I350 Gigabit Network Connection (rev 01)

by running:

  echo 1 > /sys/bus/pci/devices/0000:01:00.0/remove
  cat /sys/bus/pci/devices/0000:01:00.1/link/l1_aspm

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030
Mem abort info:
  ESR = 0x0000000096000004
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x04: level 0 translation fault
Data abort info:
  ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
  CM = 0, WnR = 0, TnD = 0, TagAccess = 0
  GCS = 0, Overlay = 0, DirtyBit = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=0000000106959000
[0000000000000030] pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 0000000096000004 [#1]  SMP
Modules linked in:
CPU: 3 UID: 0 PID: 646 Comm: cat Not tainted 7.3.0-rc6-next-20261007-00007-gf2fe32674ebe-dirty #134 PREEMPT(full)
Hardware name: Rockchip RK3588 EVB1 V10 Board (DT)
pstate: 40400009 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : l1_aspm_show+0x64/0x88
lr : dev_attr_show+0x24/0x68
sp : ffff800086f9bbf0
x29: ffff800086f9bbf0 x28: ffff0001036c0000 x27: 0000000000000000
x26: 0000000000000000 x25: 000000007ffff001 x24: 0000000000000001
x23: ffff800081781630 x22: ffff0001008d3c00 x21: ffff0001053b9000
x20: ffff0001020880d0 x19: ffff800082441c28 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
x14: 0000000000000000 x13: 0000000000000000 x12: 00000000ffffe000
x11: ffff0001053b9000 x10: 0000000000c00010 x9 : 0000000000000070
x8 : 0000000000000000 x7 : 0000000000000000 x6 : 000000000000003f
x5 : 0000000000000040 x4 : 0000000000000000 x3 : 0000000000000004
x2 : ffff0001053b9000 x1 : ffff800082441c28 x0 : ffff0001053b9000
Call trace:
 l1_aspm_show+0x64/0x88 (P)
 dev_attr_show+0x24/0x68
 sysfs_kf_seq_show+0xb8/0x138
 kernfs_seq_show+0x2c/0x38
 seq_read_iter+0x120/0x3e0
 kernfs_fop_read_iter+0x60/0x188
 vfs_read+0x200/0x2b8
 ksys_read+0x74/0xe8
 __arm64_sys_read+0x1c/0x28
 invoke_syscall+0x44/0xec
 el0_svc_common+0x8c/0xd8
 do_el0_svc+0x1c/0x28
 el0_svc+0x3c/0x98
 el0t_64_sync_handler+0x84/0x12c
 el0t_64_sync+0x198/0x19c
Code: aa1f03e8 d503233f a9bf7bfd 910003fd (b9403108)
 ---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Oops: Fatal exception
SMP: stopping secondary CPUs
Kernel Offset: disabled
CPU features: 0x0,70000000,0070002a,01180a42,0800e20b
Memory Limit: none
 ---[ end Kernel panic - not syncing: Oops: Fatal exception ]---

The sysfs attributes are created for every device below the link, but
the link state is freed by pcie_aspm_exit_link_state() when function 0
of the downstream component is removed. A sibling function or a sysfs
reader still holding a reference can then call into the show/store
callbacks while pcie_aspm_get_link() returns NULL.

aspm_attr_show_common(), aspm_attr_store_common(), clkpm_show() and
clkpm_store() all dereference the link pointer unconditionally. Add the
missing NULL checks, mirroring pcie_aspm_enabled(), and return -ENODEV.

Cc: stable@vger.kernel.org
Fixes: 72ea91afbfb0 ("PCI/ASPM: Add sysfs attributes for controlling ASPM link states")
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/pci/pcie/aspm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index f8e6497..0e0ba0e6 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1777,6 +1777,9 @@ static ssize_t aspm_attr_show_common(struct device *dev,
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
+	if (!link)
+		return -ENODEV;
+
 	return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
 }
 
@@ -1788,6 +1791,9 @@ static ssize_t aspm_attr_store_common(struct device *dev,
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 	bool state_enable;
 
+	if (!link)
+		return -ENODEV;
+
 	if (kstrtobool(buf, &state_enable) < 0)
 		return -EINVAL;
 
@@ -1836,6 +1842,9 @@ static ssize_t clkpm_show(struct device *dev,
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
+	if (!link)
+		return -ENODEV;
+
 	return sysfs_emit(buf, "%d\n", link->clkpm_enabled);
 }
 
@@ -1847,6 +1856,9 @@ static ssize_t clkpm_store(struct device *dev,
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 	bool state_enable;
 
+	if (!link)
+		return -ENODEV;
+
 	if (kstrtobool(buf, &state_enable) < 0)
 		return -EINVAL;
 
-- 
2.7.4

Re: [PATCH] PCI/ASPM: Fix NULL pointer dereference in sysfs show/store callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci/aspm: use-after-free race in sysfs callbacks due to lockless pcie_link_state check
--

commit 9e5cf0a1d40aaa7028d286ec96a1c08c4a3dfd68
Author: Shawn Lin <shawn.lin@rock-chips.com>

    PCI/ASPM: Fix NULL pointer dereference in sysfs show/store callbacks
    
    Reading or writing a link state sysfs attribute after the upstream
    bridge's pcie_link_state has been freed triggers a NULL pointer
    dereference.

Cc: stable@vger.kernel.org

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index f8e64971c6c3d..0e0ba0e677e50 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -1777,6 +1777,9 @@ static ssize_t aspm_attr_show_common(struct device *dev,
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>  
> +	if (!link)
> +		return -ENODEV;
> +
>  	return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
>  }

[Severity: High]
Does this locklessly fetch and check the link pointer, leaving it exposed to
concurrent freeing?

If another process triggers the removal of function 0 of the downstream
component concurrently, pcie_aspm_exit_link_state() could acquire the locks,
clear the link state, and free the link after the check but before
sysfs_emit() dereferences it. Can this lead to a use-after-free regression?

> @@ -1788,6 +1791,9 @@ static ssize_t aspm_attr_store_common(struct device *dev,
>  	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>  	bool state_enable;
>  
> +	if (!link)
> +		return -ENODEV;
> +
>  	if (kstrtobool(buf, &state_enable) < 0)
>  		return -EINVAL;
>  

[Severity: High]
Could this cause a similar regression here?

The pointer is validated locklessly but dereferenced inside the lock. It can
be freed by a concurrent thread while the current thread is waiting for
pci_bus_sem or aspm_lock further down in the function.

> @@ -1836,6 +1842,9 @@ static ssize_t clkpm_show(struct device *dev,
>  	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>  
> +	if (!link)
> +		return -ENODEV;
> +
>  	return sysfs_emit(buf, "%d\n", link->clkpm_enabled);
>  }

[Severity: High]
Does this code have the same use-after-free race condition?

The link pointer is validated outside any locks, but could be freed by a
concurrent removal before it is dereferenced in sysfs_emit().

> @@ -1847,6 +1856,9 @@ static ssize_t clkpm_store(struct device *dev,
>  	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>  	bool state_enable;
>  
> +	if (!link)
> +		return -ENODEV;
> +
>  	if (kstrtobool(buf, &state_enable) < 0)
>  		return -EINVAL;
>  

[Severity: High]
Does this demonstrate the same race condition as aspm_attr_store_common()?

The pointer is validated locklessly, but dereferenced inside the lock. It can
be freed by a concurrent thread while the current thread is waiting for the
locks further down in the function.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/1791596305-153177-1-git-send-email-shawn.lin@rock-chips.com?part=1

Re: [PATCH] PCI/ASPM: Fix NULL pointer dereference in sysfs show/store callbacks

[Shawn Lin]

Hi, Sashiko and maintainers

在 2026/10/10 星期六 11:01, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] pci/aspm: use-after-free race in sysfs callbacks due to lockless pcie_link_state check
> --
> 
> commit 9e5cf0a1d40aaa7028d286ec96a1c08c4a3dfd68
> Author: Shawn Lin <shawn.lin@rock-chips.com>
> 
>      PCI/ASPM: Fix NULL pointer dereference in sysfs show/store callbacks
>      
>      Reading or writing a link state sysfs attribute after the upstream
>      bridge's pcie_link_state has been freed triggers a NULL pointer
>      dereference.
> 
> Cc: stable@vger.kernel.org
> 
>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> index f8e64971c6c3d..0e0ba0e677e50 100644
>> --- a/drivers/pci/pcie/aspm.c
>> +++ b/drivers/pci/pcie/aspm.c
>> @@ -1777,6 +1777,9 @@ static ssize_t aspm_attr_show_common(struct device *dev,
>>   	struct pci_dev *pdev = to_pci_dev(dev);
>>   	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>>   
>> +	if (!link)
>> +		return -ENODEV;
>> +
>>   	return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
>>   }
> 
> [Severity: High]
> Does this locklessly fetch and check the link pointer, leaving it exposed to
> concurrent freeing?
> 

Sashiko, thanks for reporting it.

This is not a regression introduced by my patch. Both the NULL
dereference and the use-after-free pre-exist in the original code, where
the link pointer is fetched and dereferenced without holding aspm_lock,
while pcie_aspm_exit_link_state() clears and kfree()s the link state
under aspm_lock. My patch only covered the NULL dereference (the part I
could reproduce); it did not touch the existing UAF window.

The UAF window is extremely narrow and is not reachable by a plain
userspace sequence, so I'd prefer to keep this patch focused on the NULL
dereference, which has a reliable reproduction. Serializing the lookup
against aspm_lock would be a larger change and is not suitable for
backporting to stable. It can be addressed separately if needed.

Hi Bjorn,

Please let know if you'd prefer to address UAF issue in the same patch?

> If another process triggers the removal of function 0 of the downstream
> component concurrently, pcie_aspm_exit_link_state() could acquire the locks,
> clear the link state, and free the link after the check but before
> sysfs_emit() dereferences it. Can this lead to a use-after-free regression?
> 
>> @@ -1788,6 +1791,9 @@ static ssize_t aspm_attr_store_common(struct device *dev,
>>   	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>>   	bool state_enable;
>>   
>> +	if (!link)
>> +		return -ENODEV;
>> +
>>   	if (kstrtobool(buf, &state_enable) < 0)
>>   		return -EINVAL;
>>   
> 
> [Severity: High]
> Could this cause a similar regression here?
> 
> The pointer is validated locklessly but dereferenced inside the lock. It can
> be freed by a concurrent thread while the current thread is waiting for
> pci_bus_sem or aspm_lock further down in the function.
> 
>> @@ -1836,6 +1842,9 @@ static ssize_t clkpm_show(struct device *dev,
>>   	struct pci_dev *pdev = to_pci_dev(dev);
>>   	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>>   
>> +	if (!link)
>> +		return -ENODEV;
>> +
>>   	return sysfs_emit(buf, "%d\n", link->clkpm_enabled);
>>   }
> 
> [Severity: High]
> Does this code have the same use-after-free race condition?
> 
> The link pointer is validated outside any locks, but could be freed by a
> concurrent removal before it is dereferenced in sysfs_emit().
> 
>> @@ -1847,6 +1856,9 @@ static ssize_t clkpm_store(struct device *dev,
>>   	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
>>   	bool state_enable;
>>   
>> +	if (!link)
>> +		return -ENODEV;
>> +
>>   	if (kstrtobool(buf, &state_enable) < 0)
>>   		return -EINVAL;
>>   
> 
> [Severity: High]
> Does this demonstrate the same race condition as aspm_attr_store_common()?
> 
> The pointer is validated locklessly, but dereferenced inside the lock. It can
> be freed by a concurrent thread while the current thread is waiting for the
> locks further down in the function.
>