Re: [PATCH v7 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT

[Alexey Kardashevskiy <aik@amd.com>]

On 20/10/23 12:17, Dan Williams wrote:
> The sevguest driver was a first mover in the confidential computing
> space. As a first mover that afforded some leeway to build the driver
> without concern for common infrastructure.
> 
> Now that sevguest is no longer a singleton [1] the common operation of
> building and transmitting attestation report blobs can / should be made
> common. In this model the so called "TSM-provider" implementations can
> share a common envelope ABI even if the contents of that envelope remain
> vendor-specific. When / if the industry agrees on an attestation record
> format, that definition can also fit in the same ABI. In the meantime
> the kernel's maintenance burden is reduced and collaboration on the
> commons is increased.
> 
> Convert sevguest to use CONFIG_TSM_REPORTS to retrieve the data that
> the SNP_GET_EXT_REPORT ioctl produces. An example flow follows for
> retrieving the report blob via the TSM interface utility,
> assuming no nonce and VMPL==2:
> 
>      report=/sys/kernel/config/tsm/report/report0
>      mkdir $report
>      echo 2 > $report/privlevel
>      dd if=/dev/urandom bs=64 count=1 > $report/inblob
>      hexdump -C $report/outblob # SNP report
>      hexdump -C $report/auxblob # cert_table
>      rmdir $report
> 
> Given that the platform implementation is free to return empty
> certificate data if none is available it lets configfs-tsm be simplified
> as it only needs to worry about wrapping SNP_GET_EXT_REPORT, and leave
> SNP_GET_REPORT alone.
> 
> The old ioctls can be lazily deprecated, the main motivation of this
> effort is to stop the proliferation of new ioctls, and to increase
> cross-vendor collaboration.
> 
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Dionna Glaze <dionnaglaze@google.com>
> Cc: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
> Tested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Tested-by: Alexey Kardashevskiy <aik@amd.com>

Still works :)

Reviewed-by: Alexey Kardashevskiy <aik@amd.com>

Thanks,

> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>   drivers/virt/coco/sev-guest/Kconfig     |    1
>   drivers/virt/coco/sev-guest/sev-guest.c |  135 +++++++++++++++++++++++++++++++
>   include/uapi/linux/psp-sev.h            |    1
>   include/uapi/linux/sev-guest.h          |    4 +
>   4 files changed, 140 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/virt/coco/sev-guest/Kconfig b/drivers/virt/coco/sev-guest/Kconfig
> index da2d7ca531f0..1cffc72c41cb 100644
> --- a/drivers/virt/coco/sev-guest/Kconfig
> +++ b/drivers/virt/coco/sev-guest/Kconfig
> @@ -5,6 +5,7 @@ config SEV_GUEST
>   	select CRYPTO
>   	select CRYPTO_AEAD2
>   	select CRYPTO_GCM
> +	select TSM_REPORTS
>   	help
>   	  SEV-SNP firmware provides the guest a mechanism to communicate with
>   	  the PSP without risk from a malicious hypervisor who wishes to read,
> diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
> index e5f8f115f4af..bc564adcf499 100644
> --- a/drivers/virt/coco/sev-guest/sev-guest.c
> +++ b/drivers/virt/coco/sev-guest/sev-guest.c
> @@ -16,10 +16,13 @@
>   #include <linux/miscdevice.h>
>   #include <linux/set_memory.h>
>   #include <linux/fs.h>
> +#include <linux/tsm.h>
>   #include <crypto/aead.h>
>   #include <linux/scatterlist.h>
>   #include <linux/psp-sev.h>
>   #include <linux/sockptr.h>
> +#include <linux/cleanup.h>
> +#include <linux/uuid.h>
>   #include <uapi/linux/sev-guest.h>
>   #include <uapi/linux/psp-sev.h>
>   
> @@ -768,6 +771,130 @@ static u8 *get_vmpck(int id, struct snp_secrets_page_layout *layout, u32 **seqno
>   	return key;
>   }
>   
> +struct snp_msg_report_resp_hdr {
> +	u32 status;
> +	u32 report_size;
> +	u8 rsvd[24];
> +};
> +
> +struct snp_msg_cert_entry {
> +	guid_t guid;
> +	u32 offset;
> +	u32 length;
> +};
> +
> +static int sev_report_new(struct tsm_report *report, void *data)
> +{
> +	struct snp_msg_cert_entry *cert_table;
> +	struct tsm_desc *desc = &report->desc;
> +	struct snp_guest_dev *snp_dev = data;
> +	struct snp_msg_report_resp_hdr hdr;
> +	const u32 report_size = SZ_4K;
> +	const u32 ext_size = SEV_FW_BLOB_MAX_SIZE;
> +	u32 certs_size, i, size = report_size + ext_size;
> +	int ret;
> +
> +	if (desc->inblob_len != SNP_REPORT_USER_DATA_SIZE)
> +		return -EINVAL;
> +
> +	void *buf __free(kvfree) = kvzalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	guard(mutex)(&snp_cmd_mutex);
> +
> +	/* Check if the VMPCK is not empty */
> +	if (is_vmpck_empty(snp_dev)) {
> +		dev_err_ratelimited(snp_dev->dev, "VMPCK is disabled\n");
> +		return -ENOTTY;
> +	}
> +
> +	cert_table = buf + report_size;
> +	struct snp_ext_report_req ext_req = {
> +		.data = { .vmpl = desc->privlevel },
> +		.certs_address = (__u64)cert_table,
> +		.certs_len = ext_size,
> +	};
> +	memcpy(&ext_req.data.user_data, desc->inblob, desc->inblob_len);
> +
> +	struct snp_guest_request_ioctl input = {
> +		.msg_version = 1,
> +		.req_data = (__u64)&ext_req,
> +		.resp_data = (__u64)buf,
> +		.exitinfo2 = 0xff,
> +	};
> +	struct snp_req_resp io = {
> +		.req_data = KERNEL_SOCKPTR(&ext_req),
> +		.resp_data = KERNEL_SOCKPTR(buf),
> +	};
> +
> +	ret = get_ext_report(snp_dev, &input, &io);
> +	if (ret)
> +		return ret;
> +
> +	memcpy(&hdr, buf, sizeof(hdr));
> +	if (hdr.status == SEV_RET_INVALID_PARAM)
> +		return -EINVAL;
> +	if (hdr.status == SEV_RET_INVALID_KEY)
> +		return -EINVAL;
> +	if (hdr.status)
> +		return -ENXIO;
> +	if ((hdr.report_size + sizeof(hdr)) > report_size)
> +		return -ENOMEM;
> +
> +	void *rbuf __free(kvfree) = kvzalloc(hdr.report_size, GFP_KERNEL);
> +	if (!rbuf)
> +		return -ENOMEM;
> +
> +	memcpy(rbuf, buf + sizeof(hdr), hdr.report_size);
> +	report->outblob = no_free_ptr(rbuf);
> +	report->outblob_len = hdr.report_size;
> +
> +	certs_size = 0;
> +	for (i = 0; i < ext_size / sizeof(struct snp_msg_cert_entry); i++) {
> +		struct snp_msg_cert_entry *ent = &cert_table[i];
> +
> +		if (guid_is_null(&ent->guid) && !ent->offset && !ent->length)
> +			break;
> +		certs_size = max(certs_size, ent->offset + ent->length);
> +	}
> +
> +	/* Suspicious that the response populated entries without populating size */
> +	if (!certs_size && i)
> +		dev_warn_ratelimited(snp_dev->dev, "certificate slots conveyed without size\n");
> +
> +	/* No certs to report */
> +	if (!certs_size)
> +		return 0;
> +
> +	/* Suspicious that the certificate blob size contract was violated
> +	 */
> +	if (certs_size > ext_size) {
> +		dev_warn_ratelimited(snp_dev->dev, "certificate data truncated\n");
> +		certs_size = ext_size;
> +	}
> +
> +	void *cbuf __free(kvfree) = kvzalloc(certs_size, GFP_KERNEL);
> +	if (!cbuf)
> +		return -ENOMEM;
> +
> +	memcpy(cbuf, cert_table, certs_size);
> +	report->auxblob = no_free_ptr(cbuf);
> +	report->auxblob_len = certs_size;
> +
> +	return 0;
> +}
> +
> +static const struct tsm_ops sev_tsm_ops = {
> +	.name = KBUILD_MODNAME,
> +	.report_new = sev_report_new,
> +};
> +
> +static void unregister_sev_tsm(void *data)
> +{
> +	tsm_unregister(&sev_tsm_ops);
> +}
> +
>   static int __init sev_guest_probe(struct platform_device *pdev)
>   {
>   	struct snp_secrets_page_layout *layout;
> @@ -841,6 +968,14 @@ static int __init sev_guest_probe(struct platform_device *pdev)
>   	snp_dev->input.resp_gpa = __pa(snp_dev->response);
>   	snp_dev->input.data_gpa = __pa(snp_dev->certs_data);
>   
> +	ret = tsm_register(&sev_tsm_ops, snp_dev, &tsm_report_extra_type);
> +	if (ret)
> +		goto e_free_cert_data;
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, unregister_sev_tsm, NULL);
> +	if (ret)
> +		goto e_free_cert_data;
> +
>   	ret =  misc_register(misc);
>   	if (ret)
>   		goto e_free_cert_data;
> diff --git a/include/uapi/linux/psp-sev.h b/include/uapi/linux/psp-sev.h
> index 1c9da485318f..b44ba7dcdefc 100644
> --- a/include/uapi/linux/psp-sev.h
> +++ b/include/uapi/linux/psp-sev.h
> @@ -68,6 +68,7 @@ typedef enum {
>   	SEV_RET_INVALID_PARAM,
>   	SEV_RET_RESOURCE_LIMIT,
>   	SEV_RET_SECURE_DATA_INVALID,
> +	SEV_RET_INVALID_KEY = 0x27,
>   	SEV_RET_MAX,
>   } sev_ret_code;
>   
> diff --git a/include/uapi/linux/sev-guest.h b/include/uapi/linux/sev-guest.h
> index 2aa39112cf8d..154a87a1eca9 100644
> --- a/include/uapi/linux/sev-guest.h
> +++ b/include/uapi/linux/sev-guest.h
> @@ -14,9 +14,11 @@
>   
>   #include <linux/types.h>
>   
> +#define SNP_REPORT_USER_DATA_SIZE 64
> +
>   struct snp_report_req {
>   	/* user data that should be included in the report */
> -	__u8 user_data[64];
> +	__u8 user_data[SNP_REPORT_USER_DATA_SIZE];
>   
>   	/* The vmpl level to be included in the report */
>   	__u32 vmpl;
> 

-- 
Alexey