Support userspace hypercalls for TDX

Add a new prctl option to enable/disable user-level hypercalls when running in a confidential VM. Add support for checking this flag on VMCALL #VE for TDX and transfer control to a hypervisor vendor-specific handler.

Signed-off-by Tim Merrifield <[email protected]>

---
 arch/x86/coco/tdx/tdx.c            | 18 ++++++++++++++++++
 arch/x86/include/asm/thread_info.h |  2 ++
 arch/x86/include/asm/x86_init.h    |  1 +
 arch/x86/include/uapi/asm/prctl.h  |  3 +++
 arch/x86/kernel/process.c          | 20 ++++++++++++++++++++
 5 files changed, 44 insertions(+)

arch/x86/coco/tdx/tdx.c +4 -0

index ef8ec2425998..23111e4c1f91 100644
@@ -239,6 +239,7 @@ static int ve_instr_len(struct ve_info *ve)
 	case EXIT_REASON_MSR_WRITE:
 	case EXIT_REASON_CPUID:
 	case EXIT_REASON_IO_INSTRUCTION:
+	case EXIT_REASON_VMCALL:
 		/* It is safe to use ve->instr_len for #VE due instructions */
 		return ve->instr_len;
 	case EXIT_REASON_EPT_VIOLATION:

@@ -635,6 +636,21 @@ void tdx_get_ve_info(struct ve_info *ve)
 	ve->instr_info  = upper_32_bits(args.r10);
 }
 
+/*
+ * Handle user-initiated, hypervisor-specific VMCALLs.
+ */

arch/x86/coco/tdx/tdx.c +4 -0

+}
+
 /*
  * Handle the user initiated #VE.
  *

@@ -646,6 +662,8 @@ static int virt_exception_user(struct pt_regs *regs, struct ve_info *ve)
 	switch (ve->exit_reason) {
 	case EXIT_REASON_CPUID:
 		return handle_cpuid(regs, ve);
+	case EXIT_REASON_VMCALL:
+		return handle_user_vmcall(regs, ve);
 	default:
 		pr_warn("Unexpected #VE: %lld\n", ve->exit_reason);
 		return -EIO;

arch/x86/include/asm/thread_info.h +0 -0

index 12da7dfd5ef1..9f69a26a5e68 100644

arch/x86/include/asm/thread_info.h +0 -0

 /* flags to check in __switch_to() */
 #define _TIF_WORK_CTXSW_BASE					\

arch/x86/include/asm/x86_init.h +1 -0

index 213cf5379a5a..52975bedd33e 100644
@@ -282,6 +282,7 @@ struct x86_hyper_runtime {
 	void (*sev_es_hcall_prepare)(struct ghcb *ghcb, struct pt_regs *regs);
 	bool (*sev_es_hcall_finish)(struct ghcb *ghcb, struct pt_regs *regs);
 	bool (*is_private_mmio)(u64 addr);
+	bool (*tdx_hcall)(struct pt_regs *regs);
 };
 
 /**

arch/x86/include/uapi/asm/prctl.h +0 -0

index 384e2cc6ac19..7fa289a1815b 100644

arch/x86/include/uapi/asm/prctl.h +0 -0

 #define ARCH_XCOMP_TILECFG		17
 #define ARCH_XCOMP_TILEDATA		18
 

arch/x86/kernel/process.c +5 -0

index 1b3d417cd6c4..16f8ab6cde2e 100644
@@ -1039,6 +1039,21 @@ unsigned long __get_wchan(struct task_struct *p)
 	return addr;
 }
 
+static int get_coco_user_hcall_mode(void)
+{
+	return !test_thread_flag(TIF_COCO_USER_HCALL);
+}
+

arch/x86/kernel/process.c +5 -0

 long do_arch_prctl_common(int option, unsigned long arg2)
 {
 	switch (option) {

@@ -1052,6 +1067,11 @@ long do_arch_prctl_common(int option, unsigned long arg2)
 	case ARCH_GET_XCOMP_GUEST_PERM:
 	case ARCH_REQ_XCOMP_GUEST_PERM:
 		return fpu_xstate_prctl(option, arg2);
+	case ARCH_GET_COCO_USER_HCALL:
+		return get_coco_user_hcall_mode();
+	case ARCH_SET_COCO_USER_HCALL:
+		return set_coco_user_hcall_mode(arg2);
+
 	}
 
 	return -EINVAL;

Signature · 1 line

2.40.1

Hm. Per-thread flag is odd. I think it should be per-process.

Thanks for the review, Kirill.

Quoted 1 line · Kirill A . Shutemov

Hm. Per-thread flag is odd. I think it should be per-process.

This is the only point I might need some clarification on. I agree there doesn't seem to be much value in allowing per-thread control, but I don't see any precedence for setting per-process flags through arch_prctl or similar interfaces. Am I missing something?

arch/x86/kernel/cpu/vmware.c +17 -0

+
+	vmware_init_tdx_args(&args, true, regs->cx, regs->bx,
+			     regs->dx, regs->si, regs->di, regs->bp);
+	__tdx_hypercall(&args);
+	regs->ax = args.r12;
+	regs->bx = args.rbx;
+	regs->cx = args.r13;
+	regs->dx = args.rdx;
+	regs->si = args.rsi;
+	regs->di = args.rdi;
+	regs->bp = args.r14;
+
+	return true;
+}
 #endif
 
 #ifdef CONFIG_AMD_MEM_ENCRYPT

@@ -586,4 +612,7 @@ const __initconst struct hypervisor_x86 x86_hyper_vmware = {
 	.runtime.sev_es_hcall_prepare	= vmware_sev_es_hcall_prepare,
 	.runtime.sev_es_hcall_finish	= vmware_sev_es_hcall_finish,
 #endif
+#ifdef CONFIG_INTEL_TDX_GUEST
+	.runtime.tdx_hcall              = vmware_tdx_user_hcall,
+#endif
 };

Signature · 1 line

2.40.1

VMCALL and VMMCALL instructions are used by x86 guests to request services from the host VMM. Both VMCALL and VMMCALL are not restricted to CPL 0. This allows userspace software like open-vm-tools to communicate directly with the VMM.

This patchset introduces a new x86 process control flag to address this concern. By setting the TIF_COCO_USER_HCALL thread information flag, the process opts in to user-level hypercalls.

Could we please be frank and transparent about what you actually want here and how you expect this mechanism to be used?

This inheritance model seems more suited to wrapping a tiny helper app around an existing binary, a la:

	prctl(ARCH_SET_COCO_USER_HCALL);
	execve("/existing/binary/that/i/surely/did/not/audit", ...);

... as opposed to something that you set in new versions of open-vm-tools after an extensive audit and a bug fixing campaign to clean up everything that the audit found.

VMCALL and VMMCALL instructions are used by x86 guests to request services from the host VMM. Both VMCALL and VMMCALL are not restricted to CPL 0. This allows userspace software like open-vm-tools to communicate directly with the VMM.

In the context of confidential VMs, direct communication with the host may violate the security model. Existing binaries that make use of hypercalls and are not hardened against malicious hypervisors can become a possible attack surface. For this reason, user-level VMCALLs are not currently forwarded to the host on TDX VMs. This breaks any user-level software that use these instructions.

But if user-level software is aware of the risks and has been hardened to address any known violations of the security model, then it seems reasonable to allow hypercalls from this process to proceed.

And how are we to ascertain the software using these hooks is deemed secure? What security risks are there for the kernel if a malicious userspace process asks for these rights?

The kernel must assume malice on the part of userspace.