linux-kernel - Re: [PATCH v2 08/11] coco: guest: arm64: Add support for fetching and verifying device info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251120175432.00004af8@huawei.com>
Date: Thu, 20 Nov 2025 17:54:32 +0000
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: "Aneesh Kumar K.V (Arm)" <aneesh.kumar@...nel.org>
CC: <linux-coco@...ts.linux.dev>, <kvmarm@...ts.linux.dev>,
	<linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<dan.j.williams@...el.com>, <aik@....com>, <lukas@...ner.de>, Samuel Ortiz
	<sameo@...osinc.com>, Xu Yilun <yilun.xu@...ux.intel.com>, Jason Gunthorpe
	<jgg@...pe.ca>, Suzuki K Poulose <Suzuki.Poulose@....com>, Steven Price
	<steven.price@....com>, Bjorn Helgaas <helgaas@...nel.org>, Catalin Marinas
	<catalin.marinas@....com>, Marc Zyngier <maz@...nel.org>, Will Deacon
	<will@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>
Subject: Re: [PATCH v2 08/11] coco: guest: arm64: Add support for fetching
 and verifying device info

On Mon, 17 Nov 2025 19:30:04 +0530
"Aneesh Kumar K.V (Arm)" <aneesh.kumar@...nel.org> wrote:

> RSI_RDEV_GET_INFO returns different digest hash values, which can be
> compared with host cached values to ensure the host didn't tamper with
> the cached data.
> 
> Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@...nel.org>

> diff --git a/drivers/virt/coco/arm-cca-guest/rsi-da.c b/drivers/virt/coco/arm-cca-guest/rsi-da.c
> index c70fb7dd4838..c6b92f4ae9c5 100644
> --- a/drivers/virt/coco/arm-cca-guest/rsi-da.c
> +++ b/drivers/virt/coco/arm-cca-guest/rsi-da.c

> +
> +static int verify_digests(struct cca_guest_dsc *dsc)
> +{
> +	u8 digest[SHA512_DIGEST_SIZE];
> +	size_t digest_size;
> +	void (*digest_func)(const u8 *data, size_t len, u8 *out);
> +
> +	struct pci_dev *pdev = dsc->pci.base_tsm.pdev;
> +	struct {
> +		uint8_t *report;
> +		size_t size;
> +		uint8_t *digest;
> +	} reports[] = {
> +		{
> +			dsc->interface_report,
> +			dsc->interface_report_size,
> +			dsc->dev_info.report_digest
> +		},
> +		{
> +			dsc->certificate,
> +			dsc->certificate_size,
> +			dsc->dev_info.cert_digest
> +		},
> +		{
> +			dsc->measurements,
> +			dsc->measurements_size,
> +			dsc->dev_info.meas_digest
> +		}
> +	};
> +
> +	switch (dsc->dev_info.hash_algo) {
> +	case RSI_HASH_SHA_256:
> +		digest_func = sha256;
> +		digest_size = SHA256_DIGEST_SIZE;
> +		break;
> +
> +	case RSI_HASH_SHA_512:
> +		digest_func = sha512;
> +		digest_size = SHA512_DIGEST_SIZE;
> +		break;
> +	default:
> +		pci_err(pdev, "Unknown realm hash algorithm!\n");
> +		return -EINVAL;
> +	}
> +
> +	for (int i = 0; i < ARRAY_SIZE(reports); i++) {
> +

I'd drop this blank line as it doesn't for me at least enhance readability
and I don't recall it being particularly common to have one here
in kernel code.

> +		digest_func(reports[i].report, reports[i].size, digest);
> +		if (memcmp(reports[i].digest, digest, digest_size)) {
> +			pci_err(pdev, "Invalid digest\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	pci_dbg(pdev, "Successfully verified the digests\n");
> +	return 0;
> +}
> +
> +int cca_device_verify_and_accept(struct pci_dev *pdev)
> +{
> +	int ret;
> +	int vdev_id = rsi_vdev_id(pdev);
> +	struct rsi_vdevice_info *dev_info;
> +	struct cca_guest_dsc *dsc = to_cca_guest_dsc(pdev);
> +
> +	/* Now make a host call to copy the interface report to guest. */
> +	ret = rhi_read_cached_object(vdev_id, RHI_DA_OBJECT_INTERFACE_REPORT,
> +				     &dsc->interface_report, &dsc->interface_report_size);
> +	if (ret) {
> +		pci_err(pdev, "failed to get interface report from the host (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = rhi_read_cached_object(vdev_id, RHI_DA_OBJECT_CERTIFICATE,
> +				     &dsc->certificate, &dsc->certificate_size);
> +	if (ret) {
> +		pci_err(pdev, "failed to get device certificate from the host (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = rhi_read_cached_object(vdev_id, RHI_DA_OBJECT_MEASUREMENT,
> +				     &dsc->measurements, &dsc->measurements_size);
> +	if (ret) {
> +		pci_err(pdev, "failed to get device certificate from the host (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	/* RMM expects sizeof(*dev_info) = 512 bytes aligned address */
> +	BUILD_BUG_ON(sizeof(*dev_info) != 512);
> +	dev_info = kmalloc(sizeof(*dev_info), GFP_KERNEL);
> +	if (!dev_info)
> +		return -ENOMEM;
> +
> +	if (rsi_vdev_get_info(vdev_id, virt_to_phys(dev_info))) {
> +		pci_err(pdev, "failed to get device digests (%d)\n", ret);
> +		kfree(dev_info);

Could use __free for that and not worry that we free it a little later than
last place we need it.

> +		return -EIO;
> +	}
> +
> +	dsc->dev_info.cert_id       = dev_info->cert_id;
> +	dsc->dev_info.hash_algo     = dev_info->hash_algo;
> +	dsc->dev_info.lock_nonce    = dev_info->lock_nonce;
> +	dsc->dev_info.meas_nonce    = dev_info->meas_nonce;
> +	dsc->dev_info.report_nonce  = dev_info->report_nonce;
> +	memcpy(dsc->dev_info.cert_digest, dev_info->cert_digest, SHA512_DIGEST_SIZE);
> +	memcpy(dsc->dev_info.meas_digest, dev_info->meas_digest, SHA512_DIGEST_SIZE);
> +	memcpy(dsc->dev_info.report_digest, dev_info->report_digest, SHA512_DIGEST_SIZE);

So copy everything other than flags.  Any reason why not flags?
> +
> +	kfree(dev_info);
> +	/*
> +	 * Verify that the digests of the provided reports match with the
> +	 * digests from RMM
> +	 */
> +	ret = verify_digests(dsc);
> +	if (ret) {
> +		pci_err(pdev, "device digest validation failed (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = cca_apply_interface_report_mappings(pdev, true);
> +	if (ret) {
> +		pci_err(pdev, "failed to validate the interface report\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}