Re: [PATCH 06/15] PCI/TSM: Add device evidence support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL check for `pdev->tsm->evidence` in `pci_tsm_refresh_evidence()` leads to a NULL pointer dereference.
- [Medium] `pci_tsm_evidence_read_begin()` incorrectly masks wait interrupt errors, returning `-ENXIO` instead of the original error code.
--

commit 59da43773186eeb0cfa223261094a555b176c018
Author: Dan Williams <djbw@kernel.org>

PCI/TSM: Add device evidence support

This commit registers the PCI TSM framework with the device-evidence
netlink ABI. It introduces a struct device_evidence context for TSMs to own
SPDM sessions and handle evidence refresh requests.

> diff --git a/drivers/pci/tsm/evidence.c b/drivers/pci/tsm/evidence.c
> new file mode 100644
> index 0000000000000..ffb08208f0c0d
> --- /dev/null
> +++ b/drivers/pci/tsm/evidence.c
[ ... ]
> +static struct device_evidence *pci_tsm_evidence_read_begin(struct device *dev)
> +{
[ ... ]
> +	/* Hold the evidence stable against conflicting refresh updates */
> +	evidence = pdev->tsm->evidence;
> +	rc = down_read_interruptible(&evidence->lock);
> +	if (rc)
> +		goto err;
> +
> +	return evidence;
> +err:
> +	up_read(&pci_tsm_rwsem);
> +	return ERR_PTR(-ENXIO);

[Severity: Medium]
Does this error path intentionally mask the error code from
down_read_interruptible()?

If the wait is interrupted, rc is populated with -EINTR, but the err label
unconditionally returns -ENXIO. Could this lead to userspace receiving a
misleading 'No such device or address' error instead of a retryable
interrupt signal?

[ ... ]
> +static int pci_tsm_refresh_evidence(struct device *dev, const void *nonce,
> +				    size_t nonce_len)
> +{
[ ... ]
> +	/* Sync against pci_tsm_evidence_read_begin */
> +	evidence = pdev->tsm->evidence;
> +	ACQUIRE(rwsem_write_kill, elock)(&evidence->lock);

[Severity: High]
Is it possible for pdev->tsm->evidence to be NULL here?

Looking at the evidence_available() helper earlier in this file, it appears
structurally valid for a TSM to exist without an associated evidence context.

If evidence is NULL, would passing &evidence->lock into ACQUIRE() cause a
NULL pointer dereference?

> +	if ((rc = ACQUIRE_ERR(rwsem_write_kill, &elock)))
> +		return rc;
> +
> +	return ops->refresh_evidence(pdev->tsm, nonce, nonce_len);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260705220819.2472765-1-djbw@kernel.org?part=6