Re: [PATCH v2 16/19] samples/devsec: Introduce a "Device Security TSM" sample driver

["Lai, Yi" <yi1.lai@intel.com>]

On Mon, Mar 02, 2026 at 04:02:04PM -0800, Dan Williams wrote:
> There are 2 sides to a TEE Security Manager (TSM), the 'link' TSM, and the
> 'devsec' TSM. The 'link' TSM, outside the TEE, establishes physical link
> confidentiality and integerity, and a secure session for transporting
> commands the manage the security state of devices. The 'devsec' TSM, within
> the TEE, issues requests for confidential devices to lock their
> configuration and transition to secure operation.
> 
> Implement a sample implementation of a 'devsec' TSM. This leverages the PCI
> core's ability to register multiple TSMs at a time to load a sample
> devsec_tsm module alongside the existing devsec_link_tsm module. When both
> are loaded the TSM personality is selected by choosing to 'connect' vs
> 'lock' the device.
> 
> Drivers like tdx_guest, sev_guest, or arm-cca-guest are examples of "Device
> Security TSM" drivers.
> 
> A devsec_pci driver is included to test the device_cc_probe() helper for
> drivers that need to coordinate some configuration before 'lock' and
> 'accept'.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  samples/devsec/Makefile |   6 ++
>  samples/devsec/pci.c    |  39 +++++++++++++
>  samples/devsec/tsm.c    | 124 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 169 insertions(+)
>  create mode 100644 samples/devsec/pci.c
>  create mode 100644 samples/devsec/tsm.c
> 
> diff --git a/samples/devsec/Makefile b/samples/devsec/Makefile
> index da122eb8d23d..0c52448a629f 100644
> --- a/samples/devsec/Makefile
> +++ b/samples/devsec/Makefile
> @@ -8,3 +8,9 @@ devsec_bus-y := bus.o
>  
>  obj-$(CONFIG_SAMPLE_DEVSEC) += devsec_link_tsm.o
>  devsec_link_tsm-y := link_tsm.o
> +
> +obj-$(CONFIG_SAMPLE_DEVSEC) += devsec_tsm.o
> +devsec_tsm-y := tsm.o
> +
> +obj-$(CONFIG_SAMPLE_DEVSEC) += devsec_pci.o
> +devsec_pci-y := pci.o
> diff --git a/samples/devsec/pci.c b/samples/devsec/pci.c
> new file mode 100644
> index 000000000000..50519be412ed
> --- /dev/null
> +++ b/samples/devsec/pci.c
> @@ -0,0 +1,39 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2024 - 2026 Intel Corporation */
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +
> +static int devsec_pci_probe(struct pci_dev *pdev,
> +			    const struct pci_device_id *id)
> +{
> +	void __iomem *base;
> +	int rc;
> +
> +	rc = pcim_enable_device(pdev);
> +	if (rc)
> +		return dev_err_probe(&pdev->dev, rc, "enable failed\n");
> +
> +	base = pcim_iomap_region(pdev, 0, KBUILD_MODNAME);
> +	if (IS_ERR(base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(base),
> +				     "iomap failed\n");
> +
> +	dev_dbg(&pdev->dev, "attach\n");
> +	return 0;
> +}
> +
> +static const struct pci_device_id devsec_pci_ids[] = {
> +	{ PCI_DEVICE(0x8086, 0xffff), .override_only = 1, },
> +	{ }
> +};
> +
> +static struct pci_driver devsec_pci_driver = {
> +	.name = "devsec_pci",
> +	.probe = devsec_pci_probe,
> +	.id_table = devsec_pci_ids,
> +};
> +
> +module_pci_driver(devsec_pci_driver);
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Device Security Sample Infrastructure: Secure PCI Driver");
> diff --git a/samples/devsec/tsm.c b/samples/devsec/tsm.c
> new file mode 100644
> index 000000000000..46dbe668945a
> --- /dev/null
> +++ b/samples/devsec/tsm.c
> @@ -0,0 +1,124 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (C) 2024 - 2026 Intel Corporation */
> +
> +#define dev_fmt(fmt) "devsec: " fmt
> +#include <linux/device/faux.h>
> +#include <linux/pci-tsm.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/tsm.h>
> +#include "devsec.h"
> +
> +struct devsec_dev_data {
> +	struct pci_tsm_devsec pci;
> +};
> +
> +static struct devsec_dev_data *to_devsec_data(struct pci_tsm *tsm)
> +{
> +	return container_of(tsm, struct devsec_dev_data, pci.base_tsm);
> +}
> +
> +static struct pci_tsm *devsec_tsm_lock(struct tsm_dev *tsm_dev, struct pci_dev *pdev)
> +{
> +	int rc;
> +
> +	struct devsec_dev_data *devsec_data __free(kfree) =
> +		kzalloc(sizeof(*devsec_data), GFP_KERNEL);
> +	if (!devsec_data)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rc = pci_tsm_devsec_constructor(pdev, &devsec_data->pci, tsm_dev);
> +	if (rc)
> +		return ERR_PTR(rc);
> +
> +	return &no_free_ptr(devsec_data)->pci.base_tsm;
> +}
> +
> +static void devsec_tsm_unlock(struct pci_tsm *tsm)
> +{
> +	struct devsec_dev_data *devsec_data = to_devsec_data(tsm);
> +	struct pci_tsm_devsec *devsec_tsm = to_pci_tsm_devsec(tsm);
> +
> +	pci_tsm_mmio_teardown(devsec_tsm->mmio);
> +	kfree(devsec_tsm->mmio);
> +	kfree(devsec_data);
> +}
> +

Hi Dan,

While validating devsec mode transitions, I hit a reproducible crash in the
sample devsec driver.

Reproducer:
1. lock with devsec tsm
2. unlock

Observed: NULL pointer dereference in the MMIO teardown path

Expected: unlock from LOCKED should return to UNLOCKED safely.

My understanding is that this is a sample driver implementation bug - missing
NULL guard before MMIO teardown.

A follow-up question: do you prefer current design and each device
security TSM driver is responsible for MMIO check, or should tsm/core
adds a NULL guard to avoid potential crash?

Regards,
Yi Lai

> +static int devsec_tsm_accept(struct pci_dev *pdev)
> +{
> +	struct pci_tsm_devsec *devsec_tsm = to_pci_tsm_devsec(pdev->tsm);
> +	int rc;
> +
> +	struct pci_tsm_mmio *mmio __free(kfree) =
> +		kzalloc(struct_size(mmio, mmio, PCI_NUM_RESOURCES), GFP_KERNEL);
> +	if (!mmio)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Typically this range request would come from the TDISP Interface
> +	 * Report. For this sample, just request all BARs be marked encrypted
> +	 */
> +	for (int i = 0; i < PCI_NUM_RESOURCES; i++) {
> +		struct resource *res = pci_tsm_mmio_resource(mmio, mmio->nr);
> +
> +		if (pci_resource_len(pdev, i) == 0 ||
> +		    !(pci_resource_flags(pdev, i) & IORESOURCE_MEM))
> +			continue;
> +		res->start = pci_resource_start(pdev, i);
> +		res->end = pci_resource_end(pdev, i);
> +		mmio->nr++;
> +	}
> +
> +	rc = pci_tsm_mmio_setup(pdev, mmio);
> +	if (rc)
> +		return rc;
> +	devsec_tsm->mmio = no_free_ptr(mmio);
> +	return 0;
> +}
> +
> +static struct pci_tsm_ops devsec_pci_ops = {
> +	.lock = devsec_tsm_lock,
> +	.unlock = devsec_tsm_unlock,
> +	.accept = devsec_tsm_accept,
> +};
> +
> +static void devsec_tsm_remove(void *tsm_dev)
> +{
> +	tsm_unregister(tsm_dev);
> +}
> +
> +static int devsec_tsm_probe(struct faux_device *fdev)
> +{
> +	struct tsm_dev *tsm_dev;
> +
> +	tsm_dev = tsm_register(&fdev->dev, &devsec_pci_ops);
> +	if (IS_ERR(tsm_dev))
> +		return PTR_ERR(tsm_dev);
> +
> +	return devm_add_action_or_reset(&fdev->dev, devsec_tsm_remove,
> +					tsm_dev);
> +}
> +
> +static struct faux_device *devsec_tsm;
> +
> +static const struct faux_device_ops devsec_device_ops = {
> +	.probe = devsec_tsm_probe,
> +};
> +
> +static int __init devsec_tsm_init(void)
> +{
> +	devsec_tsm = faux_device_create("devsec_tsm", NULL, &devsec_device_ops);
> +	if (!devsec_tsm)
> +		return -ENOMEM;
> +	return 0;
> +}
> +module_init(devsec_tsm_init);
> +
> +static void __exit devsec_tsm_exit(void)
> +{
> +	faux_device_destroy(devsec_tsm);
> +}
> +module_exit(devsec_tsm_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Device Security Sample Infrastructure: Device Security TSM Driver");
> -- 
> 2.52.0
>