Thread (27 messages) 27 messages, 8 authors, 2021-07-15

Re: [PATCH v2 5/6] platform/x86: intel_tdx_attest: Add TDX Guest attestation interface driver

From: Andy Lutomirski <luto@kernel.org>
Date: 2021-07-08 22:21:34
Also in: bpf, lkml, platform-driver-x86

On 7/7/21 1:42 PM, Kuppuswamy Sathyanarayanan wrote:
quoted hunk ↗ jump to hunk
The interaction with the TDX module is like a RPM protocol here. There
are several operations (get tdreport, get quote) that need to input a
blob, and then output another blob. It was considered to use a sysfs
interface for this, but it doesn't fit well into the standard sysfs
model for configuring values. It would be possible to do read/write on
files, but it would need multiple file descriptors, which would be
somewhat messy. ioctls seems to be the best fitting and simplest model
here. There is one ioctl per operation, that takes the input blob and
returns the output blob, and as well as auxiliary ioctls to return the
blob lengths. The ioctls are documented in the header file. 

Reviewed-by: Tony Luck <tony.luck@intel.com>
Reviewed-by: Andi Kleen <redacted>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 drivers/platform/x86/Kconfig            |   9 ++
 drivers/platform/x86/Makefile           |   1 +
 drivers/platform/x86/intel_tdx_attest.c | 171 ++++++++++++++++++++++++
 include/uapi/misc/tdx.h                 |  37 +++++
 4 files changed, 218 insertions(+)
 create mode 100644 drivers/platform/x86/intel_tdx_attest.c
 create mode 100644 include/uapi/misc/tdx.h
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 60592fb88e7a..7d01c473aef6 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1301,6 +1301,15 @@ config INTEL_SCU_IPC_UTIL
 	  low level access for debug work and updating the firmware. Say
 	  N unless you will be doing this on an Intel MID platform.
 
+config INTEL_TDX_ATTESTATION
+	tristate "Intel TDX attestation driver"
+	depends on INTEL_TDX_GUEST
+	help
+	  The TDX attestation driver provides IOCTL or MMAP interfaces to
+	  the user to request TDREPORT from the TDX module or request quote
+	  from VMM. It is mainly used to get secure disk decryption keys from
+	  the key server.
What's the MMAP interface
quoted hunk ↗ jump to hunk
+
 config INTEL_TELEMETRY
 	tristate "Intel SoC Telemetry Driver"
 	depends on X86_64
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index dcc8cdb95b4d..83439990ae47 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -138,6 +138,7 @@ obj-$(CONFIG_INTEL_SCU_PCI)		+= intel_scu_pcidrv.o
 obj-$(CONFIG_INTEL_SCU_PLATFORM)	+= intel_scu_pltdrv.o
 obj-$(CONFIG_INTEL_SCU_WDT)		+= intel_scu_wdt.o
 obj-$(CONFIG_INTEL_SCU_IPC_UTIL)	+= intel_scu_ipcutil.o
+obj-$(CONFIG_INTEL_TDX_ATTESTATION)	+= intel_tdx_attest.o
 obj-$(CONFIG_INTEL_TELEMETRY)		+= intel_telemetry_core.o \
 					   intel_telemetry_pltdrv.o \
 					   intel_telemetry_debugfs.o
diff --git a/drivers/platform/x86/intel_tdx_attest.c b/drivers/platform/x86/intel_tdx_attest.c
new file mode 100644
index 000000000000..a0225d053851
--- /dev/null
+++ b/drivers/platform/x86/intel_tdx_attest.c
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * intel_tdx_attest.c - TDX guest attestation interface driver.
+ *
+ * Implements user interface to trigger attestation process and
+ * read the TD Quote result.
+ *
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * Author:
+ *     Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
+ */
+
+#define pr_fmt(fmt) "x86/tdx: attest: " fmt
+
+#include <linux/module.h>
+#include <linux/miscdevice.h>
+#include <linux/uaccess.h>
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/set_memory.h>
+#include <linux/io.h>
+#include <asm/apic.h>
+#include <asm/tdx.h>
+#include <asm/irq_vectors.h>
+#include <uapi/misc/tdx.h>
+
+#define VERSION				"1.0"
+
+/* Used in Quote memory allocation */
+#define QUOTE_SIZE			(2 * PAGE_SIZE)
+
+/* Mutex to synchronize attestation requests */
+static DEFINE_MUTEX(attestation_lock);
+/* Completion object to track attestation status */
+static DECLARE_COMPLETION(attestation_done);
+
+static void attestation_callback_handler(void)
+{
+	complete(&attestation_done);
+}
+
+static long tdg_attest_ioctl(struct file *file, unsigned int cmd,
+			     unsigned long arg)
+{
+	u64 data = virt_to_phys(file->private_data);
+	void __user *argp = (void __user *)arg;
+	u8 *reportdata;
+	long ret = 0;
+
+	mutex_lock(&attestation_lock);
+
+	reportdata = kzalloc(TDX_TDREPORT_LEN, GFP_KERNEL);
+	if (!reportdata) {
+		mutex_unlock(&attestation_lock);
+		return -ENOMEM;
+	}
+
+	switch (cmd) {
+	case TDX_CMD_GET_TDREPORT:
+		if (copy_from_user(reportdata, argp, TDX_REPORT_DATA_LEN)) {
+			ret = -EFAULT;
+			break;
+		}
This copies from user memory to reportdata.
+
+		/* Generate TDREPORT_STRUCT */
+		if (tdx_mcall_tdreport(data, virt_to_phys(reportdata))) {
+			ret = -EIO;
+			break;
+		}
This does the hypercall.
+
+		if (copy_to_user(argp, file->private_data, TDX_TDREPORT_LEN))
+			ret = -EFAULT;
This copies from private_data to user memory.  How did the report get to
private_data?
+		break;
+	case TDX_CMD_GEN_QUOTE:
+		if (copy_from_user(reportdata, argp, TDX_REPORT_DATA_LEN)) {
+			ret = -EFAULT;
+			break;
+		}
+
+		/* Generate TDREPORT_STRUCT */
+		if (tdx_mcall_tdreport(data, virt_to_phys(reportdata))) {
+			ret = -EIO;
+			break;
+		}
+
+		ret = set_memory_decrypted((unsigned long)file->private_data,
+					   1UL << get_order(QUOTE_SIZE));
+		if (ret)
+			break;
Now private_data is decrypted.  (And this operation is *expensive*.  Why
is it done at ioctl time?)
+
+		/* Submit GetQuote Request */
+		if (tdx_hcall_get_quote(data)) {
+			ret = -EIO;
+			goto done;
+		}
+
+		/* Wait for attestation completion */
+		wait_for_completion_interruptible(&attestation_done);
+
+		if (copy_to_user(argp, file->private_data, QUOTE_SIZE))
+			ret = -EFAULT;
+done:
+		ret = set_memory_encrypted((unsigned long)file->private_data,
+					   1UL << get_order(QUOTE_SIZE));
And this is, again, quite expensive.
+
+		break;
+	case TDX_CMD_GET_QUOTE_SIZE:
+		if (put_user(QUOTE_SIZE, (u64 __user *)argp))
+			ret = -EFAULT;
+
+		break;
+	default:
+		pr_err("cmd %d not supported\n", cmd);
+		break;
+	}
+
+	mutex_unlock(&attestation_lock);
+
+	kfree(reportdata);
+
+	return ret;
+}
+
+static int tdg_attest_open(struct inode *inode, struct file *file)
+{
+	/*
+	 * Currently tdg_event_notify_handler is only used in attestation
+	 * driver. But, WRITE_ONCE is used as benign data race notice.
+	 */
+	WRITE_ONCE(tdg_event_notify_handler, attestation_callback_handler);
+
+	file->private_data = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+						      get_order(QUOTE_SIZE));
This allocation has negligible cost compared to changing memory to
decrypted.

Shouldn't you allocate a buffer once at driver load time or even at boot
and just keep reusing it as needed?  You could have a few pages of
shared memory for the specific purposes of hypercalls, and you could
check them out and release them when you need some.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help