2023-09-26 05:48:32

by Dan Williams

[permalink] [raw]
Subject: [PATCH v4 5/6] virt: sevguest: Add TSM_REPORTS support for SNP_{GET, GET_EXT}_REPORT

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
cat $report/certs
rmdir $report

Given that the platform implementation is free to return empty certificate data
if none is available it lets configfs-tsm be simplified if it only needs
to worry about one output format.

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.

Note, only compile-tested.

Link: http://lore.kernel.org/r/[email protected] [1]
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Dionna Glaze <[email protected]>
Cc: Brijesh Singh <[email protected]>
Cc: Jeremi Piotrowski <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
---
drivers/virt/coco/sev-guest/Kconfig | 1
drivers/virt/coco/sev-guest/sev-guest.c | 130 +++++++++++++++++++++++++++++++
2 files changed, 131 insertions(+)

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 c3c9e9ea691f..646feb433b1c 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -16,10 +16,12 @@
#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 <uapi/linux/sev-guest.h>
#include <uapi/linux/psp-sev.h>

@@ -759,6 +761,126 @@ 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];
+};
+#define SNP_REPORT_INVALID_PARAM 0x16
+#define SNP_REPORT_INVALID_KEY_SEL 0x27
+
+struct snp_msg_cert_entry {
+ unsigned char guid[16];
+ u32 offset;
+ u32 length;
+};
+
+static int sev_report_new(struct tsm_report *report, void *data)
+{
+ static const struct snp_msg_cert_entry zero_ent = { 0 };
+ struct tsm_desc *desc = &report->desc;
+ struct snp_guest_dev *snp_dev = data;
+ struct snp_msg_report_resp_hdr hdr;
+ const int report_size = SZ_4K;
+ const int ext_size = SZ_16K;
+ int ret, size = report_size + ext_size;
+ int certs_size, cert_count, i, offset;
+ u8 *certs_address;
+
+ if (desc->inblob_len != 64)
+ return -EINVAL;
+
+ void *buf __free(kvfree) = kvzalloc(size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ guard(mutex)(&snp_cmd_mutex);
+ certs_address = buf + report_size;
+ struct snp_ext_report_req ext_req = {
+ .data = { .vmpl = desc->privlevel },
+ .certs_address = (__u64)certs_address,
+ .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,
+ };
+ 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 == SNP_REPORT_INVALID_PARAM)
+ return -EINVAL;
+ if (hdr.status == SNP_REPORT_INVALID_KEY_SEL)
+ 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;
+
+ for (i = 0; i < ext_size / sizeof(struct snp_msg_cert_entry); i++) {
+ struct snp_msg_cert_entry *certs = buf + report_size;
+
+ if (memcmp(&certs[i], &zero_ent, sizeof(zero_ent)) == 0)
+ break;
+ certs_size += certs[i].length;
+ }
+ cert_count = i;
+
+ /* No certs to report */
+ if (cert_count == 0)
+ return 0;
+
+ /* sanity check that the entire certs table with metadata fits */
+ if ((cert_count + 1) * sizeof(zero_ent) + certs_size > ext_size)
+ return -ENXIO;
+
+ void *cbuf __free(kvfree) = kvzalloc(certs_size, GFP_KERNEL);
+ if (!cbuf)
+ return -ENOMEM;
+
+ /* Concatenate returned certs */
+ for (i = 0, offset = 0; i < cert_count; i++) {
+ struct snp_msg_cert_entry *certs = buf + report_size;
+
+ memcpy(cbuf + offset, certs_address + certs[i].offset, certs[i].length);
+ offset += certs[i].length;
+ }
+
+ report->certs = no_free_ptr(cbuf);
+ report->certs_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;
@@ -832,6 +954,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_ext_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;


2023-10-04 08:23:15

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v4 5/6] virt: sevguest: Add TSM_REPORTS support for SNP_{GET, GET_EXT}_REPORT

Hi Dan,

kernel test robot noticed the following build warnings:

url: https://github.com/intel-lab-lkp/linux/commits/Dan-Williams/virt-coco-Add-a-coco-Makefile-and-coco-Kconfig/20230926-121843
base: 6465e260f48790807eef06b583b38ca9789b6072
patch link: https://lore.kernel.org/r/169570184829.596431.15991881056638719011.stgit%40dwillia2-xfh.jf.intel.com
patch subject: [PATCH v4 5/6] virt: sevguest: Add TSM_REPORTS support for SNP_{GET, GET_EXT}_REPORT
config: x86_64-randconfig-161-20231002 (https://download.01.org/0day-ci/archive/20231003/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231003/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/

smatch warnings:
drivers/virt/coco/sev-guest/sev-guest.c:853 sev_report_new() error: uninitialized symbol 'certs_size'.

vim +/certs_size +853 drivers/virt/coco/sev-guest/sev-guest.c

80013405d5b7c6 Dan Williams 2023-09-25 778 static int sev_report_new(struct tsm_report *report, void *data)
80013405d5b7c6 Dan Williams 2023-09-25 779 {
80013405d5b7c6 Dan Williams 2023-09-25 780 static const struct snp_msg_cert_entry zero_ent = { 0 };
80013405d5b7c6 Dan Williams 2023-09-25 781 struct tsm_desc *desc = &report->desc;
80013405d5b7c6 Dan Williams 2023-09-25 782 struct snp_guest_dev *snp_dev = data;
80013405d5b7c6 Dan Williams 2023-09-25 783 struct snp_msg_report_resp_hdr hdr;
80013405d5b7c6 Dan Williams 2023-09-25 784 const int report_size = SZ_4K;
80013405d5b7c6 Dan Williams 2023-09-25 785 const int ext_size = SZ_16K;
80013405d5b7c6 Dan Williams 2023-09-25 786 int ret, size = report_size + ext_size;
80013405d5b7c6 Dan Williams 2023-09-25 787 int certs_size, cert_count, i, offset;
80013405d5b7c6 Dan Williams 2023-09-25 788 u8 *certs_address;
80013405d5b7c6 Dan Williams 2023-09-25 789
80013405d5b7c6 Dan Williams 2023-09-25 790 if (desc->inblob_len != 64)
80013405d5b7c6 Dan Williams 2023-09-25 791 return -EINVAL;
80013405d5b7c6 Dan Williams 2023-09-25 792
80013405d5b7c6 Dan Williams 2023-09-25 793 void *buf __free(kvfree) = kvzalloc(size, GFP_KERNEL);
80013405d5b7c6 Dan Williams 2023-09-25 794 if (!buf)
80013405d5b7c6 Dan Williams 2023-09-25 795 return -ENOMEM;
80013405d5b7c6 Dan Williams 2023-09-25 796
80013405d5b7c6 Dan Williams 2023-09-25 797 guard(mutex)(&snp_cmd_mutex);

Hoho. Need guard stuff and no warnings generated. Perhaps I have added
all the new unlock functions. :)

80013405d5b7c6 Dan Williams 2023-09-25 798 certs_address = buf + report_size;
80013405d5b7c6 Dan Williams 2023-09-25 799 struct snp_ext_report_req ext_req = {
80013405d5b7c6 Dan Williams 2023-09-25 800 .data = { .vmpl = desc->privlevel },
80013405d5b7c6 Dan Williams 2023-09-25 801 .certs_address = (__u64)certs_address,
80013405d5b7c6 Dan Williams 2023-09-25 802 .certs_len = ext_size,
80013405d5b7c6 Dan Williams 2023-09-25 803 };
80013405d5b7c6 Dan Williams 2023-09-25 804 memcpy(&ext_req.data.user_data, desc->inblob, desc->inblob_len);
80013405d5b7c6 Dan Williams 2023-09-25 805
80013405d5b7c6 Dan Williams 2023-09-25 806 struct snp_guest_request_ioctl input = {
80013405d5b7c6 Dan Williams 2023-09-25 807 .msg_version = 1,
80013405d5b7c6 Dan Williams 2023-09-25 808 .req_data = (__u64)&ext_req,
80013405d5b7c6 Dan Williams 2023-09-25 809 .resp_data = (__u64)buf,
80013405d5b7c6 Dan Williams 2023-09-25 810 };
80013405d5b7c6 Dan Williams 2023-09-25 811 struct snp_req_resp io = {
80013405d5b7c6 Dan Williams 2023-09-25 812 .req_data = KERNEL_SOCKPTR(&ext_req),
80013405d5b7c6 Dan Williams 2023-09-25 813 .resp_data = KERNEL_SOCKPTR(buf),
80013405d5b7c6 Dan Williams 2023-09-25 814 };
80013405d5b7c6 Dan Williams 2023-09-25 815
80013405d5b7c6 Dan Williams 2023-09-25 816 ret = get_ext_report(snp_dev, &input, &io);
80013405d5b7c6 Dan Williams 2023-09-25 817
80013405d5b7c6 Dan Williams 2023-09-25 818 if (ret)
80013405d5b7c6 Dan Williams 2023-09-25 819 return ret;
80013405d5b7c6 Dan Williams 2023-09-25 820
80013405d5b7c6 Dan Williams 2023-09-25 821 memcpy(&hdr, buf, sizeof(hdr));
80013405d5b7c6 Dan Williams 2023-09-25 822 if (hdr.status == SNP_REPORT_INVALID_PARAM)
80013405d5b7c6 Dan Williams 2023-09-25 823 return -EINVAL;
80013405d5b7c6 Dan Williams 2023-09-25 824 if (hdr.status == SNP_REPORT_INVALID_KEY_SEL)
80013405d5b7c6 Dan Williams 2023-09-25 825 return -EINVAL;
80013405d5b7c6 Dan Williams 2023-09-25 826 if (hdr.status)
80013405d5b7c6 Dan Williams 2023-09-25 827 return -ENXIO;
80013405d5b7c6 Dan Williams 2023-09-25 828 if ((hdr.report_size + sizeof(hdr)) > report_size)
80013405d5b7c6 Dan Williams 2023-09-25 829 return -ENOMEM;
80013405d5b7c6 Dan Williams 2023-09-25 830
80013405d5b7c6 Dan Williams 2023-09-25 831 void *rbuf __free(kvfree) = kvzalloc(hdr.report_size, GFP_KERNEL);
80013405d5b7c6 Dan Williams 2023-09-25 832 if (!rbuf)
80013405d5b7c6 Dan Williams 2023-09-25 833 return -ENOMEM;
80013405d5b7c6 Dan Williams 2023-09-25 834
80013405d5b7c6 Dan Williams 2023-09-25 835 memcpy(rbuf, buf + sizeof(hdr), hdr.report_size);
80013405d5b7c6 Dan Williams 2023-09-25 836 report->outblob = no_free_ptr(rbuf);
80013405d5b7c6 Dan Williams 2023-09-25 837 report->outblob_len = hdr.report_size;
80013405d5b7c6 Dan Williams 2023-09-25 838
80013405d5b7c6 Dan Williams 2023-09-25 839 for (i = 0; i < ext_size / sizeof(struct snp_msg_cert_entry); i++) {
80013405d5b7c6 Dan Williams 2023-09-25 840 struct snp_msg_cert_entry *certs = buf + report_size;
80013405d5b7c6 Dan Williams 2023-09-25 841
80013405d5b7c6 Dan Williams 2023-09-25 842 if (memcmp(&certs[i], &zero_ent, sizeof(zero_ent)) == 0)
80013405d5b7c6 Dan Williams 2023-09-25 843 break;
80013405d5b7c6 Dan Williams 2023-09-25 844 certs_size += certs[i].length;

certs_size needs to be initialized to zero.

80013405d5b7c6 Dan Williams 2023-09-25 845 }
80013405d5b7c6 Dan Williams 2023-09-25 846 cert_count = i;
80013405d5b7c6 Dan Williams 2023-09-25 847
80013405d5b7c6 Dan Williams 2023-09-25 848 /* No certs to report */
80013405d5b7c6 Dan Williams 2023-09-25 849 if (cert_count == 0)
80013405d5b7c6 Dan Williams 2023-09-25 850 return 0;
80013405d5b7c6 Dan Williams 2023-09-25 851
80013405d5b7c6 Dan Williams 2023-09-25 852 /* sanity check that the entire certs table with metadata fits */
80013405d5b7c6 Dan Williams 2023-09-25 @853 if ((cert_count + 1) * sizeof(zero_ent) + certs_size > ext_size)
80013405d5b7c6 Dan Williams 2023-09-25 854 return -ENXIO;
80013405d5b7c6 Dan Williams 2023-09-25 855
80013405d5b7c6 Dan Williams 2023-09-25 856 void *cbuf __free(kvfree) = kvzalloc(certs_size, GFP_KERNEL);
80013405d5b7c6 Dan Williams 2023-09-25 857 if (!cbuf)
80013405d5b7c6 Dan Williams 2023-09-25 858 return -ENOMEM;
80013405d5b7c6 Dan Williams 2023-09-25 859
80013405d5b7c6 Dan Williams 2023-09-25 860 /* Concatenate returned certs */
80013405d5b7c6 Dan Williams 2023-09-25 861 for (i = 0, offset = 0; i < cert_count; i++) {
80013405d5b7c6 Dan Williams 2023-09-25 862 struct snp_msg_cert_entry *certs = buf + report_size;
80013405d5b7c6 Dan Williams 2023-09-25 863
80013405d5b7c6 Dan Williams 2023-09-25 864 memcpy(cbuf + offset, certs_address + certs[i].offset, certs[i].length);
80013405d5b7c6 Dan Williams 2023-09-25 865 offset += certs[i].length;
80013405d5b7c6 Dan Williams 2023-09-25 866 }
80013405d5b7c6 Dan Williams 2023-09-25 867
80013405d5b7c6 Dan Williams 2023-09-25 868 report->certs = no_free_ptr(cbuf);
80013405d5b7c6 Dan Williams 2023-09-25 869 report->certs_len = certs_size;
80013405d5b7c6 Dan Williams 2023-09-25 870
80013405d5b7c6 Dan Williams 2023-09-25 871 return 0;
80013405d5b7c6 Dan Williams 2023-09-25 872 }

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki