[PATCH] PCI/ASPM: Add missing NULL check for link in sysfs show/store callbacks

[PATCH] PCI/ASPM: Add missing NULL check for link in sysfs show/store callbacks

[ZhaoJinming]

The aspm_attr_show_common(), aspm_attr_store_common(), clkpm_show()
and clkpm_store() sysfs callbacks call pcie_aspm_get_link() without
checking the return value. A NULL return would indicate the device is
not PCIe, has no upstream bridge, or the link state has not been
initialized.

While the aspm_ctrl_attrs_are_visible() is_visible callback already
hides these attributes when link is NULL, add the NULL check for
defense in depth and consistency with other callers of
pcie_aspm_get_link() in the same file (__pci_disable_link_state,
__pci_enable_link_state, pcie_aspm_enabled).

Signed-off-by: ZhaoJinming <zhaojinming@uniontech.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 172783e7f519..108d151a786e 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1654,6 +1654,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 0;
+
 	return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
 }
 
@@ -1665,6 +1668,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;
 
@@ -1713,6 +1719,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 0;
+
 	return sysfs_emit(buf, "%d\n", link->clkpm_enabled);
 }
 
@@ -1724,6 +1733,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.20.1

Re: [PATCH] PCI/ASPM: Add missing NULL check for link in sysfs show/store callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Inconsistent Return Code in sysfs show callbacks

Pre-existing issues:
- [High] Use-After-Free (TOCTOU Race) due to lockless fetch of pcie_link_state
--

--- Patch [1]: [PATCH] PCI/ASPM: Add missing NULL check for link in sysfs show/store callbacks ---
commit 123c7550e88a60508484ee2915e5a520ce8e1364
Author: ZhaoJinming <zhaojinming@uniontech.com>

PCI/ASPM: Add missing NULL check for link in sysfs show/store callbacks

This commit adds NULL checks for the return value of pcie_aspm_get_link() in
the ASPM and clkpm sysfs show and store callbacks. It aims to prevent NULL
pointer dereferences when the device is not PCIe or the link state has not
been initialized.

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 172783e7f519a..108d151a786e7 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -1654,6 +1654,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 0;
> +

[Severity: Low]
Does returning 0 here mean a successful read of 0 bytes, presenting an empty
file to userspace instead of an error? Should this return -ENODEV similar
to the corresponding store callback?

>  	return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
>  }
>  
> @@ -1665,6 +1668,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;
> +

[Severity: High]
This isn't a bug introduced by this patch, but does fetching the link
locklessly here introduce a use-after-free race?

If a concurrent device hot-unplug of a sibling function occurs,
pcie_aspm_exit_link_state() could acquire the aspm_lock, free the shared link
structure, and set the parent's link_state to NULL.

Because this sysfs callback fetches the link before acquiring the lock, could
it hold a dangling pointer, which is then dereferenced later in this function
when the lock is acquired?

aspm_attr_store_common()
...
	down_read(&pci_bus_sem);
	mutex_lock(&aspm_lock);
...
	link->aspm_disable &= ~state;

Does this same lockless fetch race also exist in clkpm_store(),
aspm_attr_show_common(), and clkpm_show()?

[ ... ]

> @@ -1713,6 +1719,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 0;
> +

[Severity: Low]
Does returning 0 here mean a successful read of 0 bytes, presenting an empty
file to userspace instead of an error? Should this return -ENODEV similar
to the clkpm_store() callback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727072915.3954329-1-zhaojinming@uniontech.com?part=1

Re: [PATCH] PCI/ASPM: Add missing NULL check for link in sysfs show/store callbacks

[Lukas Wunner]

On Mon, Jul 27, 2026 at 03:29:15PM +0800, ZhaoJinming wrote:
> The aspm_attr_show_common(), aspm_attr_store_common(), clkpm_show()
> and clkpm_store() sysfs callbacks call pcie_aspm_get_link() without
> checking the return value. A NULL return would indicate the device is
> not PCIe, has no upstream bridge, or the link state has not been
> initialized.
> 
> While the aspm_ctrl_attrs_are_visible() is_visible callback already
> hides these attributes when link is NULL, add the NULL check for
> defense in depth and consistency with other callers of
> pcie_aspm_get_link() in the same file (__pci_disable_link_state,
> __pci_enable_link_state, pcie_aspm_enabled).

Well, if we don't need the checks because the ->is_visible() callback
already hides the attributes, then we shouldn't add them.

Otherwise it's quite possible that someone else comes along and deletes
the checks again because they're superfluous.  

Thanks,

Lukas