Re: [PATCH 08/12] PCI/CMA: Authenticate devices on enumeration

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 28 Sep 2023 19:32:38 +0200
Lukas Wunner <lukas@wunner.de> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> Component Measurement and Authentication (CMA, PCIe r6.1 sec 6.31)
> allows for measurement and authentication of PCIe devices.  It is
> based on the Security Protocol and Data Model specification (SPDM,
> https://www.dmtf.org/dsp/DSP0274).
> 
> CMA-SPDM in turn forms the basis for Integrity and Data Encryption
> (IDE, PCIe r6.1 sec 6.33) because the key material used by IDE is
> exchanged over a CMA-SPDM session.
> 
> As a first step, authenticate CMA-capable devices on enumeration.
> A subsequent commit will expose the result in sysfs.
> 
> When allocating SPDM session state with spdm_create(), the maximum SPDM
> message length needs to be passed.  Make the PCI_DOE_MAX_LENGTH macro
> public and calculate the maximum payload length from it.
> 
> Credits:  Jonathan wrote a proof-of-concept of this CMA implementation.
> Lukas reworked it for upstream.  Wilfred contributed fixes for issues
> discovered during testing.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
Hi Lukas,

A few things inline. Biggest of which is making this one build
warning free by pulling forward the cma_capable flag from
patch 10.

>  
> diff --git a/drivers/pci/cma.c b/drivers/pci/cma.c
> new file mode 100644
> index 000000000000..06e5846325e3
> --- /dev/null
> +++ b/drivers/pci/cma.c


> +void pci_cma_init(struct pci_dev *pdev)
> +{
> +	struct pci_doe_mb *doe;
> +	int rc;
> +
> +	if (!pci_cma_keyring) {
> +		return;
> +	}
> +
> +	if (!pci_is_pcie(pdev))
> +		return;
> +
> +	doe = pci_find_doe_mailbox(pdev, PCI_VENDOR_ID_PCI_SIG,
> +				   PCI_DOE_PROTOCOL_CMA);
> +	if (!doe)
> +		return;
> +
> +	pdev->spdm_state = spdm_create(&pdev->dev, pci_doe_transport, doe,
> +				       PCI_DOE_MAX_PAYLOAD, pci_cma_keyring);
> +	if (!pdev->spdm_state) {
> +		return;
> +	}

Brackets not needed.

> +
> +	rc = spdm_authenticate(pdev->spdm_state);

Hanging rc?  There is a blob in patch 10 that uses it, but odd to keep it around
in meantime.  Perhaps just add the flag in this patch and set it even
though no one cares about it yet.


> +}
> +
> +void pci_cma_destroy(struct pci_dev *pdev)
> +{
> +	if (pdev->spdm_state)
> +		spdm_destroy(pdev->spdm_state);
> +}
> +
> +__init static int pci_cma_keyring_init(void)
> +{
> +	pci_cma_keyring = keyring_alloc(".cma", KUIDT_INIT(0), KGIDT_INIT(0),
> +					current_cred(),
> +					(KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +					KEY_USR_VIEW | KEY_USR_READ |
> +					KEY_USR_WRITE | KEY_USR_SEARCH,
> +					KEY_ALLOC_NOT_IN_QUOTA |
> +					KEY_ALLOC_SET_KEEP, NULL, NULL);
> +	if (IS_ERR(pci_cma_keyring)) {
> +		pr_err("Could not allocate keyring\n");
> +		return PTR_ERR(pci_cma_keyring);
> +	}
> +
> +	return 0;
> +}
> +arch_initcall(pci_cma_keyring_init);