x86/sev: Use explicit user-or-kernel accessors for #VC memory operands

5d64749b · Linus Torvalds <[email protected]> · 2026-10-19 19:24:25 -10:00

vc_read_mem() and vc_write_mem() access the memory operand of an
emulated MOVS or INS/OUTS.  That address may be a kernel address or a
user one, and the access must be a single MOV rather than a string
instruction, since the operand can itself be MMIO and take a nested #VC.
The code used __get_user() and __put_user() for this, on the grounds
that they do no pointer checks.  That is a side effect of how x86
happens to implement them, not something they are for, and they are
going away.

Replace them with two local helpers that say what this is: a "user or
kernel" access with fault handling.  They enable user access by hand
rather than through user_access_begin(), whose range check would reject
a kernel address, and use unsafe_get_user() and unsafe_put_user() for
the single MOV and its exception table entry.  The read side uses
__uaccess_begin_nospec(), keeping the lfence that __get_user() used to
provide.

The old comment also claimed this always runs in atomic context, so the
page fault handler would not try to handle a fault.  That holds when the
exception comes from kernel mode, where the handler runs as an NMI.  It
does not hold when it comes from user mode, where the handler only has
interrupts disabled and so is not in_atomic().  A user-mode "rep insb"
into a not-yet-faulted buffer therefore went into do_user_addr_fault()
with interrupts off, and was only stopped by its legacy WARN_ON_ONCE()
on a clear IF flag.  The helpers now disable page faults explicitly, so
the fault is refused up front and forwarded to the instruction as a #PF,
which is what was intended.

Signed-off-by: Linus Torvalds <[email protected]>

Commit: 5d64749b7df20ec3f0820fc94eefecfbf10f197e
Parents: 7170bd79

Changes

1 files changed · +94 −111

File	Status	Add	Del	Content
arch/x86/coco/sev/vc-handle.c	modified	+94	−111	View file

arch/x86/coco/sev/vc-handle.c

View file

diff --git a/arch/x86/coco/sev/vc-handle.c b/arch/x86/coco/sev/vc-handle.c
index 96b62b4..f25d302 100644
--- a/arch/x86/coco/sev/vc-handle.c
+++ b/arch/x86/coco/sev/vc-handle.c
@@ -194,75 +194,113 @@ static enum es_result vc_decode_insn(struct es_em_ctxt *ctxt)
 		return __vc_decode_kern_insn(ctxt);
 }
 
-static enum es_result vc_write_mem(struct es_em_ctxt *ctxt,
-				   char *dst, char *buf, size_t size)
+/*
+ * The memory operand of an emulated string instruction can be a user or a
+ * kernel address. MOVS to and from MMIO is only emulated for the kernel,
+ * but the kernel may be copying to or from user memory at the time, and
+ * INS/OUTS are emulated for both. So these are "user or kernel" accesses
+ * with fault handling: a single plain MOV of the given size, with an
+ * exception table entry to catch a fault. User access is enabled by hand
+ * so that a user address can be reached, without the range check of
+ * user_access_begin(), which would reject a kernel address.
+ *
+ * It must be a single MOV and not a string instruction, because the address
+ * can itself be MMIO. That takes a nested #VC, and a nested MOVS would nest
+ * forever.
+ *
+ * Page faults are disabled explicitly. A #VC from kernel mode runs as an NMI
+ * and is atomic anyway, but one from user mode only has interrupts disabled,
+ * which the page fault handler does not treat as atomic. A fault here is
+ * not handled: it is reported back as a #PF for the instruction that took the
+ * #VC, and gets handled there.
+ *
+ * The caller has already checked that size is 1, 2, 4 or 8.
+ */
+static bool vc_get_mem(void *buf, const void *addr, size_t size)
 {
-	unsigned long error_code = X86_PF_PROT | X86_PF_WRITE;
+	union { u8 d1; u16 d2; u32 d4; u64 d8; } val;
 
-	/*
-	 * This function uses __put_user() independent of whether kernel or user
-	 * memory is accessed. This works fine because __put_user() does no
-	 * sanity checks of the pointer being accessed. All that it does is
-	 * to report when the access failed.
-	 *
-	 * Also, this function runs in atomic context, so __put_user() is not
-	 * allowed to sleep. The page-fault handler detects that it is running
-	 * in atomic context and will not try to take mmap_sem and handle the
-	 * fault, so additional pagefault_enable()/disable() calls are not
-	 * needed.
-	 *
-	 * The access can't be done via copy_to_user() here because
-	 * vc_write_mem() must not use string instructions to access unsafe
-	 * memory. The reason is that MOVS is emulated by the #VC handler by
-	 * splitting the move up into a read and a write and taking a nested #VC
-	 * exception on whatever of them is the MMIO access. Using string
-	 * instructions here would cause infinite nesting.
-	 */
+	guard(pagefault)();
+	__uaccess_begin_nospec();
 	switch (size) {
-	case 1: {
-		u8 d1;
-		u8 __user *target = (u8 __user *)dst;
-
-		memcpy(&d1, buf, 1);
-		if (__put_user(d1, target))
-			goto fault;
+	case 1:
+		unsafe_get_user(val.d1, (const u8 __user __force *)addr, efault);
 		break;
-	}
-	case 2: {
-		u16 d2;
-		u16 __user *target = (u16 __user *)dst;
-
-		memcpy(&d2, buf, 2);
-		if (__put_user(d2, target))
-			goto fault;
+	case 2:
+		unsafe_get_user(val.d2, (const u16 __user __force *)addr, efault);
+		break;
+	case 4:
+		unsafe_get_user(val.d4, (const u32 __user __force *)addr, efault);
+		break;
+	case 8:
+		unsafe_get_user(val.d8, (const u64 __user __force *)addr, efault);
 		break;
 	}
-	case 4: {
-		u32 d4;
-		u32 __user *target = (u32 __user *)dst;
+	__uaccess_end();
 
-		memcpy(&d4, buf, 4);
-		if (__put_user(d4, target))
-			goto fault;
+	memcpy(buf, &val, size);
+	return true;
+
+efault:
+	__uaccess_end();
+	return false;
+}
+
+static bool vc_put_mem(void *addr, const void *buf, size_t size)
+{
+	union { u8 d1; u16 d2; u32 d4; u64 d8; } val;
+
+	memcpy(&val, buf, size);
+
+	guard(pagefault)();
+	__uaccess_begin();
+	switch (size) {
+	case 1:
+		unsafe_put_user(val.d1, (u8 __user __force *)addr, efault);
+		break;
+	case 2:
+		unsafe_put_user(val.d2, (u16 __user __force *)addr, efault);
+		break;
+	case 4:
+		unsafe_put_user(val.d4, (u32 __user __force *)addr, efault);
+		break;
+	case 8:
+		unsafe_put_user(val.d8, (u64 __user __force *)addr, efault);
 		break;
 	}
-	case 8: {
-		u64 d8;
-		u64 __user *target = (u64 __user *)dst;
+	__uaccess_end();
+	return true;
 
-		memcpy(&d8, buf, 8);
-		if (__put_user(d8, target))
-			goto fault;
-		break;
+efault:
+	__uaccess_end();
+	return false;
+}
+
+static bool vc_mem_size_ok(size_t size)
+{
+	switch (size) {
+	case 1:
+	case 2:
+	case 4:
+	case 8:
+		return true;
 	}
-	default:
+	return false;
+}
+
+static enum es_result vc_write_mem(struct es_em_ctxt *ctxt,
+				   char *dst, char *buf, size_t size)
+{
+	unsigned long error_code = X86_PF_PROT | X86_PF_WRITE;
+
+	if (!vc_mem_size_ok(size)) {
 		WARN_ONCE(1, "%s: Invalid size: %zu\n", __func__, size);
 		return ES_UNSUPPORTED;
 	}
 
-	return ES_OK;
+	if (vc_put_mem(dst, buf, size))
+		return ES_OK;
 
-fault:
 	if (user_mode(ctxt->regs))
 		error_code |= X86_PF_USER;
 
@@ -278,69 +316,14 @@ static enum es_result vc_read_mem(struct es_em_ctxt *ctxt,
 {
 	unsigned long error_code = X86_PF_PROT;
 
-	/*
-	 * This function uses __get_user() independent of whether kernel or user
-	 * memory is accessed. This works fine because __get_user() does no
-	 * sanity checks of the pointer being accessed. All that it does is
-	 * to report when the access failed.
-	 *
-	 * Also, this function runs in atomic context, so __get_user() is not
-	 * allowed to sleep. The page-fault handler detects that it is running
-	 * in atomic context and will not try to take mmap_sem and handle the
-	 * fault, so additional pagefault_enable()/disable() calls are not
-	 * needed.
-	 *
-	 * The access can't be done via copy_from_user() here because
-	 * vc_read_mem() must not use string instructions to access unsafe
-	 * memory. The reason is that MOVS is emulated by the #VC handler by
-	 * splitting the move up into a read and a write and taking a nested #VC
-	 * exception on whatever of them is the MMIO access. Using string
-	 * instructions here would cause infinite nesting.
-	 */
-	switch (size) {
-	case 1: {
-		u8 d1;
-		u8 __user *s = (u8 __user *)src;
-
-		if (__get_user(d1, s))
-			goto fault;
-		memcpy(buf, &d1, 1);
-		break;
-	}
-	case 2: {
-		u16 d2;
-		u16 __user *s = (u16 __user *)src;
-
-		if (__get_user(d2, s))
-			goto fault;
-		memcpy(buf, &d2, 2);
-		break;
-	}
-	case 4: {
-		u32 d4;
-		u32 __user *s = (u32 __user *)src;
-
-		if (__get_user(d4, s))
-			goto fault;
-		memcpy(buf, &d4, 4);
-		break;
-	}
-	case 8: {
-		u64 d8;
-		u64 __user *s = (u64 __user *)src;
-		if (__get_user(d8, s))
-			goto fault;
-		memcpy(buf, &d8, 8);
-		break;
-	}
-	default:
+	if (!vc_mem_size_ok(size)) {
 		WARN_ONCE(1, "%s: Invalid size: %zu\n", __func__, size);
 		return ES_UNSUPPORTED;
 	}
 
-	return ES_OK;
+	if (vc_get_mem(buf, src, size))
+		return ES_OK;
 
-fault:
 	if (user_mode(ctxt->regs))
 		error_code |= X86_PF_USER;
 