* [PATCH v7 1/7] virt: sevguest: Fix passing a stack buffer as a scatterlist target
2023-10-20 1:16 [PATCH v7 0/7] configfs-tsm: Attestation Report ABI Dan Williams
@ 2023-10-20 1:16 ` Dan Williams
2023-10-20 1:16 ` [PATCH v7 2/7] virt: coco: Add a coco/Makefile and coco/Kconfig Dan Williams
` (5 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Dan Williams @ 2023-10-20 1:16 UTC (permalink / raw)
To: linux-coco
Cc: Peter Gonda, Borislav Petkov, Tom Lendacky, Dionna Glaze,
Jeremi Piotrowski, Kuppuswamy Sathyanarayanan, Tom Lendacky,
peterz, dave.hansen, x86
CONFIG_DEBUG_SG highlights that get_{report,ext_report,derived_key)()}
are passing stack buffers as the @req_buf argument to
handle_guest_request(), generating a Call Trace of the following form:
WARNING: CPU: 0 PID: 1175 at include/linux/scatterlist.h:187 enc_dec_message+0x518/0x5b0 [sev_guest]
[..]
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/26/2023
RIP: 0010:enc_dec_message+0x518/0x5b0 [sev_guest]
Call Trace:
<TASK>
[..]
handle_guest_request+0x135/0x520 [sev_guest]
get_ext_report+0x1ec/0x3e0 [sev_guest]
snp_guest_ioctl+0x157/0x200 [sev_guest]
Note that the above Call Trace was with the DEBUG_SG BUG_ON()s converted
to WARN_ON()s.
This is benign as long as there are no hardware crypto accelerators
loaded for the aead cipher, and no subsequent dma_map_sg() is performed
on the scatterlist. However, sev-guest can not assume the presence of
an aead accelerator nor can it assume that CONFIG_DEBUG_SG is disabled.
Resolve this bug by allocating virt_addr_valid() memory, similar to the
other buffers am @snp_dev instance carries, to marshal requests from
user buffers to kernel buffers.
Reported-by: Peter Gonda <pgonda@google.com>
Closes: http://lore.kernel.org/r/CAMkAt6r2VPPMZ__SQfJse8qWsUyYW3AgYbOUVM0S_Vtk=KvkxQ@mail.gmail.com
Fixes: fce96cf04430 ("virt: Add SEV-SNP guest driver")
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>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/virt/coco/sev-guest/sev-guest.c | 45 +++++++++++++++++--------------
1 file changed, 25 insertions(+), 20 deletions(-)
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 97dbe715e96a..5bee58ef5f1e 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -57,6 +57,11 @@ struct snp_guest_dev {
struct snp_secrets_page_layout *layout;
struct snp_req_data input;
+ union {
+ struct snp_report_req report;
+ struct snp_derived_key_req derived_key;
+ struct snp_ext_report_req ext_report;
+ } req;
u32 *os_area_msg_seqno;
u8 *vmpck;
};
@@ -473,8 +478,8 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
{
struct snp_guest_crypto *crypto = snp_dev->crypto;
+ struct snp_report_req *req = &snp_dev->req.report;
struct snp_report_resp *resp;
- struct snp_report_req req;
int rc, resp_len;
lockdep_assert_held(&snp_cmd_mutex);
@@ -482,7 +487,7 @@ static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_io
if (!arg->req_data || !arg->resp_data)
return -EINVAL;
- if (copy_from_user(&req, (void __user *)arg->req_data, sizeof(req)))
+ if (copy_from_user(req, (void __user *)arg->req_data, sizeof(*req)))
return -EFAULT;
/*
@@ -496,7 +501,7 @@ static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_io
return -ENOMEM;
rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg,
- SNP_MSG_REPORT_REQ, &req, sizeof(req), resp->data,
+ SNP_MSG_REPORT_REQ, req, sizeof(*req), resp->data,
resp_len);
if (rc)
goto e_free;
@@ -511,9 +516,9 @@ static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_io
static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
{
+ struct snp_derived_key_req *req = &snp_dev->req.derived_key;
struct snp_guest_crypto *crypto = snp_dev->crypto;
struct snp_derived_key_resp resp = {0};
- struct snp_derived_key_req req;
int rc, resp_len;
/* Response data is 64 bytes and max authsize for GCM is 16 bytes. */
u8 buf[64 + 16];
@@ -532,11 +537,11 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
if (sizeof(buf) < resp_len)
return -ENOMEM;
- if (copy_from_user(&req, (void __user *)arg->req_data, sizeof(req)))
+ if (copy_from_user(req, (void __user *)arg->req_data, sizeof(*req)))
return -EFAULT;
rc = handle_guest_request(snp_dev, SVM_VMGEXIT_GUEST_REQUEST, arg,
- SNP_MSG_KEY_REQ, &req, sizeof(req), buf, resp_len);
+ SNP_MSG_KEY_REQ, req, sizeof(*req), buf, resp_len);
if (rc)
return rc;
@@ -552,8 +557,8 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
{
+ struct snp_ext_report_req *req = &snp_dev->req.ext_report;
struct snp_guest_crypto *crypto = snp_dev->crypto;
- struct snp_ext_report_req req;
struct snp_report_resp *resp;
int ret, npages = 0, resp_len;
@@ -562,18 +567,18 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
if (!arg->req_data || !arg->resp_data)
return -EINVAL;
- if (copy_from_user(&req, (void __user *)arg->req_data, sizeof(req)))
+ if (copy_from_user(req, (void __user *)arg->req_data, sizeof(*req)))
return -EFAULT;
/* userspace does not want certificate data */
- if (!req.certs_len || !req.certs_address)
+ if (!req->certs_len || !req->certs_address)
goto cmd;
- if (req.certs_len > SEV_FW_BLOB_MAX_SIZE ||
- !IS_ALIGNED(req.certs_len, PAGE_SIZE))
+ if (req->certs_len > SEV_FW_BLOB_MAX_SIZE ||
+ !IS_ALIGNED(req->certs_len, PAGE_SIZE))
return -EINVAL;
- if (!access_ok((const void __user *)req.certs_address, req.certs_len))
+ if (!access_ok((const void __user *)req->certs_address, req->certs_len))
return -EFAULT;
/*
@@ -582,8 +587,8 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
* the host. If host does not supply any certs in it, then copy
* zeros to indicate that certificate data was not provided.
*/
- memset(snp_dev->certs_data, 0, req.certs_len);
- npages = req.certs_len >> PAGE_SHIFT;
+ memset(snp_dev->certs_data, 0, req->certs_len);
+ npages = req->certs_len >> PAGE_SHIFT;
cmd:
/*
* The intermediate response buffer is used while decrypting the
@@ -597,14 +602,14 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
snp_dev->input.data_npages = npages;
ret = handle_guest_request(snp_dev, SVM_VMGEXIT_EXT_GUEST_REQUEST, arg,
- SNP_MSG_REPORT_REQ, &req.data,
- sizeof(req.data), resp->data, resp_len);
+ SNP_MSG_REPORT_REQ, &req->data,
+ sizeof(req->data), resp->data, resp_len);
/* If certs length is invalid then copy the returned length */
if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
- req.certs_len = snp_dev->input.data_npages << PAGE_SHIFT;
+ req->certs_len = snp_dev->input.data_npages << PAGE_SHIFT;
- if (copy_to_user((void __user *)arg->req_data, &req, sizeof(req)))
+ if (copy_to_user((void __user *)arg->req_data, req, sizeof(*req)))
ret = -EFAULT;
}
@@ -612,8 +617,8 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
goto e_free;
if (npages &&
- copy_to_user((void __user *)req.certs_address, snp_dev->certs_data,
- req.certs_len)) {
+ copy_to_user((void __user *)req->certs_address, snp_dev->certs_data,
+ req->certs_len)) {
ret = -EFAULT;
goto e_free;
}
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v7 2/7] virt: coco: Add a coco/Makefile and coco/Kconfig
2023-10-20 1:16 [PATCH v7 0/7] configfs-tsm: Attestation Report ABI Dan Williams
2023-10-20 1:16 ` [PATCH v7 1/7] virt: sevguest: Fix passing a stack buffer as a scatterlist target Dan Williams
@ 2023-10-20 1:16 ` Dan Williams
2023-10-20 1:16 ` [PATCH v7 3/7] configfs-tsm: Introduce a shared ABI for attestation reports Dan Williams
` (4 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Dan Williams @ 2023-10-20 1:16 UTC (permalink / raw)
To: linux-coco
Cc: Kuppuswamy Sathyanarayanan, Kuppuswamy Sathyanarayanan,
Tom Lendacky, peterz, dave.hansen, x86
In preparation for adding another coco build target, relieve
drivers/virt/Makefile of the responsibility to track new compilation
unit additions to drivers/virt/coco/, and do the same for
drivers/virt/Kconfig.
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Tested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/virt/Kconfig | 6 +-----
drivers/virt/Makefile | 4 +---
drivers/virt/coco/Kconfig | 9 +++++++++
drivers/virt/coco/Makefile | 7 +++++++
4 files changed, 18 insertions(+), 8 deletions(-)
create mode 100644 drivers/virt/coco/Kconfig
create mode 100644 drivers/virt/coco/Makefile
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index f79ab13a5c28..40129b6f0eca 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -48,10 +48,6 @@ source "drivers/virt/nitro_enclaves/Kconfig"
source "drivers/virt/acrn/Kconfig"
-source "drivers/virt/coco/efi_secret/Kconfig"
-
-source "drivers/virt/coco/sev-guest/Kconfig"
-
-source "drivers/virt/coco/tdx-guest/Kconfig"
+source "drivers/virt/coco/Kconfig"
endif
diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index e9aa6fc96fab..f29901bd7820 100644
--- a/drivers/virt/Makefile
+++ b/drivers/virt/Makefile
@@ -9,6 +9,4 @@ obj-y += vboxguest/
obj-$(CONFIG_NITRO_ENCLAVES) += nitro_enclaves/
obj-$(CONFIG_ACRN_HSM) += acrn/
-obj-$(CONFIG_EFI_SECRET) += coco/efi_secret/
-obj-$(CONFIG_SEV_GUEST) += coco/sev-guest/
-obj-$(CONFIG_INTEL_TDX_GUEST) += coco/tdx-guest/
+obj-y += coco/
diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
new file mode 100644
index 000000000000..fc5c64f04c4a
--- /dev/null
+++ b/drivers/virt/coco/Kconfig
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Confidential computing related collateral
+#
+source "drivers/virt/coco/efi_secret/Kconfig"
+
+source "drivers/virt/coco/sev-guest/Kconfig"
+
+source "drivers/virt/coco/tdx-guest/Kconfig"
diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
new file mode 100644
index 000000000000..55302ef719ad
--- /dev/null
+++ b/drivers/virt/coco/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Confidential computing related collateral
+#
+obj-$(CONFIG_EFI_SECRET) += efi_secret/
+obj-$(CONFIG_SEV_GUEST) += sev-guest/
+obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v7 3/7] configfs-tsm: Introduce a shared ABI for attestation reports
2023-10-20 1:16 [PATCH v7 0/7] configfs-tsm: Attestation Report ABI Dan Williams
2023-10-20 1:16 ` [PATCH v7 1/7] virt: sevguest: Fix passing a stack buffer as a scatterlist target Dan Williams
2023-10-20 1:16 ` [PATCH v7 2/7] virt: coco: Add a coco/Makefile and coco/Kconfig Dan Williams
@ 2023-10-20 1:16 ` Dan Williams
2023-10-25 4:14 ` Qinkun Bao
2023-10-20 1:16 ` [PATCH v7 4/7] virt: sevguest: Prep for kernel internal get_ext_report() Dan Williams
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Dan Williams @ 2023-10-20 1:16 UTC (permalink / raw)
To: linux-coco
Cc: Kuppuswamy Sathyanarayanan, Dionna Amalie Glaze, James Bottomley,
Peter Gonda, Greg Kroah-Hartman, Samuel Ortiz, Greg Kroah-Hartman,
Thomas Gleixner, Kuppuswamy Sathyanarayanan,
Kuppuswamy Sathyanarayanan, Tom Lendacky, peterz, dave.hansen,
x86
One of the common operations of a TSM (Trusted Security Module) is to
provide a way for a TVM (confidential computing guest execution
environment) to take a measurement of its launch state, sign it and
submit it to a verifying party. Upon successful attestation that
verifies the integrity of the TVM additional secrets may be deployed.
The concept is common across TSMs, but the implementations are
unfortunately vendor specific. While the industry grapples with a common
definition of this attestation format [1], Linux need not make this
problem worse by defining a new ABI per TSM that wants to perform a
similar operation. The current momentum has been to invent new ioctl-ABI
per TSM per function which at best is an abdication of the kernel's
responsibility to make common infrastructure concepts share common ABI.
The proposal, targeted to conceptually work with TDX, SEV-SNP, COVE if
not more, is to define a configfs interface to retrieve the TSM-specific
blob.
report=/sys/kernel/config/tsm/report/report0
mkdir $report
dd if=binary_userdata_plus_nonce > $report/inblob
hexdump $report/outblob
This approach later allows for the standardization of the attestation
blob format without needing to invent a new ABI. Once standardization
happens the standard format can be emitted by $report/outblob and
indicated by $report/provider, or a new attribute like
"$report/tcg_coco_report" can emit the standard format alongside the
vendor format.
Review of previous iterations of this interface identified that there is
a need to scale report generation for multiple container environments
[2]. Configfs enables a model where each container can bind mount one or
more report generation item instances. Still, within a container only a
single thread can be manipulating a given configuration instance at a
time. A 'generation' count is provided to detect conflicts between
multiple threads racing to configure a report instance.
The SEV-SNP concepts of "extended reports" and "privilege levels" are
optionally enabled by selecting 'tsm_report_ext_type' at register_tsm()
time. The expectation is that those concepts are generic enough that
they may be adopted by other TSM implementations. In other words,
configfs-tsm aims to address a superset of TSM specific functionality
with a common ABI where attributes may appear, or not appear, based on
the set of concepts the implementation supports.
Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
Link: http://lore.kernel.org/r/57f3a05e-8fcd-4656-beea-56bb8365ae64@linux.microsoft.com [2]
Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Peter Gonda <pgonda@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Samuel Ortiz <sameo@rivosinc.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Tested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Documentation/ABI/testing/configfs-tsm | 82 ++++++
MAINTAINERS | 8 +
drivers/virt/coco/Kconfig | 5
drivers/virt/coco/Makefile | 1
drivers/virt/coco/tsm.c | 425 ++++++++++++++++++++++++++++++++
include/linux/tsm.h | 69 +++++
6 files changed, 590 insertions(+)
create mode 100644 Documentation/ABI/testing/configfs-tsm
create mode 100644 drivers/virt/coco/tsm.c
create mode 100644 include/linux/tsm.h
diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
new file mode 100644
index 000000000000..dd24202b5ba5
--- /dev/null
+++ b/Documentation/ABI/testing/configfs-tsm
@@ -0,0 +1,82 @@
+What: /sys/kernel/config/tsm/report/$name/inblob
+Date: September, 2023
+KernelVersion: v6.7
+Contact: linux-coco@lists.linux.dev
+Description:
+ (WO) Up to 64 bytes of user specified binary data. For replay
+ protection this should include a nonce, but the kernel does not
+ place any restrictions on the content.
+
+What: /sys/kernel/config/tsm/report/$name/outblob
+Date: September, 2023
+KernelVersion: v6.7
+Contact: linux-coco@lists.linux.dev
+Description:
+ (RO) Binary attestation report generated from @inblob and other
+ options The format of the report is implementation specific
+ where the implementation is conveyed via the @provider
+ attribute.
+
+What: /sys/kernel/config/tsm/report/$name/auxblob
+Date: October, 2023
+KernelVersion: v6.7
+Contact: linux-coco@lists.linux.dev
+Description:
+ (RO) Optional supplemental data that a TSM may emit, visibility
+ of this attribute depends on TSM, and may be empty if no
+ auxiliary data is available.
+
+ When @provider is "sev_guest" this file contains the
+ "cert_table" from SEV-ES Guest-Hypervisor Communication Block
+ Standardization v2.03 Section 4.1.8.1 MSG_REPORT_REQ.
+ https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56421.pdf
+
+What: /sys/kernel/config/tsm/report/$name/provider
+Date: September, 2023
+KernelVersion: v6.7
+Contact: linux-coco@lists.linux.dev
+Description:
+ (RO) A name for the format-specification of @outblob like
+ "sev_guest" [1] or "tdx_guest" [2] in the near term, or a
+ common standard format in the future.
+
+ [1]: SEV Secure Nested Paging Firmware ABI Specification
+ Revision 1.55 Table 22
+ https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56860.pdf
+
+ [2]: Intel® Trust Domain Extensions Data Center Attestation
+ Primitives : Quote Generation Library and Quote Verification
+ Library Revision 0.8 Appendix 4,5
+ https://download.01.org/intel-sgx/latest/dcap-latest/linux/docs/Intel_TDX_DCAP_Quoting_Library_API.pdf
+
+What: /sys/kernel/config/tsm/report/$name/generation
+Date: September, 2023
+KernelVersion: v6.7
+Contact: linux-coco@lists.linux.dev
+Description:
+ (RO) The value in this attribute increments each time @inblob or
+ any option is written. Userspace can detect conflicts by
+ checking generation before writing to any attribute and making
+ sure the number of writes matches expectations after reading
+ @outblob, or it can prevent conflicts by creating a report
+ instance per requesting context.
+
+What: /sys/kernel/config/tsm/report/$name/privlevel
+Date: September, 2023
+KernelVersion: v6.7
+Contact: linux-coco@lists.linux.dev
+Description:
+ (WO) Attribute is visible if a TSM implementation provider
+ supports the concept of attestation reports for TVMs running at
+ different privilege levels, like SEV-SNP "VMPL", specify the
+ privilege level via this attribute. The minimum acceptable
+ value is conveyed via @privlevel_floor and the maximum
+ acceptable value is TSM_PRIVLEVEL_MAX (3).
+
+What: /sys/kernel/config/tsm/report/$name/privlevel_floor
+Date: September, 2023
+KernelVersion: v6.7
+Contact: linux-coco@lists.linux.dev
+Description:
+ (RO) Indicates the minimum permissible value that can be written
+ to @privlevel.
diff --git a/MAINTAINERS b/MAINTAINERS
index b19995690904..8acbeb029ba1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21889,6 +21889,14 @@ W: https://github.com/srcres258/linux-doc
T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
F: Documentation/translations/zh_TW/
+TRUSTED SECURITY MODULE (TSM) ATTESTATION REPORTS
+M: Dan Williams <dan.j.williams@intel.com>
+L: linux-coco@lists.linux.dev
+S: Maintained
+F: Documentation/ABI/testing/configfs-tsm
+F: drivers/virt/coco/tsm.c
+F: include/linux/tsm.h
+
TTY LAYER AND SERIAL DRIVERS
M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
M: Jiri Slaby <jirislaby@kernel.org>
diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
index fc5c64f04c4a..87d142c1f932 100644
--- a/drivers/virt/coco/Kconfig
+++ b/drivers/virt/coco/Kconfig
@@ -2,6 +2,11 @@
#
# Confidential computing related collateral
#
+
+config TSM_REPORTS
+ select CONFIGFS_FS
+ tristate
+
source "drivers/virt/coco/efi_secret/Kconfig"
source "drivers/virt/coco/sev-guest/Kconfig"
diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
index 55302ef719ad..18c1aba5edb7 100644
--- a/drivers/virt/coco/Makefile
+++ b/drivers/virt/coco/Makefile
@@ -2,6 +2,7 @@
#
# Confidential computing related collateral
#
+obj-$(CONFIG_TSM_REPORTS) += tsm.o
obj-$(CONFIG_EFI_SECRET) += efi_secret/
obj-$(CONFIG_SEV_GUEST) += sev-guest/
obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
new file mode 100644
index 000000000000..d1c2db83a8ca
--- /dev/null
+++ b/drivers/virt/coco/tsm.c
@@ -0,0 +1,425 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2023 Intel Corporation. All rights reserved. */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/tsm.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/rwsem.h>
+#include <linux/string.h>
+#include <linux/module.h>
+#include <linux/cleanup.h>
+#include <linux/configfs.h>
+
+static struct tsm_provider {
+ const struct tsm_ops *ops;
+ const struct config_item_type *type;
+ void *data;
+} provider;
+static DECLARE_RWSEM(tsm_rwsem);
+
+/**
+ * DOC: Trusted Security Module (TSM) Attestation Report Interface
+ *
+ * The TSM report interface is a common provider of blobs that facilitate
+ * attestation of a TVM (confidential computing guest) by an attestation
+ * service. A TSM report combines a user-defined blob (likely a public-key with
+ * a nonce for a key-exchange protocol) with a signed attestation report. That
+ * combined blob is then used to obtain secrets provided by an agent that can
+ * validate the attestation report. The expectation is that this interface is
+ * invoked infrequently, however configfs allows for multiple agents to
+ * own their own report generation instances to generate reports as
+ * often as needed.
+ *
+ * The attestation report format is TSM provider specific, when / if a standard
+ * materializes that can be published instead of the vendor layout. Until then
+ * the 'provider' attribute indicates the format of 'outblob', and optionally
+ * 'auxblob'.
+ */
+
+struct tsm_report_state {
+ struct tsm_report report;
+ unsigned long write_generation;
+ unsigned long read_generation;
+ struct config_item cfg;
+};
+
+enum tsm_data_select {
+ TSM_REPORT,
+ TSM_CERTS,
+};
+
+static struct tsm_report *to_tsm_report(struct config_item *cfg)
+{
+ struct tsm_report_state *state =
+ container_of(cfg, struct tsm_report_state, cfg);
+
+ return &state->report;
+}
+
+static struct tsm_report_state *to_state(struct tsm_report *report)
+{
+ return container_of(report, struct tsm_report_state, report);
+}
+
+static int try_advance_write_generation(struct tsm_report *report)
+{
+ struct tsm_report_state *state = to_state(report);
+
+ lockdep_assert_held_write(&tsm_rwsem);
+
+ /*
+ * Malicious or broken userspace has written enough times for
+ * read_generation == write_generation by modular arithmetic without an
+ * interim read. Stop accepting updates until the current report
+ * configuration is read.
+ */
+ if (state->write_generation == state->read_generation - 1)
+ return -EBUSY;
+ state->write_generation++;
+ return 0;
+}
+
+static ssize_t tsm_report_privlevel_store(struct config_item *cfg,
+ const char *buf, size_t len)
+{
+ struct tsm_report *report = to_tsm_report(cfg);
+ unsigned int val;
+ int rc;
+
+ rc = kstrtouint(buf, 0, &val);
+ if (rc)
+ return rc;
+
+ /*
+ * The valid privilege levels that a TSM might accept, if it accepts a
+ * privilege level setting at all, are a max of TSM_PRIVLEVEL_MAX (see
+ * SEV-SNP GHCB) and a minimum of a TSM selected floor value no less
+ * than 0.
+ */
+ if (provider.ops->privlevel_floor > val || val > TSM_PRIVLEVEL_MAX)
+ return -EINVAL;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ rc = try_advance_write_generation(report);
+ if (rc)
+ return rc;
+ report->desc.privlevel = val;
+
+ return len;
+}
+CONFIGFS_ATTR_WO(tsm_report_, privlevel);
+
+static ssize_t tsm_report_privlevel_floor_show(struct config_item *cfg,
+ char *buf)
+{
+ guard(rwsem_read)(&tsm_rwsem);
+ return sysfs_emit(buf, "%u\n", provider.ops->privlevel_floor);
+}
+CONFIGFS_ATTR_RO(tsm_report_, privlevel_floor);
+
+static ssize_t tsm_report_inblob_write(struct config_item *cfg,
+ const void *buf, size_t count)
+{
+ struct tsm_report *report = to_tsm_report(cfg);
+ int rc;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ rc = try_advance_write_generation(report);
+ if (rc)
+ return rc;
+
+ report->desc.inblob_len = count;
+ memcpy(report->desc.inblob, buf, count);
+ return count;
+}
+CONFIGFS_BIN_ATTR_WO(tsm_report_, inblob, NULL, TSM_INBLOB_MAX);
+
+static ssize_t tsm_report_generation_show(struct config_item *cfg, char *buf)
+{
+ struct tsm_report *report = to_tsm_report(cfg);
+ struct tsm_report_state *state = to_state(report);
+
+ guard(rwsem_read)(&tsm_rwsem);
+ return sysfs_emit(buf, "%lu\n", state->write_generation);
+}
+CONFIGFS_ATTR_RO(tsm_report_, generation);
+
+static ssize_t tsm_report_provider_show(struct config_item *cfg, char *buf)
+{
+ guard(rwsem_read)(&tsm_rwsem);
+ return sysfs_emit(buf, "%s\n", provider.ops->name);
+}
+CONFIGFS_ATTR_RO(tsm_report_, provider);
+
+static ssize_t __read_report(struct tsm_report *report, void *buf, size_t count,
+ enum tsm_data_select select)
+{
+ loff_t offset = 0;
+ ssize_t len;
+ u8 *out;
+
+ if (select == TSM_REPORT) {
+ out = report->outblob;
+ len = report->outblob_len;
+ } else {
+ out = report->auxblob;
+ len = report->auxblob_len;
+ }
+
+ /*
+ * Recall that a NULL @buf is configfs requesting the size of
+ * the buffer.
+ */
+ if (!buf)
+ return len;
+ return memory_read_from_buffer(buf, count, &offset, out, len);
+}
+
+static ssize_t read_cached_report(struct tsm_report *report, void *buf,
+ size_t count, enum tsm_data_select select)
+{
+ struct tsm_report_state *state = to_state(report);
+
+ guard(rwsem_read)(&tsm_rwsem);
+ if (!report->desc.inblob_len)
+ return -EINVAL;
+
+ /*
+ * A given TSM backend always fills in ->outblob regardless of
+ * whether the report includes an auxblob or not.
+ */
+ if (!report->outblob ||
+ state->read_generation != state->write_generation)
+ return -EWOULDBLOCK;
+
+ return __read_report(report, buf, count, select);
+}
+
+static ssize_t tsm_report_read(struct tsm_report *report, void *buf,
+ size_t count, enum tsm_data_select select)
+{
+ struct tsm_report_state *state = to_state(report);
+ const struct tsm_ops *ops;
+ ssize_t rc;
+
+ /* try to read from the existing report if present and valid... */
+ rc = read_cached_report(report, buf, count, select);
+ if (rc >= 0 || rc != -EWOULDBLOCK)
+ return rc;
+
+ /* slow path, report may need to be regenerated... */
+ guard(rwsem_write)(&tsm_rwsem);
+ ops = provider.ops;
+ if (!ops)
+ return -ENOTTY;
+ if (!report->desc.inblob_len)
+ return -EINVAL;
+
+ /* did another thread already generate this report? */
+ if (report->outblob &&
+ state->read_generation == state->write_generation)
+ goto out;
+
+ kvfree(report->outblob);
+ kvfree(report->auxblob);
+ report->outblob = NULL;
+ report->auxblob = NULL;
+ rc = ops->report_new(report, provider.data);
+ if (rc < 0)
+ return rc;
+ state->read_generation = state->write_generation;
+out:
+ return __read_report(report, buf, count, select);
+}
+
+static ssize_t tsm_report_outblob_read(struct config_item *cfg, void *buf,
+ size_t count)
+{
+ struct tsm_report *report = to_tsm_report(cfg);
+
+ return tsm_report_read(report, buf, count, TSM_REPORT);
+}
+CONFIGFS_BIN_ATTR_RO(tsm_report_, outblob, NULL, TSM_OUTBLOB_MAX);
+
+static ssize_t tsm_report_auxblob_read(struct config_item *cfg, void *buf,
+ size_t count)
+{
+ struct tsm_report *report = to_tsm_report(cfg);
+
+ return tsm_report_read(report, buf, count, TSM_CERTS);
+}
+CONFIGFS_BIN_ATTR_RO(tsm_report_, auxblob, NULL, TSM_OUTBLOB_MAX);
+
+#define TSM_DEFAULT_ATTRS() \
+ &tsm_report_attr_generation, \
+ &tsm_report_attr_provider
+
+static struct configfs_attribute *tsm_report_attrs[] = {
+ TSM_DEFAULT_ATTRS(),
+ NULL,
+};
+
+static struct configfs_attribute *tsm_report_extra_attrs[] = {
+ TSM_DEFAULT_ATTRS(),
+ &tsm_report_attr_privlevel,
+ &tsm_report_attr_privlevel_floor,
+ NULL,
+};
+
+#define TSM_DEFAULT_BIN_ATTRS() \
+ &tsm_report_attr_inblob, \
+ &tsm_report_attr_outblob
+
+static struct configfs_bin_attribute *tsm_report_bin_attrs[] = {
+ TSM_DEFAULT_BIN_ATTRS(),
+ NULL,
+};
+
+static struct configfs_bin_attribute *tsm_report_bin_extra_attrs[] = {
+ TSM_DEFAULT_BIN_ATTRS(),
+ &tsm_report_attr_auxblob,
+ NULL,
+};
+
+static void tsm_report_item_release(struct config_item *cfg)
+{
+ struct tsm_report *report = to_tsm_report(cfg);
+ struct tsm_report_state *state = to_state(report);
+
+ kvfree(report->auxblob);
+ kvfree(report->outblob);
+ kfree(state);
+}
+
+static struct configfs_item_operations tsm_report_item_ops = {
+ .release = tsm_report_item_release,
+};
+
+const struct config_item_type tsm_report_default_type = {
+ .ct_owner = THIS_MODULE,
+ .ct_bin_attrs = tsm_report_bin_attrs,
+ .ct_attrs = tsm_report_attrs,
+ .ct_item_ops = &tsm_report_item_ops,
+};
+EXPORT_SYMBOL_GPL(tsm_report_default_type);
+
+const struct config_item_type tsm_report_extra_type = {
+ .ct_owner = THIS_MODULE,
+ .ct_bin_attrs = tsm_report_bin_extra_attrs,
+ .ct_attrs = tsm_report_extra_attrs,
+ .ct_item_ops = &tsm_report_item_ops,
+};
+EXPORT_SYMBOL_GPL(tsm_report_extra_type);
+
+static struct config_item *tsm_report_make_item(struct config_group *group,
+ const char *name)
+{
+ struct tsm_report_state *state;
+
+ guard(rwsem_read)(&tsm_rwsem);
+ if (!provider.ops)
+ return ERR_PTR(-ENXIO);
+
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return ERR_PTR(-ENOMEM);
+
+ config_item_init_type_name(&state->cfg, name, provider.type);
+ return &state->cfg;
+}
+
+static struct configfs_group_operations tsm_report_group_ops = {
+ .make_item = tsm_report_make_item,
+};
+
+static const struct config_item_type tsm_reports_type = {
+ .ct_owner = THIS_MODULE,
+ .ct_group_ops = &tsm_report_group_ops,
+};
+
+static const struct config_item_type tsm_root_group_type = {
+ .ct_owner = THIS_MODULE,
+};
+
+static struct configfs_subsystem tsm_configfs = {
+ .su_group = {
+ .cg_item = {
+ .ci_namebuf = "tsm",
+ .ci_type = &tsm_root_group_type,
+ },
+ },
+ .su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
+};
+
+int tsm_register(const struct tsm_ops *ops, void *priv,
+ const struct config_item_type *type)
+{
+ const struct tsm_ops *conflict;
+
+ if (!type)
+ type = &tsm_report_default_type;
+ if (!(type == &tsm_report_default_type || type == &tsm_report_extra_type))
+ return -EINVAL;
+
+ guard(rwsem_write)(&tsm_rwsem);
+ conflict = provider.ops;
+ if (conflict) {
+ pr_err("\"%s\" ops already registered\n", conflict->name);
+ return -EBUSY;
+ }
+
+ provider.ops = ops;
+ provider.data = priv;
+ provider.type = type;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tsm_register);
+
+int tsm_unregister(const struct tsm_ops *ops)
+{
+ guard(rwsem_write)(&tsm_rwsem);
+ if (ops != provider.ops)
+ return -EBUSY;
+ provider.ops = NULL;
+ provider.data = NULL;
+ provider.type = NULL;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(tsm_unregister);
+
+static struct config_group *tsm_report_group;
+
+static int __init tsm_init(void)
+{
+ struct config_group *root = &tsm_configfs.su_group;
+ struct config_group *tsm;
+ int rc;
+
+ config_group_init(root);
+ rc = configfs_register_subsystem(&tsm_configfs);
+ if (rc)
+ return rc;
+
+ tsm = configfs_register_default_group(root, "report",
+ &tsm_reports_type);
+ if (IS_ERR(tsm)) {
+ configfs_unregister_subsystem(&tsm_configfs);
+ return PTR_ERR(tsm);
+ }
+ tsm_report_group = tsm;
+
+ return 0;
+}
+module_init(tsm_init);
+
+static void __exit tsm_exit(void)
+{
+ configfs_unregister_default_group(tsm_report_group);
+ configfs_unregister_subsystem(&tsm_configfs);
+}
+module_exit(tsm_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Provide Trusted Security Module attestation reports via configfs");
diff --git a/include/linux/tsm.h b/include/linux/tsm.h
new file mode 100644
index 000000000000..de8324a2223c
--- /dev/null
+++ b/include/linux/tsm.h
@@ -0,0 +1,69 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TSM_H
+#define __TSM_H
+
+#include <linux/sizes.h>
+#include <linux/types.h>
+
+#define TSM_INBLOB_MAX 64
+#define TSM_OUTBLOB_MAX SZ_32K
+
+/*
+ * Privilege level is a nested permission concept to allow confidential
+ * guests to partition address space, 4-levels are supported.
+ */
+#define TSM_PRIVLEVEL_MAX 3
+
+/**
+ * struct tsm_desc - option descriptor for generating tsm report blobs
+ * @privlevel: optional privilege level to associate with @outblob
+ * @inblob_len: sizeof @inblob
+ * @inblob: arbitrary input data
+ */
+struct tsm_desc {
+ unsigned int privlevel;
+ size_t inblob_len;
+ u8 inblob[TSM_INBLOB_MAX];
+};
+
+/**
+ * struct tsm_report - track state of report generation relative to options
+ * @desc: input parameters to @report_new()
+ * @outblob_len: sizeof(@outblob)
+ * @outblob: generated evidence to provider to the attestation agent
+ * @auxblob_len: sizeof(@auxblob)
+ * @auxblob: (optional) auxiliary data to the report (e.g. certificate data)
+ */
+struct tsm_report {
+ struct tsm_desc desc;
+ size_t outblob_len;
+ u8 *outblob;
+ size_t auxblob_len;
+ u8 *auxblob;
+};
+
+/**
+ * struct tsm_ops - attributes and operations for tsm instances
+ * @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
+ * @privlevel_floor: convey base privlevel for nested scenarios
+ * @report_new: Populate @report with the report blob and auxblob
+ * (optional), return 0 on successful population, or -errno otherwise
+ *
+ * Implementation specific ops, only one is expected to be registered at
+ * a time i.e. only one of "sev-guest", "tdx-guest", etc.
+ */
+struct tsm_ops {
+ const char *name;
+ const unsigned int privlevel_floor;
+ int (*report_new)(struct tsm_report *report, void *data);
+};
+
+extern const struct config_item_type tsm_report_default_type;
+
+/* publish @privlevel, @privlevel_floor, and @auxblob attributes */
+extern const struct config_item_type tsm_report_extra_type;
+
+int tsm_register(const struct tsm_ops *ops, void *priv,
+ const struct config_item_type *type);
+int tsm_unregister(const struct tsm_ops *ops);
+#endif /* __TSM_H */
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v7 3/7] configfs-tsm: Introduce a shared ABI for attestation reports
2023-10-20 1:16 ` [PATCH v7 3/7] configfs-tsm: Introduce a shared ABI for attestation reports Dan Williams
@ 2023-10-25 4:14 ` Qinkun Bao
2023-10-25 20:06 ` Dan Williams
0 siblings, 1 reply; 16+ messages in thread
From: Qinkun Bao @ 2023-10-25 4:14 UTC (permalink / raw)
To: Dan Williams
Cc: linux-coco, Kuppuswamy Sathyanarayanan, Dionna Amalie Glaze,
James Bottomley, Peter Gonda, Greg Kroah-Hartman, Samuel Ortiz,
Thomas Gleixner, Tom Lendacky, peterz, Dave Hansen, x86, kmoy,
Chong Cai
> On Oct 19, 2023, at 6:16 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>
> One of the common operations of a TSM (Trusted Security Module) is to
> provide a way for a TVM (confidential computing guest execution
> environment) to take a measurement of its launch state, sign it and
> submit it to a verifying party. Upon successful attestation that
> verifies the integrity of the TVM additional secrets may be deployed.
> The concept is common across TSMs, but the implementations are
> unfortunately vendor specific. While the industry grapples with a common
> definition of this attestation format [1], Linux need not make this
> problem worse by defining a new ABI per TSM that wants to perform a
> similar operation. The current momentum has been to invent new ioctl-ABI
> per TSM per function which at best is an abdication of the kernel's
> responsibility to make common infrastructure concepts share common ABI.
>
> The proposal, targeted to conceptually work with TDX, SEV-SNP, COVE if
> not more, is to define a configfs interface to retrieve the TSM-specific
> blob.
>
> report=/sys/kernel/config/tsm/report/report0
> mkdir $report
> dd if=binary_userdata_plus_nonce > $report/inblob
> hexdump $report/outblob
The patch raised some internal discussions at Google. Some have concerns
about the potential collision problems. We have reached out to Dan. Dan
mentioned that “configfs puts some onus on userspace to avoid collisions”.
I have listed the changes in the userspace here to ensure that we have the
correct understanding and that the complexity for the userspace is necessary.
Changes for the userspace:
The system administrator needs to create a new group, add users to the
group, change the folder owner, and set the permissions.
# System Administrator
groupadd tsm
report_dir=/sys/kernel/config/tsm/report
chown root:tsm $report_dir
chmod og-r $report_dir
chmod g+w $report_dir
# Users under the group tsm can not traverse the $report_dir
# but can create their dedicated interface under the $report_dir
usermod -a -G tsm user1
usermod -a -G tsm user2
…
For each quote generation request, users (e.g., user1, user2) need to create
a temporary folder, generate the quote request and clean up the temporary
directory. If any errors (crashing, not cleaning up on all exit paths) occur
during the quote request, the directories get left behind.
# The operation could be many times per second in the future
instance=$(uuidgen)
mkdir $report_dir/$instance
gen1=$(cat $instance/generation)
dd if=user_data_A > $instance/inblob
hexdump -C $instance/outblob
gen2=$(cat $instance/generation)
# Check gen2 = gen1 + 1
rmdir $report_dir/$instance
>
> This approach later allows for the standardization of the attestation
> blob format without needing to invent a new ABI. Once standardization
> happens the standard format can be emitted by $report/outblob and
> indicated by $report/provider, or a new attribute like
> "$report/tcg_coco_report" can emit the standard format alongside the
> vendor format.
>
> Review of previous iterations of this interface identified that there is
> a need to scale report generation for multiple container environments
> [2]. Configfs enables a model where each container can bind mount one or
> more report generation item instances. Still, within a container only a
> single thread can be manipulating a given configuration instance at a
> time. A 'generation' count is provided to detect conflicts between
> multiple threads racing to configure a report instance.
>
> The SEV-SNP concepts of "extended reports" and "privilege levels" are
> optionally enabled by selecting 'tsm_report_ext_type' at register_tsm()
> time. The expectation is that those concepts are generic enough that
> they may be adopted by other TSM implementations. In other words,
> configfs-tsm aims to address a superset of TSM specific functionality
> with a common ABI where attributes may appear, or not appear, based on
> the set of concepts the implementation supports.
>
> Link: http://lore.kernel.org/r/64961c3baf8ce_142af829436@dwillia2-xfh.jf.intel.com.notmuch [1]
> Link: http://lore.kernel.org/r/57f3a05e-8fcd-4656-beea-56bb8365ae64@linux.microsoft.com [2]
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Cc: Dionna Amalie Glaze <dionnaglaze@google.com>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Peter Gonda <pgonda@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Samuel Ortiz <sameo@rivosinc.com>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Tested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Documentation/ABI/testing/configfs-tsm | 82 ++++++
> MAINTAINERS | 8 +
> drivers/virt/coco/Kconfig | 5
> drivers/virt/coco/Makefile | 1
> drivers/virt/coco/tsm.c | 425 ++++++++++++++++++++++++++++++++
> include/linux/tsm.h | 69 +++++
> 6 files changed, 590 insertions(+)
> create mode 100644 Documentation/ABI/testing/configfs-tsm
> create mode 100644 drivers/virt/coco/tsm.c
> create mode 100644 include/linux/tsm.h
>
> diff --git a/Documentation/ABI/testing/configfs-tsm b/Documentation/ABI/testing/configfs-tsm
> new file mode 100644
> index 000000000000..dd24202b5ba5
> --- /dev/null
> +++ b/Documentation/ABI/testing/configfs-tsm
> @@ -0,0 +1,82 @@
> +What: /sys/kernel/config/tsm/report/$name/inblob
> +Date: September, 2023
> +KernelVersion: v6.7
> +Contact: linux-coco@lists.linux.dev
> +Description:
> + (WO) Up to 64 bytes of user specified binary data. For replay
> + protection this should include a nonce, but the kernel does not
> + place any restrictions on the content.
> +
> +What: /sys/kernel/config/tsm/report/$name/outblob
> +Date: September, 2023
> +KernelVersion: v6.7
> +Contact: linux-coco@lists.linux.dev
> +Description:
> + (RO) Binary attestation report generated from @inblob and other
> + options The format of the report is implementation specific
> + where the implementation is conveyed via the @provider
> + attribute.
> +
> +What: /sys/kernel/config/tsm/report/$name/auxblob
> +Date: October, 2023
> +KernelVersion: v6.7
> +Contact: linux-coco@lists.linux.dev
> +Description:
> + (RO) Optional supplemental data that a TSM may emit, visibility
> + of this attribute depends on TSM, and may be empty if no
> + auxiliary data is available.
> +
> + When @provider is "sev_guest" this file contains the
> + "cert_table" from SEV-ES Guest-Hypervisor Communication Block
> + Standardization v2.03 Section 4.1.8.1 MSG_REPORT_REQ.
> + https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56421.pdf
> +
> +What: /sys/kernel/config/tsm/report/$name/provider
> +Date: September, 2023
> +KernelVersion: v6.7
> +Contact: linux-coco@lists.linux.dev
> +Description:
> + (RO) A name for the format-specification of @outblob like
> + "sev_guest" [1] or "tdx_guest" [2] in the near term, or a
> + common standard format in the future.
> +
> + [1]: SEV Secure Nested Paging Firmware ABI Specification
> + Revision 1.55 Table 22
> + https://www.amd.com/content/dam/amd/en/documents/epyc-technical-docs/specifications/56860.pdf
> +
> + [2]: Intel® Trust Domain Extensions Data Center Attestation
> + Primitives : Quote Generation Library and Quote Verification
> + Library Revision 0.8 Appendix 4,5
> + https://download.01.org/intel-sgx/latest/dcap-latest/linux/docs/Intel_TDX_DCAP_Quoting_Library_API.pdf
> +
> +What: /sys/kernel/config/tsm/report/$name/generation
> +Date: September, 2023
> +KernelVersion: v6.7
> +Contact: linux-coco@lists.linux.dev
> +Description:
> + (RO) The value in this attribute increments each time @inblob or
> + any option is written. Userspace can detect conflicts by
> + checking generation before writing to any attribute and making
> + sure the number of writes matches expectations after reading
> + @outblob, or it can prevent conflicts by creating a report
> + instance per requesting context.
> +
> +What: /sys/kernel/config/tsm/report/$name/privlevel
> +Date: September, 2023
> +KernelVersion: v6.7
> +Contact: linux-coco@lists.linux.dev
> +Description:
> + (WO) Attribute is visible if a TSM implementation provider
> + supports the concept of attestation reports for TVMs running at
> + different privilege levels, like SEV-SNP "VMPL", specify the
> + privilege level via this attribute. The minimum acceptable
> + value is conveyed via @privlevel_floor and the maximum
> + acceptable value is TSM_PRIVLEVEL_MAX (3).
> +
> +What: /sys/kernel/config/tsm/report/$name/privlevel_floor
> +Date: September, 2023
> +KernelVersion: v6.7
> +Contact: linux-coco@lists.linux.dev
> +Description:
> + (RO) Indicates the minimum permissible value that can be written
> + to @privlevel.
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b19995690904..8acbeb029ba1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21889,6 +21889,14 @@ W: https://github.com/srcres258/linux-doc
> T: git git://github.com/srcres258/linux-doc.git doc-zh-tw
> F: Documentation/translations/zh_TW/
>
> +TRUSTED SECURITY MODULE (TSM) ATTESTATION REPORTS
> +M: Dan Williams <dan.j.williams@intel.com>
> +L: linux-coco@lists.linux.dev
> +S: Maintained
> +F: Documentation/ABI/testing/configfs-tsm
> +F: drivers/virt/coco/tsm.c
> +F: include/linux/tsm.h
> +
> TTY LAYER AND SERIAL DRIVERS
> M: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> M: Jiri Slaby <jirislaby@kernel.org>
> diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
> index fc5c64f04c4a..87d142c1f932 100644
> --- a/drivers/virt/coco/Kconfig
> +++ b/drivers/virt/coco/Kconfig
> @@ -2,6 +2,11 @@
> #
> # Confidential computing related collateral
> #
> +
> +config TSM_REPORTS
> + select CONFIGFS_FS
> + tristate
> +
> source "drivers/virt/coco/efi_secret/Kconfig"
>
> source "drivers/virt/coco/sev-guest/Kconfig"
> diff --git a/drivers/virt/coco/Makefile b/drivers/virt/coco/Makefile
> index 55302ef719ad..18c1aba5edb7 100644
> --- a/drivers/virt/coco/Makefile
> +++ b/drivers/virt/coco/Makefile
> @@ -2,6 +2,7 @@
> #
> # Confidential computing related collateral
> #
> +obj-$(CONFIG_TSM_REPORTS) += tsm.o
> obj-$(CONFIG_EFI_SECRET) += efi_secret/
> obj-$(CONFIG_SEV_GUEST) += sev-guest/
> obj-$(CONFIG_INTEL_TDX_GUEST) += tdx-guest/
> diff --git a/drivers/virt/coco/tsm.c b/drivers/virt/coco/tsm.c
> new file mode 100644
> index 000000000000..d1c2db83a8ca
> --- /dev/null
> +++ b/drivers/virt/coco/tsm.c
> @@ -0,0 +1,425 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright(c) 2023 Intel Corporation. All rights reserved. */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/tsm.h>
> +#include <linux/err.h>
> +#include <linux/slab.h>
> +#include <linux/rwsem.h>
> +#include <linux/string.h>
> +#include <linux/module.h>
> +#include <linux/cleanup.h>
> +#include <linux/configfs.h>
> +
> +static struct tsm_provider {
> + const struct tsm_ops *ops;
> + const struct config_item_type *type;
> + void *data;
> +} provider;
> +static DECLARE_RWSEM(tsm_rwsem);
> +
> +/**
> + * DOC: Trusted Security Module (TSM) Attestation Report Interface
> + *
> + * The TSM report interface is a common provider of blobs that facilitate
> + * attestation of a TVM (confidential computing guest) by an attestation
> + * service. A TSM report combines a user-defined blob (likely a public-key with
> + * a nonce for a key-exchange protocol) with a signed attestation report. That
> + * combined blob is then used to obtain secrets provided by an agent that can
> + * validate the attestation report. The expectation is that this interface is
> + * invoked infrequently, however configfs allows for multiple agents to
> + * own their own report generation instances to generate reports as
> + * often as needed.
> + *
> + * The attestation report format is TSM provider specific, when / if a standard
> + * materializes that can be published instead of the vendor layout. Until then
> + * the 'provider' attribute indicates the format of 'outblob', and optionally
> + * 'auxblob'.
> + */
> +
> +struct tsm_report_state {
> + struct tsm_report report;
> + unsigned long write_generation;
> + unsigned long read_generation;
> + struct config_item cfg;
> +};
> +
> +enum tsm_data_select {
> + TSM_REPORT,
> + TSM_CERTS,
> +};
> +
> +static struct tsm_report *to_tsm_report(struct config_item *cfg)
> +{
> + struct tsm_report_state *state =
> + container_of(cfg, struct tsm_report_state, cfg);
> +
> + return &state->report;
> +}
> +
> +static struct tsm_report_state *to_state(struct tsm_report *report)
> +{
> + return container_of(report, struct tsm_report_state, report);
> +}
> +
> +static int try_advance_write_generation(struct tsm_report *report)
> +{
> + struct tsm_report_state *state = to_state(report);
> +
> + lockdep_assert_held_write(&tsm_rwsem);
> +
> + /*
> + * Malicious or broken userspace has written enough times for
> + * read_generation == write_generation by modular arithmetic without an
> + * interim read. Stop accepting updates until the current report
> + * configuration is read.
> + */
> + if (state->write_generation == state->read_generation - 1)
> + return -EBUSY;
> + state->write_generation++;
> + return 0;
> +}
> +
> +static ssize_t tsm_report_privlevel_store(struct config_item *cfg,
> + const char *buf, size_t len)
> +{
> + struct tsm_report *report = to_tsm_report(cfg);
> + unsigned int val;
> + int rc;
> +
> + rc = kstrtouint(buf, 0, &val);
> + if (rc)
> + return rc;
> +
> + /*
> + * The valid privilege levels that a TSM might accept, if it accepts a
> + * privilege level setting at all, are a max of TSM_PRIVLEVEL_MAX (see
> + * SEV-SNP GHCB) and a minimum of a TSM selected floor value no less
> + * than 0.
> + */
> + if (provider.ops->privlevel_floor > val || val > TSM_PRIVLEVEL_MAX)
> + return -EINVAL;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + rc = try_advance_write_generation(report);
> + if (rc)
> + return rc;
> + report->desc.privlevel = val;
> +
> + return len;
> +}
> +CONFIGFS_ATTR_WO(tsm_report_, privlevel);
> +
> +static ssize_t tsm_report_privlevel_floor_show(struct config_item *cfg,
> + char *buf)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + return sysfs_emit(buf, "%u\n", provider.ops->privlevel_floor);
> +}
> +CONFIGFS_ATTR_RO(tsm_report_, privlevel_floor);
> +
> +static ssize_t tsm_report_inblob_write(struct config_item *cfg,
> + const void *buf, size_t count)
> +{
> + struct tsm_report *report = to_tsm_report(cfg);
> + int rc;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + rc = try_advance_write_generation(report);
> + if (rc)
> + return rc;
> +
> + report->desc.inblob_len = count;
> + memcpy(report->desc.inblob, buf, count);
> + return count;
> +}
> +CONFIGFS_BIN_ATTR_WO(tsm_report_, inblob, NULL, TSM_INBLOB_MAX);
> +
> +static ssize_t tsm_report_generation_show(struct config_item *cfg, char *buf)
> +{
> + struct tsm_report *report = to_tsm_report(cfg);
> + struct tsm_report_state *state = to_state(report);
> +
> + guard(rwsem_read)(&tsm_rwsem);
> + return sysfs_emit(buf, "%lu\n", state->write_generation);
> +}
> +CONFIGFS_ATTR_RO(tsm_report_, generation);
> +
> +static ssize_t tsm_report_provider_show(struct config_item *cfg, char *buf)
> +{
> + guard(rwsem_read)(&tsm_rwsem);
> + return sysfs_emit(buf, "%s\n", provider.ops->name);
> +}
> +CONFIGFS_ATTR_RO(tsm_report_, provider);
> +
> +static ssize_t __read_report(struct tsm_report *report, void *buf, size_t count,
> + enum tsm_data_select select)
> +{
> + loff_t offset = 0;
> + ssize_t len;
> + u8 *out;
> +
> + if (select == TSM_REPORT) {
> + out = report->outblob;
> + len = report->outblob_len;
> + } else {
> + out = report->auxblob;
> + len = report->auxblob_len;
> + }
> +
> + /*
> + * Recall that a NULL @buf is configfs requesting the size of
> + * the buffer.
> + */
> + if (!buf)
> + return len;
> + return memory_read_from_buffer(buf, count, &offset, out, len);
> +}
> +
> +static ssize_t read_cached_report(struct tsm_report *report, void *buf,
> + size_t count, enum tsm_data_select select)
> +{
> + struct tsm_report_state *state = to_state(report);
> +
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!report->desc.inblob_len)
> + return -EINVAL;
> +
> + /*
> + * A given TSM backend always fills in ->outblob regardless of
> + * whether the report includes an auxblob or not.
> + */
> + if (!report->outblob ||
> + state->read_generation != state->write_generation)
> + return -EWOULDBLOCK;
> +
> + return __read_report(report, buf, count, select);
> +}
> +
> +static ssize_t tsm_report_read(struct tsm_report *report, void *buf,
> + size_t count, enum tsm_data_select select)
> +{
> + struct tsm_report_state *state = to_state(report);
> + const struct tsm_ops *ops;
> + ssize_t rc;
> +
> + /* try to read from the existing report if present and valid... */
> + rc = read_cached_report(report, buf, count, select);
> + if (rc >= 0 || rc != -EWOULDBLOCK)
> + return rc;
> +
> + /* slow path, report may need to be regenerated... */
> + guard(rwsem_write)(&tsm_rwsem);
> + ops = provider.ops;
> + if (!ops)
> + return -ENOTTY;
> + if (!report->desc.inblob_len)
> + return -EINVAL;
> +
> + /* did another thread already generate this report? */
> + if (report->outblob &&
> + state->read_generation == state->write_generation)
> + goto out;
> +
> + kvfree(report->outblob);
> + kvfree(report->auxblob);
> + report->outblob = NULL;
> + report->auxblob = NULL;
> + rc = ops->report_new(report, provider.data);
> + if (rc < 0)
> + return rc;
> + state->read_generation = state->write_generation;
> +out:
> + return __read_report(report, buf, count, select);
> +}
> +
> +static ssize_t tsm_report_outblob_read(struct config_item *cfg, void *buf,
> + size_t count)
> +{
> + struct tsm_report *report = to_tsm_report(cfg);
> +
> + return tsm_report_read(report, buf, count, TSM_REPORT);
> +}
> +CONFIGFS_BIN_ATTR_RO(tsm_report_, outblob, NULL, TSM_OUTBLOB_MAX);
> +
> +static ssize_t tsm_report_auxblob_read(struct config_item *cfg, void *buf,
> + size_t count)
> +{
> + struct tsm_report *report = to_tsm_report(cfg);
> +
> + return tsm_report_read(report, buf, count, TSM_CERTS);
> +}
> +CONFIGFS_BIN_ATTR_RO(tsm_report_, auxblob, NULL, TSM_OUTBLOB_MAX);
> +
> +#define TSM_DEFAULT_ATTRS() \
> + &tsm_report_attr_generation, \
> + &tsm_report_attr_provider
> +
> +static struct configfs_attribute *tsm_report_attrs[] = {
> + TSM_DEFAULT_ATTRS(),
> + NULL,
> +};
> +
> +static struct configfs_attribute *tsm_report_extra_attrs[] = {
> + TSM_DEFAULT_ATTRS(),
> + &tsm_report_attr_privlevel,
> + &tsm_report_attr_privlevel_floor,
> + NULL,
> +};
> +
> +#define TSM_DEFAULT_BIN_ATTRS() \
> + &tsm_report_attr_inblob, \
> + &tsm_report_attr_outblob
> +
> +static struct configfs_bin_attribute *tsm_report_bin_attrs[] = {
> + TSM_DEFAULT_BIN_ATTRS(),
> + NULL,
> +};
> +
> +static struct configfs_bin_attribute *tsm_report_bin_extra_attrs[] = {
> + TSM_DEFAULT_BIN_ATTRS(),
> + &tsm_report_attr_auxblob,
> + NULL,
> +};
> +
> +static void tsm_report_item_release(struct config_item *cfg)
> +{
> + struct tsm_report *report = to_tsm_report(cfg);
> + struct tsm_report_state *state = to_state(report);
> +
> + kvfree(report->auxblob);
> + kvfree(report->outblob);
> + kfree(state);
> +}
> +
> +static struct configfs_item_operations tsm_report_item_ops = {
> + .release = tsm_report_item_release,
> +};
> +
> +const struct config_item_type tsm_report_default_type = {
> + .ct_owner = THIS_MODULE,
> + .ct_bin_attrs = tsm_report_bin_attrs,
> + .ct_attrs = tsm_report_attrs,
> + .ct_item_ops = &tsm_report_item_ops,
> +};
> +EXPORT_SYMBOL_GPL(tsm_report_default_type);
> +
> +const struct config_item_type tsm_report_extra_type = {
> + .ct_owner = THIS_MODULE,
> + .ct_bin_attrs = tsm_report_bin_extra_attrs,
> + .ct_attrs = tsm_report_extra_attrs,
> + .ct_item_ops = &tsm_report_item_ops,
> +};
> +EXPORT_SYMBOL_GPL(tsm_report_extra_type);
> +
> +static struct config_item *tsm_report_make_item(struct config_group *group,
> + const char *name)
> +{
> + struct tsm_report_state *state;
> +
> + guard(rwsem_read)(&tsm_rwsem);
> + if (!provider.ops)
> + return ERR_PTR(-ENXIO);
> +
> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> + if (!state)
> + return ERR_PTR(-ENOMEM);
> +
> + config_item_init_type_name(&state->cfg, name, provider.type);
> + return &state->cfg;
> +}
> +
> +static struct configfs_group_operations tsm_report_group_ops = {
> + .make_item = tsm_report_make_item,
> +};
> +
> +static const struct config_item_type tsm_reports_type = {
> + .ct_owner = THIS_MODULE,
> + .ct_group_ops = &tsm_report_group_ops,
> +};
> +
> +static const struct config_item_type tsm_root_group_type = {
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static struct configfs_subsystem tsm_configfs = {
> + .su_group = {
> + .cg_item = {
> + .ci_namebuf = "tsm",
> + .ci_type = &tsm_root_group_type,
> + },
> + },
> + .su_mutex = __MUTEX_INITIALIZER(tsm_configfs.su_mutex),
> +};
> +
> +int tsm_register(const struct tsm_ops *ops, void *priv,
> + const struct config_item_type *type)
> +{
> + const struct tsm_ops *conflict;
> +
> + if (!type)
> + type = &tsm_report_default_type;
> + if (!(type == &tsm_report_default_type || type == &tsm_report_extra_type))
> + return -EINVAL;
> +
> + guard(rwsem_write)(&tsm_rwsem);
> + conflict = provider.ops;
> + if (conflict) {
> + pr_err("\"%s\" ops already registered\n", conflict->name);
> + return -EBUSY;
> + }
> +
> + provider.ops = ops;
> + provider.data = priv;
> + provider.type = type;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(tsm_register);
> +
> +int tsm_unregister(const struct tsm_ops *ops)
> +{
> + guard(rwsem_write)(&tsm_rwsem);
> + if (ops != provider.ops)
> + return -EBUSY;
> + provider.ops = NULL;
> + provider.data = NULL;
> + provider.type = NULL;
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(tsm_unregister);
> +
> +static struct config_group *tsm_report_group;
> +
> +static int __init tsm_init(void)
> +{
> + struct config_group *root = &tsm_configfs.su_group;
> + struct config_group *tsm;
> + int rc;
> +
> + config_group_init(root);
> + rc = configfs_register_subsystem(&tsm_configfs);
> + if (rc)
> + return rc;
> +
> + tsm = configfs_register_default_group(root, "report",
> + &tsm_reports_type);
> + if (IS_ERR(tsm)) {
> + configfs_unregister_subsystem(&tsm_configfs);
> + return PTR_ERR(tsm);
> + }
> + tsm_report_group = tsm;
> +
> + return 0;
> +}
> +module_init(tsm_init);
> +
> +static void __exit tsm_exit(void)
> +{
> + configfs_unregister_default_group(tsm_report_group);
> + configfs_unregister_subsystem(&tsm_configfs);
> +}
> +module_exit(tsm_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Provide Trusted Security Module attestation reports via configfs");
> diff --git a/include/linux/tsm.h b/include/linux/tsm.h
> new file mode 100644
> index 000000000000..de8324a2223c
> --- /dev/null
> +++ b/include/linux/tsm.h
> @@ -0,0 +1,69 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __TSM_H
> +#define __TSM_H
> +
> +#include <linux/sizes.h>
> +#include <linux/types.h>
> +
> +#define TSM_INBLOB_MAX 64
> +#define TSM_OUTBLOB_MAX SZ_32K
> +
> +/*
> + * Privilege level is a nested permission concept to allow confidential
> + * guests to partition address space, 4-levels are supported.
> + */
> +#define TSM_PRIVLEVEL_MAX 3
> +
> +/**
> + * struct tsm_desc - option descriptor for generating tsm report blobs
> + * @privlevel: optional privilege level to associate with @outblob
> + * @inblob_len: sizeof @inblob
> + * @inblob: arbitrary input data
> + */
> +struct tsm_desc {
> + unsigned int privlevel;
> + size_t inblob_len;
> + u8 inblob[TSM_INBLOB_MAX];
> +};
> +
> +/**
> + * struct tsm_report - track state of report generation relative to options
> + * @desc: input parameters to @report_new()
> + * @outblob_len: sizeof(@outblob)
> + * @outblob: generated evidence to provider to the attestation agent
> + * @auxblob_len: sizeof(@auxblob)
> + * @auxblob: (optional) auxiliary data to the report (e.g. certificate data)
> + */
> +struct tsm_report {
> + struct tsm_desc desc;
> + size_t outblob_len;
> + u8 *outblob;
> + size_t auxblob_len;
> + u8 *auxblob;
> +};
> +
> +/**
> + * struct tsm_ops - attributes and operations for tsm instances
> + * @name: tsm id reflected in /sys/kernel/config/tsm/report/$report/provider
> + * @privlevel_floor: convey base privlevel for nested scenarios
> + * @report_new: Populate @report with the report blob and auxblob
> + * (optional), return 0 on successful population, or -errno otherwise
> + *
> + * Implementation specific ops, only one is expected to be registered at
> + * a time i.e. only one of "sev-guest", "tdx-guest", etc.
> + */
> +struct tsm_ops {
> + const char *name;
> + const unsigned int privlevel_floor;
> + int (*report_new)(struct tsm_report *report, void *data);
> +};
> +
> +extern const struct config_item_type tsm_report_default_type;
> +
> +/* publish @privlevel, @privlevel_floor, and @auxblob attributes */
> +extern const struct config_item_type tsm_report_extra_type;
> +
> +int tsm_register(const struct tsm_ops *ops, void *priv,
> + const struct config_item_type *type);
> +int tsm_unregister(const struct tsm_ops *ops);
> +#endif /* __TSM_H */
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v7 3/7] configfs-tsm: Introduce a shared ABI for attestation reports
2023-10-25 4:14 ` Qinkun Bao
@ 2023-10-25 20:06 ` Dan Williams
2023-10-27 1:30 ` Chong Cai
0 siblings, 1 reply; 16+ messages in thread
From: Dan Williams @ 2023-10-25 20:06 UTC (permalink / raw)
To: Qinkun Bao, Dan Williams
Cc: linux-coco, Kuppuswamy Sathyanarayanan, Dionna Amalie Glaze,
James Bottomley, Peter Gonda, Greg Kroah-Hartman, Samuel Ortiz,
Thomas Gleixner, Tom Lendacky, peterz, Dave Hansen, x86, kmoy,
Chong Cai
Qinkun Bao wrote:
[..]
> The patch raised some internal discussions at Google. Some have concerns
> about the potential collision problems. We have reached out to Dan. Dan
> mentioned that “configfs puts some onus on userspace to avoid collisions”.
> I have listed the changes in the userspace here to ensure that we have the
> correct understanding and that the complexity for the userspace is necessary.
I am disappointed this conversation continues to be revisited. Configfs
is an established ABI mechanism that requires well behaved userspace.
Configfs is not alone in this requirement which is why mechanisms like
D-Bus exist to broker a service for multiple clients. Configfs at least
allows each container to have its own broker which was a concession in
this effort to balance cross-container management complexity.
Yes, userspace care is necessary for configfs. On balance, I would
choose to spend my time building a userspace broker before entertaining
an idea to onboard more complication to the kernel.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/7] configfs-tsm: Introduce a shared ABI for attestation reports
2023-10-25 20:06 ` Dan Williams
@ 2023-10-27 1:30 ` Chong Cai
0 siblings, 0 replies; 16+ messages in thread
From: Chong Cai @ 2023-10-27 1:30 UTC (permalink / raw)
To: Dan Williams
Cc: Qinkun Bao, linux-coco, Kuppuswamy Sathyanarayanan,
Dionna Amalie Glaze, James Bottomley, Peter Gonda,
Greg Kroah-Hartman, Samuel Ortiz, Thomas Gleixner, Tom Lendacky,
peterz, Dave Hansen, x86, kmoy
Hi Dan,
Thank you for your effort to get this important patch ready to unblock
various user cases including ours.
Sorry for any confusion our reply may have caused. Just to clarify,
the goal of the previous reply from Qinkun was not to push back/delay
this patch. We have already tested the patch internally for both
SEV-SNP and TDX, and have no intention to add more complication to the
kernel.
We discussed the synchronization issue internally and with you and
others before, and we understand what is required from userspace to
adopt this patch, which is reasonable. The previous reply simply
serves the purpose for other users to be aware which user space care
they need to provide, since they may not be involved in the previous
discussions.
Chong
On Wed, Oct 25, 2023 at 1:06 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> Qinkun Bao wrote:
> [..]
> > The patch raised some internal discussions at Google. Some have concerns
> > about the potential collision problems. We have reached out to Dan. Dan
> > mentioned that “configfs puts some onus on userspace to avoid collisions”.
> > I have listed the changes in the userspace here to ensure that we have the
> > correct understanding and that the complexity for the userspace is necessary.
>
> I am disappointed this conversation continues to be revisited. Configfs
> is an established ABI mechanism that requires well behaved userspace.
> Configfs is not alone in this requirement which is why mechanisms like
> D-Bus exist to broker a service for multiple clients. Configfs at least
> allows each container to have its own broker which was a concession in
> this effort to balance cross-container management complexity.
>
> Yes, userspace care is necessary for configfs. On balance, I would
> choose to spend my time building a userspace broker before entertaining
> an idea to onboard more complication to the kernel.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 4/7] virt: sevguest: Prep for kernel internal get_ext_report()
2023-10-20 1:16 [PATCH v7 0/7] configfs-tsm: Attestation Report ABI Dan Williams
` (2 preceding siblings ...)
2023-10-20 1:16 ` [PATCH v7 3/7] configfs-tsm: Introduce a shared ABI for attestation reports Dan Williams
@ 2023-10-20 1:16 ` Dan Williams
2023-10-20 1:16 ` [PATCH v7 5/7] mm/slab: Add __free() support for kvfree Dan Williams
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Dan Williams @ 2023-10-20 1:16 UTC (permalink / raw)
To: linux-coco
Cc: Borislav Petkov, Tom Lendacky, Dionna Glaze,
Kuppuswamy Sathyanarayanan, Kuppuswamy Sathyanarayanan,
Tom Lendacky, peterz, dave.hansen, x86
In preparation for using the configs-tsm facility to convey attestation
blobs to userspace, switch to using the 'sockptr' api for copying
payloads to provided buffers where 'sockptr' handles user vs kernel
buffers.
While configfs-tsm is meant to replace existing confidential computing
ioctl() implementations for attestation report retrieval the old ioctl()
path needs to stick around for a deprecation period.
No behavior change intended.
Cc: Borislav Petkov <bp@alien8.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Dionna Glaze <dionnaglaze@google.com>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Tested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/virt/coco/sev-guest/sev-guest.c | 44 +++++++++++++++++++++++--------
1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/virt/coco/sev-guest/sev-guest.c b/drivers/virt/coco/sev-guest/sev-guest.c
index 5bee58ef5f1e..e5f8f115f4af 100644
--- a/drivers/virt/coco/sev-guest/sev-guest.c
+++ b/drivers/virt/coco/sev-guest/sev-guest.c
@@ -19,6 +19,7 @@
#include <crypto/aead.h>
#include <linux/scatterlist.h>
#include <linux/psp-sev.h>
+#include <linux/sockptr.h>
#include <uapi/linux/sev-guest.h>
#include <uapi/linux/psp-sev.h>
@@ -475,6 +476,11 @@ static int handle_guest_request(struct snp_guest_dev *snp_dev, u64 exit_code,
return 0;
}
+struct snp_req_resp {
+ sockptr_t req_data;
+ sockptr_t resp_data;
+};
+
static int get_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
{
struct snp_guest_crypto *crypto = snp_dev->crypto;
@@ -555,22 +561,25 @@ static int get_derived_key(struct snp_guest_dev *snp_dev, struct snp_guest_reque
return rc;
}
-static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg)
+static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_request_ioctl *arg,
+ struct snp_req_resp *io)
+
{
struct snp_ext_report_req *req = &snp_dev->req.ext_report;
struct snp_guest_crypto *crypto = snp_dev->crypto;
struct snp_report_resp *resp;
int ret, npages = 0, resp_len;
+ sockptr_t certs_address;
lockdep_assert_held(&snp_cmd_mutex);
- if (!arg->req_data || !arg->resp_data)
+ if (sockptr_is_null(io->req_data) || sockptr_is_null(io->resp_data))
return -EINVAL;
- if (copy_from_user(req, (void __user *)arg->req_data, sizeof(*req)))
+ if (copy_from_sockptr(req, io->req_data, sizeof(*req)))
return -EFAULT;
- /* userspace does not want certificate data */
+ /* caller does not want certificate data */
if (!req->certs_len || !req->certs_address)
goto cmd;
@@ -578,8 +587,13 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
!IS_ALIGNED(req->certs_len, PAGE_SIZE))
return -EINVAL;
- if (!access_ok((const void __user *)req->certs_address, req->certs_len))
- return -EFAULT;
+ if (sockptr_is_kernel(io->resp_data)) {
+ certs_address = KERNEL_SOCKPTR((void *)req->certs_address);
+ } else {
+ certs_address = USER_SOCKPTR((void __user *)req->certs_address);
+ if (!access_ok(certs_address.user, req->certs_len))
+ return -EFAULT;
+ }
/*
* Initialize the intermediate buffer with all zeros. This buffer
@@ -609,21 +623,19 @@ static int get_ext_report(struct snp_guest_dev *snp_dev, struct snp_guest_reques
if (arg->vmm_error == SNP_GUEST_VMM_ERR_INVALID_LEN) {
req->certs_len = snp_dev->input.data_npages << PAGE_SHIFT;
- if (copy_to_user((void __user *)arg->req_data, req, sizeof(*req)))
+ if (copy_to_sockptr(io->req_data, req, sizeof(*req)))
ret = -EFAULT;
}
if (ret)
goto e_free;
- if (npages &&
- copy_to_user((void __user *)req->certs_address, snp_dev->certs_data,
- req->certs_len)) {
+ if (npages && copy_to_sockptr(certs_address, snp_dev->certs_data, req->certs_len)) {
ret = -EFAULT;
goto e_free;
}
- if (copy_to_user((void __user *)arg->resp_data, resp, sizeof(*resp)))
+ if (copy_to_sockptr(io->resp_data, resp, sizeof(*resp)))
ret = -EFAULT;
e_free:
@@ -636,6 +648,7 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long
struct snp_guest_dev *snp_dev = to_snp_dev(file);
void __user *argp = (void __user *)arg;
struct snp_guest_request_ioctl input;
+ struct snp_req_resp io;
int ret = -ENOTTY;
if (copy_from_user(&input, argp, sizeof(input)))
@@ -664,7 +677,14 @@ static long snp_guest_ioctl(struct file *file, unsigned int ioctl, unsigned long
ret = get_derived_key(snp_dev, &input);
break;
case SNP_GET_EXT_REPORT:
- ret = get_ext_report(snp_dev, &input);
+ /*
+ * As get_ext_report() may be called from the ioctl() path and a
+ * kernel internal path (configfs-tsm), decorate the passed
+ * buffers as user pointers.
+ */
+ io.req_data = USER_SOCKPTR((void __user *)input.req_data);
+ io.resp_data = USER_SOCKPTR((void __user *)input.resp_data);
+ ret = get_ext_report(snp_dev, &input, &io);
break;
default:
break;
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v7 5/7] mm/slab: Add __free() support for kvfree
2023-10-20 1:16 [PATCH v7 0/7] configfs-tsm: Attestation Report ABI Dan Williams
` (3 preceding siblings ...)
2023-10-20 1:16 ` [PATCH v7 4/7] virt: sevguest: Prep for kernel internal get_ext_report() Dan Williams
@ 2023-10-20 1:16 ` Dan Williams
2023-10-20 1:17 ` [PATCH v7 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT Dan Williams
2023-10-20 1:17 ` [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS Dan Williams
6 siblings, 0 replies; 16+ messages in thread
From: Dan Williams @ 2023-10-20 1:16 UTC (permalink / raw)
To: linux-coco
Cc: Andrew Morton, Peter Zijlstra, Greg Kroah-Hartman, Pankaj Gupta,
Greg Kroah-Hartman, Kuppuswamy Sathyanarayanan,
Kuppuswamy Sathyanarayanan, Tom Lendacky, dave.hansen, x86
Allow for the declaration of variables that trigger kvfree() when they
go out of scope. The check for NULL and call to kvfree() can be elided
by the compiler in most cases, otherwise without the NULL check an
unnecessary call to kvfree() may be emitted. Peter proposed a comment
for this detail [1].
Link: http://lore.kernel.org/r/20230816103102.GF980931@hirez.programming.kicks-ass.net [1]
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Pankaj Gupta <pankaj.gupta@amd.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Tested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
include/linux/slab.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 8228d1276a2f..df4c2d45bb86 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -763,6 +763,8 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n, size_t size, gfp_t fla
extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t flags)
__realloc_size(3);
extern void kvfree(const void *addr);
+DEFINE_FREE(kvfree, void *, if (_T) kvfree(_T))
+
extern void kvfree_sensitive(const void *addr, size_t len);
unsigned int kmem_cache_size(struct kmem_cache *s);
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v7 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT
2023-10-20 1:16 [PATCH v7 0/7] configfs-tsm: Attestation Report ABI Dan Williams
` (4 preceding siblings ...)
2023-10-20 1:16 ` [PATCH v7 5/7] mm/slab: Add __free() support for kvfree Dan Williams
@ 2023-10-20 1:17 ` Dan Williams
2023-10-20 3:25 ` Alexey Kardashevskiy
2023-10-20 1:17 ` [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS Dan Williams
6 siblings, 1 reply; 16+ messages in thread
From: Dan Williams @ 2023-10-20 1:17 UTC (permalink / raw)
To: linux-coco
Cc: Borislav Petkov, Tom Lendacky, Dionna Glaze, Jeremi Piotrowski,
Kuppuswamy Sathyanarayanan, Alexey Kardashevskiy, Tom Lendacky,
peterz, dave.hansen, x86
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>
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;
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v7 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT
2023-10-20 1:17 ` [PATCH v7 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT Dan Williams
@ 2023-10-20 3:25 ` Alexey Kardashevskiy
0 siblings, 0 replies; 16+ messages in thread
From: Alexey Kardashevskiy @ 2023-10-20 3:25 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: Borislav Petkov, Tom Lendacky, Dionna Glaze, Jeremi Piotrowski,
Kuppuswamy Sathyanarayanan, peterz, dave.hansen, x86
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS
2023-10-20 1:16 [PATCH v7 0/7] configfs-tsm: Attestation Report ABI Dan Williams
` (5 preceding siblings ...)
2023-10-20 1:17 ` [PATCH v7 6/7] virt: sevguest: Add TSM_REPORTS support for SNP_GET_EXT_REPORT Dan Williams
@ 2023-10-20 1:17 ` Dan Williams
2023-10-24 17:08 ` Dan Williams
2023-12-21 1:50 ` Xiaoyao Li
6 siblings, 2 replies; 16+ messages in thread
From: Dan Williams @ 2023-10-20 1:17 UTC (permalink / raw)
To: linux-coco
Cc: Kuppuswamy Sathyanarayanan, Erdem Aktas,
Kuppuswamy Sathyanarayanan, Peter Gonda, Tom Lendacky, peterz,
dave.hansen, x86
From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
In TDX guest, the attestation process is used to verify the TDX guest
trustworthiness to other entities before provisioning secrets to the
guest. The first step in the attestation process is TDREPORT
generation, which involves getting the guest measurement data in the
format of TDREPORT, which is further used to validate the authenticity
of the TDX guest. TDREPORT by design is integrity-protected and can
only be verified on the local machine.
To support remote verification of the TDREPORT in a SGX-based
attestation, the TDREPORT needs to be sent to the SGX Quoting Enclave
(QE) to convert it to a remotely verifiable Quote. SGX QE by design can
only run outside of the TDX guest (i.e. in a host process or in a
normal VM) and guest can use communication channels like vsock or
TCP/IP to send the TDREPORT to the QE. But for security concerns, the
TDX guest may not support these communication channels. To handle such
cases, TDX defines a GetQuote hypercall which can be used by the guest
to request the host VMM to communicate with the SGX QE. More details
about GetQuote hypercall can be found in TDX Guest-Host Communication
Interface (GHCI) for Intel TDX 1.0, section titled
"TDG.VP.VMCALL<GetQuote>".
Trusted Security Module (TSM) [1] exposes a common ABI for Confidential
Computing Guest platforms to get the measurement data via ConfigFS.
Extend the TSM framework and add support to allow an attestation agent
to get the TDX Quote data (included usage example below).
report=/sys/kernel/config/tsm/report/report0
mkdir $report
dd if=/dev/urandom bs=64 count=1 > $report/inblob
hexdump -C $report/outblob
rmdir $report
GetQuote TDVMCALL requires TD guest pass a 4K aligned shared buffer
with TDREPORT data as input, which is further used by the VMM to copy
the TD Quote result after successful Quote generation. To create the
shared buffer, allocate a large enough memory and mark it shared using
set_memory_decrypted() in tdx_guest_init(). This buffer will be re-used
for GetQuote requests in the TDX TSM handler.
Although this method reserves a fixed chunk of memory for GetQuote
requests, such one time allocation can help avoid memory fragmentation
related allocation failures later in the uptime of the guest.
Since the Quote generation process is not time-critical or frequently
used, the current version uses a polling model for Quote requests and
it also does not support parallel GetQuote requests.
Link: https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/ [1]
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Reviewed-by: Erdem Aktas <erdemaktas@google.com>
Tested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Tested-by: Peter Gonda <pgonda@google.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
arch/x86/coco/tdx/tdx.c | 21 +++
arch/x86/include/asm/shared/tdx.h | 1
arch/x86/include/asm/tdx.h | 2
drivers/virt/coco/tdx-guest/Kconfig | 1
drivers/virt/coco/tdx-guest/tdx-guest.c | 229 +++++++++++++++++++++++++++++++
5 files changed, 253 insertions(+), 1 deletion(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index 1d6b863c42b0..752867b1d11b 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -104,6 +104,27 @@ int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport)
}
EXPORT_SYMBOL_GPL(tdx_mcall_get_report0);
+/**
+ * tdx_hcall_get_quote() - Wrapper to request TD Quote using GetQuote
+ * hypercall.
+ * @buf: Address of the directly mapped shared kernel buffer which
+ * contains TDREPORT. The same buffer will be used by VMM to
+ * store the generated TD Quote output.
+ * @size: size of the tdquote buffer (4KB-aligned).
+ *
+ * Refer to section titled "TDG.VP.VMCALL<GetQuote>" in the TDX GHCI
+ * v1.0 specification for more information on GetQuote hypercall.
+ * It is used in the TDX guest driver module to get the TD Quote.
+ *
+ * Return 0 on success or error code on failure.
+ */
+u64 tdx_hcall_get_quote(u8 *buf, size_t size)
+{
+ /* Since buf is a shared memory, set the shared (decrypted) bits */
+ return _tdx_hypercall(TDVMCALL_GET_QUOTE, cc_mkdec(virt_to_phys(buf)), size, 0, 0);
+}
+EXPORT_SYMBOL_GPL(tdx_hcall_get_quote);
+
static void __noreturn tdx_panic(const char *msg)
{
struct tdx_hypercall_args args = {
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index 7513b3bb69b7..9eab19950f39 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -22,6 +22,7 @@
/* TDX hypercall Leaf IDs */
#define TDVMCALL_MAP_GPA 0x10001
+#define TDVMCALL_GET_QUOTE 0x10002
#define TDVMCALL_REPORT_FATAL_ERROR 0x10003
#ifndef __ASSEMBLY__
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 603e6d1e9d4a..ebd1cda4875f 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -52,6 +52,8 @@ bool tdx_early_handle_ve(struct pt_regs *regs);
int tdx_mcall_get_report0(u8 *reportdata, u8 *tdreport);
+u64 tdx_hcall_get_quote(u8 *buf, size_t size);
+
#else
static inline void tdx_early_init(void) { };
diff --git a/drivers/virt/coco/tdx-guest/Kconfig b/drivers/virt/coco/tdx-guest/Kconfig
index 14246fc2fb02..22dd59e19431 100644
--- a/drivers/virt/coco/tdx-guest/Kconfig
+++ b/drivers/virt/coco/tdx-guest/Kconfig
@@ -1,6 +1,7 @@
config TDX_GUEST_DRIVER
tristate "TDX Guest driver"
depends on INTEL_TDX_GUEST
+ select TSM_REPORTS
help
The driver provides userspace interface to communicate with
the TDX module to request the TDX guest details like attestation
diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
index 5e44a0fa69bd..1253bf76b570 100644
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c
+++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
@@ -12,12 +12,60 @@
#include <linux/mod_devicetable.h>
#include <linux/string.h>
#include <linux/uaccess.h>
+#include <linux/set_memory.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/tsm.h>
+#include <linux/sizes.h>
#include <uapi/linux/tdx-guest.h>
#include <asm/cpu_device_id.h>
#include <asm/tdx.h>
+/*
+ * Intel's SGX QE implementation generally uses Quote size less
+ * than 8K (2K Quote data + ~5K of certificate blob).
+ */
+#define GET_QUOTE_BUF_SIZE SZ_8K
+
+#define GET_QUOTE_CMD_VER 1
+
+/* TDX GetQuote status codes */
+#define GET_QUOTE_SUCCESS 0
+#define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
+
+/* struct tdx_quote_buf: Format of Quote request buffer.
+ * @version: Quote format version, filled by TD.
+ * @status: Status code of Quote request, filled by VMM.
+ * @in_len: Length of TDREPORT, filled by TD.
+ * @out_len: Length of Quote data, filled by VMM.
+ * @data: Quote data on output or TDREPORT on input.
+ *
+ * More details of Quote request buffer can be found in TDX
+ * Guest-Host Communication Interface (GHCI) for Intel TDX 1.0,
+ * section titled "TDG.VP.VMCALL<GetQuote>"
+ */
+struct tdx_quote_buf {
+ u64 version;
+ u64 status;
+ u32 in_len;
+ u32 out_len;
+ u8 data[];
+};
+
+/* Quote data buffer */
+static void *quote_data;
+
+/* Lock to streamline quote requests */
+static DEFINE_MUTEX(quote_lock);
+
+/*
+ * GetQuote request timeout in seconds. Expect that 30 seconds
+ * is enough time for QE to respond to any Quote requests.
+ */
+static u32 getquote_timeout = 30;
+
static long tdx_get_report0(struct tdx_report_req __user *req)
{
u8 *reportdata, *tdreport;
@@ -53,6 +101,154 @@ static long tdx_get_report0(struct tdx_report_req __user *req)
return ret;
}
+static void free_quote_buf(void *buf)
+{
+ size_t len = PAGE_ALIGN(GET_QUOTE_BUF_SIZE);
+ unsigned int count = len >> PAGE_SHIFT;
+
+ if (set_memory_encrypted((unsigned long)buf, count)) {
+ pr_err("Failed to restore encryption mask for Quote buffer, leak it\n");
+ return;
+ }
+
+ free_pages_exact(buf, len);
+}
+
+static void *alloc_quote_buf(void)
+{
+ size_t len = PAGE_ALIGN(GET_QUOTE_BUF_SIZE);
+ unsigned int count = len >> PAGE_SHIFT;
+ void *addr;
+
+ addr = alloc_pages_exact(len, GFP_KERNEL | __GFP_ZERO);
+ if (!addr)
+ return NULL;
+
+ if (set_memory_decrypted((unsigned long)addr, count)) {
+ free_pages_exact(addr, len);
+ return NULL;
+ }
+
+ return addr;
+}
+
+/*
+ * wait_for_quote_completion() - Wait for Quote request completion
+ * @quote_buf: Address of Quote buffer.
+ * @timeout: Timeout in seconds to wait for the Quote generation.
+ *
+ * As per TDX GHCI v1.0 specification, sec titled "TDG.VP.VMCALL<GetQuote>",
+ * the status field in the Quote buffer will be set to GET_QUOTE_IN_FLIGHT
+ * while VMM processes the GetQuote request, and will change it to success
+ * or error code after processing is complete. So wait till the status
+ * changes from GET_QUOTE_IN_FLIGHT or the request being timed out.
+ */
+static int wait_for_quote_completion(struct tdx_quote_buf *quote_buf, u32 timeout)
+{
+ int i = 0;
+
+ /*
+ * Quote requests usually take a few seconds to complete, so waking up
+ * once per second to recheck the status is fine for this use case.
+ */
+ while (quote_buf->status == GET_QUOTE_IN_FLIGHT && i++ < timeout) {
+ if (msleep_interruptible(MSEC_PER_SEC))
+ return -EINTR;
+ }
+
+ return (i == timeout) ? -ETIMEDOUT : 0;
+}
+
+static int tdx_report_new(struct tsm_report *report, void *data)
+{
+ u8 *buf, *reportdata = NULL, *tdreport = NULL;
+ struct tdx_quote_buf *quote_buf = quote_data;
+ struct tsm_desc *desc = &report->desc;
+ int ret;
+ u64 err;
+
+ /* TODO: switch to guard(mutex_intr) */
+ if (mutex_lock_interruptible("e_lock))
+ return -EINTR;
+
+ /*
+ * If the previous request is timedout or interrupted, and the
+ * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
+ * VMM), don't permit any new request.
+ */
+ if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
+ ret = -EBUSY;
+ goto done;
+ }
+
+ if (desc->inblob_len != TDX_REPORTDATA_LEN) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ reportdata = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
+ if (!reportdata) {
+ ret = -ENOMEM;
+ goto done;
+ }
+
+ tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
+ if (!tdreport) {
+ ret = -ENOMEM;
+ goto done;
+ }
+
+ memcpy(reportdata, desc->inblob, desc->inblob_len);
+
+ /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
+ ret = tdx_mcall_get_report0(reportdata, tdreport);
+ if (ret) {
+ pr_err("GetReport call failed\n");
+ goto done;
+ }
+
+ memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
+
+ /* Update Quote buffer header */
+ quote_buf->version = GET_QUOTE_CMD_VER;
+ quote_buf->in_len = TDX_REPORT_LEN;
+
+ memcpy(quote_buf->data, tdreport, TDX_REPORT_LEN);
+
+ err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
+ if (err) {
+ pr_err("GetQuote hypercall failed, status:%llx\n", err);
+ ret = -EIO;
+ goto done;
+ }
+
+ ret = wait_for_quote_completion(quote_buf, getquote_timeout);
+ if (ret) {
+ pr_err("GetQuote request timedout\n");
+ goto done;
+ }
+
+ buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto done;
+ }
+
+ report->outblob = buf;
+ report->outblob_len = quote_buf->out_len;
+
+ /*
+ * TODO: parse the PEM-formatted cert chain out of the quote buffer when
+ * provided
+ */
+done:
+ mutex_unlock("e_lock);
+ kfree(reportdata);
+ kfree(tdreport);
+
+ return ret;
+}
+
static long tdx_guest_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -82,17 +278,48 @@ static const struct x86_cpu_id tdx_guest_ids[] = {
};
MODULE_DEVICE_TABLE(x86cpu, tdx_guest_ids);
+static const struct tsm_ops tdx_tsm_ops = {
+ .name = KBUILD_MODNAME,
+ .report_new = tdx_report_new,
+};
+
static int __init tdx_guest_init(void)
{
+ int ret;
+
if (!x86_match_cpu(tdx_guest_ids))
return -ENODEV;
- return misc_register(&tdx_misc_dev);
+ ret = misc_register(&tdx_misc_dev);
+ if (ret)
+ return ret;
+
+ quote_data = alloc_quote_buf();
+ if (!quote_data) {
+ pr_err("Failed to allocate Quote buffer\n");
+ ret = -ENOMEM;
+ goto free_misc;
+ }
+
+ ret = tsm_register(&tdx_tsm_ops, NULL, NULL);
+ if (ret)
+ goto free_quote;
+
+ return 0;
+
+free_quote:
+ free_quote_buf(quote_data);
+free_misc:
+ misc_deregister(&tdx_misc_dev);
+
+ return ret;
}
module_init(tdx_guest_init);
static void __exit tdx_guest_exit(void)
{
+ tsm_unregister(&tdx_tsm_ops);
+ free_quote_buf(quote_data);
misc_deregister(&tdx_misc_dev);
}
module_exit(tdx_guest_exit);
^ permalink raw reply related [flat|nested] 16+ messages in thread* RE: [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS
2023-10-20 1:17 ` [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS Dan Williams
@ 2023-10-24 17:08 ` Dan Williams
2023-12-21 1:50 ` Xiaoyao Li
1 sibling, 0 replies; 16+ messages in thread
From: Dan Williams @ 2023-10-24 17:08 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: Kuppuswamy Sathyanarayanan, Erdem Aktas, Peter Gonda,
Tom Lendacky, peterz, dave.hansen, x86
Dan Williams wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>
> In TDX guest, the attestation process is used to verify the TDX guest
> trustworthiness to other entities before provisioning secrets to the
> guest. The first step in the attestation process is TDREPORT
> generation, which involves getting the guest measurement data in the
> format of TDREPORT, which is further used to validate the authenticity
> of the TDX guest. TDREPORT by design is integrity-protected and can
> only be verified on the local machine.
>
[..]
>
> Link: https://lore.kernel.org/lkml/169342399185.3934343.3035845348326944519.stgit@dwillia2-xfh.jf.intel.com/ [1]
> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Reviewed-by: Erdem Aktas <erdemaktas@google.com>
> Tested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> Tested-by: Peter Gonda <pgonda@google.com>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> arch/x86/coco/tdx/tdx.c | 21 +++
> arch/x86/include/asm/shared/tdx.h | 1
> arch/x86/include/asm/tdx.h | 2
Given the proximity to v6.7 window opening I am going to go ahead and
throw this into linux-next for some soak time. If an x86 maintainer
wants to take it, or otherwise wants me to drop it out linux-next just
holler.
Per above there are limited arch/x86 touches in this series, and it
merges cleanly with tip/master.
I will also copy x86@ on the linux-next branch add request.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS
2023-10-20 1:17 ` [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS Dan Williams
2023-10-24 17:08 ` Dan Williams
@ 2023-12-21 1:50 ` Xiaoyao Li
2024-01-08 2:55 ` Kuppuswamy Sathyanarayanan
1 sibling, 1 reply; 16+ messages in thread
From: Xiaoyao Li @ 2023-12-21 1:50 UTC (permalink / raw)
To: Dan Williams, linux-coco
Cc: Kuppuswamy Sathyanarayanan, Erdem Aktas, Peter Gonda,
Tom Lendacky, peterz, dave.hansen, x86
On 10/20/2023 9:17 AM, Dan Williams wrote:
> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
...
> +static int tdx_report_new(struct tsm_report *report, void *data)
> +{
> + u8 *buf, *reportdata = NULL, *tdreport = NULL;
> + struct tdx_quote_buf *quote_buf = quote_data;
> + struct tsm_desc *desc = &report->desc;
> + int ret;
> + u64 err;
> +
> + /* TODO: switch to guard(mutex_intr) */
> + if (mutex_lock_interruptible("e_lock))
> + return -EINTR;
> +
> + /*
> + * If the previous request is timedout or interrupted, and the
> + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
> + * VMM), don't permit any new request.
> + */
> + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
> + ret = -EBUSY;
> + goto done;
> + }
> +
> + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
> + ret = -EINVAL;
> + goto done;
> + }
> +
> + reportdata = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
> + if (!reportdata) {
> + ret = -ENOMEM;
> + goto done;
> + }
> +
> + tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
> + if (!tdreport) {
> + ret = -ENOMEM;
> + goto done;
> + }
> +
> + memcpy(reportdata, desc->inblob, desc->inblob_len);
> +
> + /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
> + ret = tdx_mcall_get_report0(reportdata, tdreport);
> + if (ret) {
> + pr_err("GetReport call failed\n");
> + goto done;
> + }
> +
> + memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
> +
> + /* Update Quote buffer header */
> + quote_buf->version = GET_QUOTE_CMD_VER;
> + quote_buf->in_len = TDX_REPORT_LEN;
> +
> + memcpy(quote_buf->data, tdreport, TDX_REPORT_LEN);
> +
> + err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
> + if (err) {
> + pr_err("GetQuote hypercall failed, status:%llx\n", err);
> + ret = -EIO;
> + goto done;
> + }
> +
> + ret = wait_for_quote_completion(quote_buf, getquote_timeout);
> + if (ret) {
> + pr_err("GetQuote request timedout\n");
> + goto done;
> + }
Sorry that I didn't check the previous discussion and don't know if it
is by design or not:
Why don't check the quote_buf->status? If it indicates errors, we should
return some error code instead, right?
> + buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto done;
> + }
> +
> + report->outblob = buf;
> + report->outblob_len = quote_buf->out_len;
> +
> + /*
> + * TODO: parse the PEM-formatted cert chain out of the quote buffer when
> + * provided
> + */
> +done:
> + mutex_unlock("e_lock);
> + kfree(reportdata);
> + kfree(tdreport);
> +
> + return ret;
> +}
> +
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS
2023-12-21 1:50 ` Xiaoyao Li
@ 2024-01-08 2:55 ` Kuppuswamy Sathyanarayanan
2024-01-09 2:13 ` Xiaoyao Li
0 siblings, 1 reply; 16+ messages in thread
From: Kuppuswamy Sathyanarayanan @ 2024-01-08 2:55 UTC (permalink / raw)
To: Xiaoyao Li, Dan Williams, linux-coco
Cc: Erdem Aktas, Peter Gonda, Tom Lendacky, peterz, dave.hansen, x86
On 12/20/2023 5:50 PM, Xiaoyao Li wrote:
> On 10/20/2023 9:17 AM, Dan Williams wrote:
>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> ...
>> +static int tdx_report_new(struct tsm_report *report, void *data)
>> +{
>> + u8 *buf, *reportdata = NULL, *tdreport = NULL;
>> + struct tdx_quote_buf *quote_buf = quote_data;
>> + struct tsm_desc *desc = &report->desc;
>> + int ret;
>> + u64 err;
>> +
>> + /* TODO: switch to guard(mutex_intr) */
>> + if (mutex_lock_interruptible("e_lock))
>> + return -EINTR;
>> +
>> + /*
>> + * If the previous request is timedout or interrupted, and the
>> + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
>> + * VMM), don't permit any new request.
>> + */
>> + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
>> + ret = -EBUSY;
>> + goto done;
>> + }
>> +
>> + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
>> + ret = -EINVAL;
>> + goto done;
>> + }
>> +
>> + reportdata = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
>> + if (!reportdata) {
>> + ret = -ENOMEM;
>> + goto done;
>> + }
>> +
>> + tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
>> + if (!tdreport) {
>> + ret = -ENOMEM;
>> + goto done;
>> + }
>> +
>> + memcpy(reportdata, desc->inblob, desc->inblob_len);
>> +
>> + /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
>> + ret = tdx_mcall_get_report0(reportdata, tdreport);
>> + if (ret) {
>> + pr_err("GetReport call failed\n");
>> + goto done;
>> + }
>> +
>> + memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
>> +
>> + /* Update Quote buffer header */
>> + quote_buf->version = GET_QUOTE_CMD_VER;
>> + quote_buf->in_len = TDX_REPORT_LEN;
>> +
>> + memcpy(quote_buf->data, tdreport, TDX_REPORT_LEN);
>> +
>> + err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
>> + if (err) {
>> + pr_err("GetQuote hypercall failed, status:%llx\n", err);
>> + ret = -EIO;
>> + goto done;
>> + }
>> +
>> + ret = wait_for_quote_completion(quote_buf, getquote_timeout);
>> + if (ret) {
>> + pr_err("GetQuote request timedout\n");
>> + goto done;
>> + }
>
> Sorry that I didn't check the previous discussion and don't know if it is by design or not:
>
> Why don't check the quote_buf->status? If it indicates errors, we should return some error code instead, right?
For the failed request, outblob_len will be zero and the empty output can be
treated as failed request. But I agree that it makes sense to return error
for the failed request. I can submit a patch for it. Something like below:
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c
+++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
@@ -33,6 +33,8 @@
/* TDX GetQuote status codes */
#define GET_QUOTE_SUCCESS 0
+#define GET_QUOTE_ERROR 0x8000000000000000
+#define GET_QUOTE_SERVICE_UNAVAILABLE 0x8000000000000001
#define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
/* struct tdx_quote_buf: Format of Quote request buffer.
@@ -228,6 +230,12 @@ static int tdx_report_new(struct tsm_report *report, void *data)
goto done;
}
+ if (quote_buf->status != GET_QUOTE_SUCCESS) {
+ pr_err("GetQuote request failed, ret %llx\n", quote_buf->status);
+ ret = -EIO;
+ goto done;
+ }
+
>
>> + buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
>> + if (!buf) {
>> + ret = -ENOMEM;
>> + goto done;
>> + }
>> +
>> + report->outblob = buf;
>> + report->outblob_len = quote_buf->out_len;
>> +
>> + /*
>> + * TODO: parse the PEM-formatted cert chain out of the quote buffer when
>> + * provided
>> + */
>> +done:
>> + mutex_unlock("e_lock);
>> + kfree(reportdata);
>> + kfree(tdreport);
>> +
>> + return ret;
>> +}
>> +
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v7 7/7] virt: tdx-guest: Add Quote generation support using TSM_REPORTS
2024-01-08 2:55 ` Kuppuswamy Sathyanarayanan
@ 2024-01-09 2:13 ` Xiaoyao Li
0 siblings, 0 replies; 16+ messages in thread
From: Xiaoyao Li @ 2024-01-09 2:13 UTC (permalink / raw)
To: Kuppuswamy Sathyanarayanan, Dan Williams, linux-coco
Cc: Erdem Aktas, Peter Gonda, Tom Lendacky, peterz, dave.hansen, x86
On 1/8/2024 10:55 AM, Kuppuswamy Sathyanarayanan wrote:
>
>
> On 12/20/2023 5:50 PM, Xiaoyao Li wrote:
>> On 10/20/2023 9:17 AM, Dan Williams wrote:
>>> From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>> ...
>>> +static int tdx_report_new(struct tsm_report *report, void *data)
>>> +{
>>> + u8 *buf, *reportdata = NULL, *tdreport = NULL;
>>> + struct tdx_quote_buf *quote_buf = quote_data;
>>> + struct tsm_desc *desc = &report->desc;
>>> + int ret;
>>> + u64 err;
>>> +
>>> + /* TODO: switch to guard(mutex_intr) */
>>> + if (mutex_lock_interruptible("e_lock))
>>> + return -EINTR;
>>> +
>>> + /*
>>> + * If the previous request is timedout or interrupted, and the
>>> + * Quote buf status is still in GET_QUOTE_IN_FLIGHT (owned by
>>> + * VMM), don't permit any new request.
>>> + */
>>> + if (quote_buf->status == GET_QUOTE_IN_FLIGHT) {
>>> + ret = -EBUSY;
>>> + goto done;
>>> + }
>>> +
>>> + if (desc->inblob_len != TDX_REPORTDATA_LEN) {
>>> + ret = -EINVAL;
>>> + goto done;
>>> + }
>>> +
>>> + reportdata = kmalloc(TDX_REPORTDATA_LEN, GFP_KERNEL);
>>> + if (!reportdata) {
>>> + ret = -ENOMEM;
>>> + goto done;
>>> + }
>>> +
>>> + tdreport = kzalloc(TDX_REPORT_LEN, GFP_KERNEL);
>>> + if (!tdreport) {
>>> + ret = -ENOMEM;
>>> + goto done;
>>> + }
>>> +
>>> + memcpy(reportdata, desc->inblob, desc->inblob_len);
>>> +
>>> + /* Generate TDREPORT0 using "TDG.MR.REPORT" TDCALL */
>>> + ret = tdx_mcall_get_report0(reportdata, tdreport);
>>> + if (ret) {
>>> + pr_err("GetReport call failed\n");
>>> + goto done;
>>> + }
>>> +
>>> + memset(quote_data, 0, GET_QUOTE_BUF_SIZE);
>>> +
>>> + /* Update Quote buffer header */
>>> + quote_buf->version = GET_QUOTE_CMD_VER;
>>> + quote_buf->in_len = TDX_REPORT_LEN;
>>> +
>>> + memcpy(quote_buf->data, tdreport, TDX_REPORT_LEN);
>>> +
>>> + err = tdx_hcall_get_quote(quote_data, GET_QUOTE_BUF_SIZE);
>>> + if (err) {
>>> + pr_err("GetQuote hypercall failed, status:%llx\n", err);
>>> + ret = -EIO;
>>> + goto done;
>>> + }
>>> +
>>> + ret = wait_for_quote_completion(quote_buf, getquote_timeout);
>>> + if (ret) {
>>> + pr_err("GetQuote request timedout\n");
>>> + goto done;
>>> + }
>>
>> Sorry that I didn't check the previous discussion and don't know if it is by design or not:
>>
>> Why don't check the quote_buf->status? If it indicates errors, we should return some error code instead, right?
>
> For the failed request, outblob_len will be zero and the empty output can be
> treated as failed request. But I agree that it makes sense to return error
> for the failed request. I can submit a patch for it. Something like below:
>
> --- a/drivers/virt/coco/tdx-guest/tdx-guest.c
> +++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
> @@ -33,6 +33,8 @@
>
> /* TDX GetQuote status codes */
> #define GET_QUOTE_SUCCESS 0
> +#define GET_QUOTE_ERROR 0x8000000000000000
> +#define GET_QUOTE_SERVICE_UNAVAILABLE 0x8000000000000001
they get defined but not used.
> #define GET_QUOTE_IN_FLIGHT 0xffffffffffffffff
>
> /* struct tdx_quote_buf: Format of Quote request buffer.
> @@ -228,6 +230,12 @@ static int tdx_report_new(struct tsm_report *report, void *data)
> goto done;
> }
>
> + if (quote_buf->status != GET_QUOTE_SUCCESS) {
> + pr_err("GetQuote request failed, ret %llx\n", quote_buf->status);
> + ret = -EIO;
> + goto done;
> + }
> +
Besides above, looks good to me.
>
>>
>>> + buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
>>> + if (!buf) {
>>> + ret = -ENOMEM;
>>> + goto done;
>>> + }
>>> +
>>> + report->outblob = buf;
>>> + report->outblob_len = quote_buf->out_len;
>>> +
>>> + /*
>>> + * TODO: parse the PEM-formatted cert chain out of the quote buffer when
>>> + * provided
>>> + */
>>> +done:
>>> + mutex_unlock("e_lock);
>>> + kfree(reportdata);
>>> + kfree(tdreport);
>>> +
>>> + return ret;
>>> +}
>>> +
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread