Re: [PATCH v2 11/21] PCI/TSM: Support connecting to PCIe CMA devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Bypassing the function 0 check for CMA-capable devices breaks the subsystem's architectural assumptions and physical function iteration logic, leading to UAF when unlocking a freed mutex on disconnect.
- [High] Calling `pci_find_doe_mailbox()` inside `is_pci_tsm_host()` introduces a Use-After-Free vulnerability when iterating over PCI devices, as DOE mailboxes can be concurrently freed by device removal.
--

commit 4f85c4326e0a051322285638e0233e1946a251be
Author: Alistair Francis <alistair.francis@wdc.com>

PCI/TSM: Support connecting to PCIe CMA devices

In the next patch we are going to add a PCIe CMA TSM driver, as such we
need to ensure that is_pci_tsm_host() will allow us to connect to CMA
capable devices. These devices don't necessarily has DEVCAP_TEE or IDE
support.

> diff --git a/include/linux/pci-tsm.h b/include/linux/pci-tsm.h
> --- a/include/linux/pci-tsm.h
> +++ b/include/linux/pci-tsm.h
[ ... ]
> @@ -142,6 +143,14 @@ static inline bool is_pci_tsm_host(struct pci_dev *pdev)
>  	if (pdev->is_virtfn)
>  		return false;
>  
> +	/*
> +	 * Report capable if CMA is supported, which can be supported on any PCIe
> +	 * device.
> +	 */
> +	if (pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
> +				 PCI_DOE_FEATURE_CMA))
> +		return true;

[Severity: High]
Does returning true for devices where the function is not 0 cause a use
after free during device disconnect?

If a CMA-capable device connects where its function number is greater
than 0, when the device is disconnected, __pci_tsm_disconnect() invokes
pci_tsm_walk_fns_reverse().

The reverse walk hardcodes the assumption that the host is function 0:

drivers/pci/tsm.c:pci_tsm_walk_fns_reverse() {
    ...
		/* on exit, caller will run @cb on function 0 */
		if (i > 0)
			cb(pf, data);
    ...
}

Because the host is a non-zero function, the walk mistakenly runs remove_fn
on the host itself, freeing the memory containing tsm_host.

drivers/pci/tsm.c:__pci_tsm_disconnect() {
	guard(mutex)(&tsm_host->lock);
	pci_tsm_walk_fns_reverse(pdev, remove_fn, NULL);
	ops->disconnect(pdev);
}

When __pci_tsm_disconnect() returns, does dropping the guard unlock a mutex
in the already freed memory?


[Severity: High]
Is it safe to call pci_find_doe_mailbox() here while iterating over PCI
devices without holding the device lock?

The TSM subsystem iterates over all PCI devices using for_each_pci_dev(),
repeatedly invoking is_pci_tsm_host(). While this holds a kref on the
device, pci_find_doe_mailbox() performs a lockless iteration over the xarray
without an RCU read lock:

drivers/pci/doe.c:pci_find_doe_mailbox() {
    ...
	xa_for_each(&pdev->doe_mbs, index, doe_mb)
		if (pci_doe_supports_feat(doe_mb, vendor, type))
			return doe_mb;
    ...
}

During concurrent device hot-removal, pci_destroy_dev() unconditionally
destroys mailboxes directly, without waiting for an RCU grace period:

drivers/pci/doe.c:pci_doe_destroy_mb() {
    ...
	kfree(doe_mb);
    ...
}

Can this lead to a use after free where pci_find_doe_mailbox() dereferences
mailboxes that have already been freed by a concurrent removal thread?

> +
>  	/*
>  	 * Allow for a Device Security Manager (DSM) associated with function0
>  	 * of an Endpoint to coordinate TDISP requests for other functions

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623045406.2589547-1-alistair.francis@wdc.com?part=11