Re: [PATCH v4 2/2] thunderbolt: Add device links for Apple T2 NHI

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lockless traversal of the PCI bus devices list exposes the driver to a use-after-free race during concurrent hotplug or sysfs device removal.
--

--- Patch [2]: [PATCH v4 2/2] thunderbolt: Add device links for Apple T2 NHI ---
commit ee5a1e7f9e85d72f9c6e9327c898de2f33a51e3d
Author: Andre Eikmeyer <andre@negmaster.com>

thunderbolt: Add device links for Apple T2 NHI

This commit adds device links for the Icelake Thunderbolt NHI on Apple T2
Macs. It finds the root ports (TRP0, TRP1, etc.) in ACPI and creates device
links to ensure PCIe tunnels can be re-established after sleep.

> diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
> index c69c323e6952a..625451ff3ea08 100644
> --- a/drivers/thunderbolt/tb.c
> +++ b/drivers/thunderbolt/tb.c

[ ... ]

> @@ -3305,21 +3307,73 @@ static const struct tb_cm_ops tb_cm_ops = {
>  static bool tb_apple_add_links(struct tb_nhi *nhi)
>  {
>  	struct pci_dev *upstream, *pdev;
> -	bool ret;
> +	bool ret = false;
>  
>  	if (!x86_apple_machine)
>  		return false;

[ ... ]

>  	upstream = pci_upstream_bridge(nhi->pdev);
>  	while (upstream) {
>  		if (!pci_is_pcie(upstream))

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing this function
I noticed a pre-existing issue further down in tb_apple_add_links():

	for_each_pci_bridge(pdev, upstream->subordinate) {
		const struct device_link *link;

		if (!pci_is_pcie(pdev))
			continue;

Could traversing upstream->subordinate without holding the pci_bus_sem read
lock lead to list corruption or a use-after-free?

If a user concurrently writes to /sys/bus/pci/devices/.../remove for one of
the downstream ports while the Thunderbolt host controller is probing,
pci_remove_bus_device() could delete the device from the list via
list_del(&dev->bus_list) while this lockless traversal is happening.

The newly added T2 Mac path safely avoids this by using pci_get_slot(),
which handles the locking internally. Would it make sense to acquire
pci_bus_sem for the older architecture paths as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724164641.2239-1-atharvatiwarilinuxdev@gmail.com?part=2