Re: [PATCH 09/18] PCI/CMA: Add a PCI TSM CMA driver using SPDM

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] PCI_CMA unconditionally selects RSPDM, bypassing Rust architectural dependencies.
- [High] `pci_cma_tsm_probe()` manually initializes the TSM context and incorrectly sets `dsm_dev = pdev`, causing a guaranteed NULL pointer dereference for Virtual Functions (VFs).
- [High] Missing `bind`, `unbind`, and `guest_req` callbacks in `pci_cma_tsm_ops` lead to NULL function pointer dereferences in core TSM APIs.
- [High] Missing `pm_runtime_get_sync()` before accessing the DOE mailbox in `pci_cma_tsm_connect()`.
- [Low] The struct `pci_cma_devsec` is defined but never used.
--

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

PCI/CMA: Add a PCI TSM CMA driver using SPDM

This commit adds support for component measurement and authentication 
(CMA-SPDM) per PCIe r6.2 sec 6.31. It introduces a TSM driver that uses 
a PCI DOE mailbox as a transport for SPDM-based authentication.

> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 33c88432b7287..dcf4170381f24 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -124,6 +124,20 @@ config PCI_ATS
>  config PCI_IDE
>  	bool
>  
> +config PCI_CMA
> +	bool "Component Measurement and Authentication (CMA-SPDM)"
> +	select CRYPTO_ECDSA
> +	select CRYPTO_RSA
> +	select CRYPTO_SHA256
> +	select CRYPTO_SHA512
> +	select PCI_DOE
> +	select PCI_TSM
> +	select RSPDM

Does selecting RSPDM here bypass Rust architectural dependencies?

Since RSPDM selects RUST, using select here forces RUST on regardless of
its dependencies like HAVE_RUST and RUST_IS_AVAILABLE.

Can this cause unmet dependency warnings and build failures due to missing
compiler toolchains on architectures without Rust support?

> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> new file mode 100644
> index 0000000000000..998fde6366fb8
> --- /dev/null
> +++ b/drivers/pci/cma.c

[ ... ]

> +/**
> + * struct pci_cma_devsec - CMA SPDM devsec TSM context
> + * @spdm: SPDM session for this device
> + */
> +struct pci_cma_devsec {
> +	struct spdm_state *spdm;
> +};

This isn't a bug, but struct pci_cma_devsec appears to be defined but
never used anywhere in the driver.

> +static struct pci_tsm *pci_cma_tsm_probe(struct tsm_dev *tsm_dev,
> +				      struct pci_dev *pdev)
> +{
> +	struct pci_doe_mb *doe;
> +	struct pci_cma_tsm *cma;
> +
> +	doe = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
> +				   PCI_DOE_FEATURE_CMA);
> +	if (!doe)
> +		return NULL;
> +
> +	cma = kzalloc(sizeof(*cma), GFP_KERNEL);
> +	if (!cma)
> +		return NULL;
> +
> +	mutex_init(&cma->pf0.lock);
> +	cma->pf0.doe_mb = doe;
> +	cma->pf0.base_tsm.pdev = pdev;
> +	cma->pf0.base_tsm.dsm_dev = pdev;

Is it safe to bypass pci_tsm_pf0_constructor() and manually set
dsm_dev = pdev here?

During pci_tsm_connect(), the framework walks all dependent functions,
meaning pdev can be a Virtual Function (VF).

Later, when the core TSM subsystem calls to_pci_tsm_pf0(), it validates
the DSM device with is_pci_tsm_pf0(), which returns false for VFs.

This causes the helper to return NULL, leading to a NULL pointer dereference
when the core framework attempts to acquire the lock.

> +	cma->pf0.base_tsm.tsm_dev = tsm_dev;
> +
> +	cma->spdm = spdm_create(&pdev->dev, pci_doe_transport, doe,
> +				PCI_DOE_MAX_PAYLOAD, NULL);

[ ... ]

> +static int pci_cma_tsm_connect(struct pci_dev *pdev)
> +{
> +	struct pci_cma_tsm *cma = cma_tsm_from_tsm(pdev->tsm);
> +	int rc;
> +
> +	rc = spdm_authenticate(cma->spdm);

Does this sequence need a pm_runtime_get_sync() before attempting
authentication?

Since this is triggered via the sysfs connect attribute, it does not
automatically wake the device.

If the device is in a low-power state like D3hot, accessing its DOE mailbox
registers here could result in PCI aborts or return ~0.

> +	if (rc)
> +		return rc;
> +
> +	return 0;
> +}

[ ... ]

> +static const struct pci_tsm_ops pci_cma_tsm_ops = {
> +	.link_ops = {
> +		.probe		= pci_cma_tsm_probe,
> +		.remove		= pci_cma_tsm_remove,
> +		.connect	= pci_cma_tsm_connect,
> +		.disconnect	= pci_cma_tsm_disconnect,
> +	},
> +};

Are the missing bind, unbind, and guest_req callbacks intentional?

The core TSM framework APIs like pci_tsm_bind() and pci_tsm_unbind()
unconditionally call these function pointers without verifying if they
exist.

Could this cause an immediate kernel panic if userspace or VFIO attempts
to bind a CMA-authenticated device as a TDI?

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