Re: [PATCH v2 10/21] PCI/TSM: Rename pf0 to host

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use-After-Free and freed mutex unlock in `__pci_tsm_disconnect()` for hosts on PF > 0.
- [High] Disconnecting any TSM host blindly destroys independent TSM contexts of sibling functions in the same PCI slot.
--

commit 8bf82a17d0ecf2259067413b6fb0b2c2e2bef4d9
Author: Alistair Francis <alistair.francis@wdc.com>

PCI/TSM: Rename pf0 to host

Rename pci_tsm_pf0 to pci_tsm_host (and rename variables and function
names from pf0 to host) as part of converting pci_tsm_host
to be any device that knows how to speak any of CMA, IDE, or
TDISP.

> diff --git a/drivers/pci/tsm.c b/drivers/pci/tsm.c
> --- a/drivers/pci/tsm.c
> +++ b/drivers/pci/tsm.c

[ ... ]

> @@ -441,19 +441,19 @@ static void __pci_tsm_disconnect(struct pci_dev *pdev)
>  	/*
>  	 * disconnect() is uninterruptible as it may be called for device
>  	 * teardown
>  	 */
> -	guard(mutex)(&tsm_pf0->lock);
> +	guard(mutex)(&tsm_host->lock);
>  	pci_tsm_walk_fns_reverse(pdev, remove_fn, NULL);

[Severity: High]
Can this lead to a use-after-free and memory corruption when a TSM host
is on a Physical Function > 0?

When disconnecting a host on PF > 0 (such as a CMA host on PF 1),
pci_tsm_walk_fns_reverse() iterates over the slot. The walker skips
the callback only for i == 0. When i > 0, it applies the teardown to the
function itself:

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

This causes remove_fn() to be called on PF 1 itself, which frees the
tsm_host object via pci_tsm_host_destructor() and kfree(). When
__pci_tsm_disconnect() returns, the guard attempts to unlock the destroyed
and freed mutex.

[Severity: High]
Does this unintentionally destroy the TSM contexts of independent
sibling functions?

The call to pci_tsm_walk_fns_reverse() unconditionally applies remove_fn()
to all functions in the slot. If multiple independent TSM hosts exist
within the same PCI slot (for example, PF 0 as a TDISP host and PF 1 as a
CMA host), disconnecting one host will cause the walker to execute
remove_fn() on the other siblings.

drivers/pci/tsm.c:remove_fn() {
    tsm_remove(pdev->tsm);
    link_sysfs_disable(pdev);
    return 0;
}

Since remove_fn() does not verify if the function's TSM context actually
belongs to the specific host being disconnected (the data argument is NULL),
could disconnecting PF 1 silently wipe out the entirely separate host
context of PF 0?

>  	ops->disconnect(pdev);
>  }

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