[RFC PATCH] PCI/ASPM: Handle missing link state in sysfs reads

[RFC PATCH] PCI/ASPM: Handle missing link state in sysfs reads

[Poulin, Louis]

From: Louis H. Poulin II <poulin.louis@student.oaklandcc.edu>

InputPlumber can hit a NULL pointer dereference at l0s_aspm_show()
while reading PCIe sysfs attributes on an ASUS TUF GAMING X870-PLUS
WIFI. The same fault was seen on downstream OGC kernels
7.2.9-ogc3.1.fc44 and 7.2.15-ogc1.1.fc45. The latter logged:

  BUG: kernel NULL pointer dereference, address: 0000000000000030
  RIP: 0010:l0s_aspm_show+0x47/0x80
    dev_attr_show+0x1f/0x50
    sysfs_kf_seq_show+0xa8/0x110
    seq_read_iter+0x132/0x4c0

The faulting instruction reads at offset 0x30 with a NULL base
register, consistent with accessing the ASPM state through a NULL
link.

The visibility callback already hides the ASPM attributes when there
is no link state, but the read callback looks up the link again. In
the affected boot path, the read reached the callback with no link.
A diagnostic build saw this on AMD xHCI [1022:15b7] and HD Audio
[1022:15e3], both downstream of internal GPP bridge [1022:14dd].

Return -ENODEV from aspm_attr_show_common() when the link lookup
returns NULL. On Helix, the equivalent guard with diagnostic logging
ran on the downstream kernel and the host reached Game Mode on
three repeat cold boots without the oops or an InputPlumber restart.

ZhaoJinming previously proposed NULL checks for the ASPM and clock
power-management sysfs callbacks.

Link: https://lkml.iu.edu/hypermail/linux/kernel/2607.3/04295.html
Signed-off-by: Louis H. Poulin II <poulin.louis@student.oaklandcc.edu>
---
The diagnostic build runs on the affected daily-use Helix system.
The hardware results above are from the downstream OGC build.

The RFC builds and boots in an isolated x86-64 QEMU VM on mainline.
A synthetic KUnit fixture passed for both missing and present links.
The same fixture without the guard faulted at address 0x30 in
aspm_attr_show_common(). This exercises the callback, while the
physical trigger and the cause of the visibility mismatch remain
unverified on mainline. The QEMU test called the callback directly
and did not exercise the real sysfs read path.

In the pinned mainline tree, pcie_aspm_exit_link_state() frees the
parent's link state only when removing link->downstream (function 0).
Could removing function 0 leave an ASPM attribute visible on a
surviving sibling function? The Helix traces do not establish that
this removal path ran.

The guard handles a NULL lookup. The show callback reads the link
without aspm_lock, so the guard alone does not establish safe link
lifetime during concurrent removal. clkpm_show() and the store
callbacks also dereference the link. Should the fix address group
visibility and lifetime, or should those callbacks get separate
guards if their paths can also be reached without a link?

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

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index b039b57..f78da8b 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1716,6 +1716,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);
 }


base-commit: 05441d697f7f9a927264f4058cbbc6a03dfcac9d

Re: [RFC PATCH] PCI/ASPM: Handle missing link state in sysfs reads

[sashiko-bot]

> From: Louis H. Poulin II <poulin.louis@student.oaklandcc.edu>
> 
> InputPlumber can hit a NULL pointer dereference at l0s_aspm_show()
> while reading PCIe sysfs attributes on an ASUS TUF GAMING X870-PLUS
> WIFI. The same fault was seen on downstream OGC kernels
> 7.2.9-ogc3.1.fc44 and 7.2.15-ogc1.1.fc45. The latter logged:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000030
>   RIP: 0010:l0s_aspm_show+0x47/0x80
>     dev_attr_show+0x1f/0x50
>     sysfs_kf_seq_show+0xa8/0x110
>     seq_read_iter+0x132/0x4c0
> 
> The faulting instruction reads at offset 0x30 with a NULL base
> register, consistent with accessing the ASPM state through a NULL
> [ ... ]
> ZhaoJinming previously proposed NULL checks for the ASPM and clock
> power-management sysfs callbacks.
> 
> Link: https://lkml.iu.edu/hypermail/linux/kernel/2607.3/04295.html
> Signed-off-by: Louis H. Poulin II <poulin.louis@student.oaklandcc.edu>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/CAGQButuh2zJ9dvNc4ioqaSh32EGChc9Z3HORr9DwnNd90=zgCg@mail.gmail.com?part=1