public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul
@ 2026-04-16 23:10 Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 1/6] KVM: SVM: Fix page overflow in sev_dbg_crypt() for ENCRYPT path Sean Christopherson
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-04-16 23:10 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Ashutosh Desai

Ashutosh's fix for a heap OOB/UAF bug in the debug {de,en}crypt code, now
with a selftest to detect the bug (and confirm the fix), and to validate the
functionality.

The rest of the patches completely rewrite the code.  When creating the
selftest, I did the silly thing of testing arbitrary offsets+sizes, and
couldn't trigger the true badness because the test failed long before it got
to the larger sizes.

Specifically (or, at least) the current code fails to handle cases where an
address and the size aren't naturally aligned.  E.g. when encrypting 9 bytes
at offset 8, KVM needs to _decrypt_ destination[31:0] into a temporary buffer,
buffer[31:0], then copy 9 bytes from source[8:0] to buffer[16:8], then encrypt
buffer[31:0] back into destination[31:0].  The current code only ever copies
16 bytes, and bizarrely uses a temporary buffer for the source as well.

A wholesale rewrite in a single patch isn't my first choice, but the existing
code obviously hasn't been tested, and it's so bizarre and unnecessarily
complex that I've zero confidence that an iterative cleanup would be a net
positive, especially given how many hours it would take.

The initial fix is 7.1 material, the rest (including the selftest, because it
won't pass), can wait for 7.2.

v1: https://lore.kernel.org/all/20260410050854.2463447-1-ashutoshdesai993@gmail.com

Ashutosh Desai (1):
  KVM: SVM: Fix page overflow in sev_dbg_crypt() for ENCRYPT path

Sean Christopherson (5):
  KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls
  KVM: SEV: Explicitly validate the dst buffer for debug operations
  KVM: SEV: Add helper function to pin/unpin a single page
  KVM: SEV: Rewrite logic to {de,en}crypt memory for debug
  KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers

 arch/x86/kvm/svm/sev.c                        | 423 +++++++++---------
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 tools/testing/selftests/kvm/include/x86/sev.h |  24 +
 .../testing/selftests/kvm/x86/sev_dbg_test.c  | 118 +++++
 4 files changed, 347 insertions(+), 219 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/sev_dbg_test.c


base-commit: 6b802031877a995456c528095c41d1948546bf45
-- 
2.54.0.rc1.513.gad8abe7a5a-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/6] KVM: SVM: Fix page overflow in sev_dbg_crypt() for ENCRYPT path
  2026-04-16 23:10 [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
@ 2026-04-16 23:10 ` Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls Sean Christopherson
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-04-16 23:10 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Ashutosh Desai

From: Ashutosh Desai <ashutoshdesai993@gmail.com>

In sev_dbg_crypt(), the per-iteration transfer length is bounded by
the source page offset (PAGE_SIZE - s_off) but not by the destination
page offset (PAGE_SIZE - d_off).  When d_off > s_off, the encrypt
path (__sev_dbg_encrypt_user) performs a read-modify-write using a
single-page intermediate buffer (dst_tpage):

  1. __sev_dbg_decrypt() expands the size to round_up(len + (d_off & 15), 16)
     before issuing the PSP command.  If len + (d_off & 15) > PAGE_SIZE,
     the PSP writes beyond the end of the 4096-byte dst_tpage allocation.

  2. The subsequent memcpy()/copy_from_user() into
     page_address(dst_tpage) + (d_off & 15) of 'len' bytes overflows
     by up to 15 bytes under the same condition.

Trigger example: s_off = 0, d_off = 1, debug.len = PAGE_SIZE -
the PSP is instructed to write round_up(4097, 16) = 4112 bytes to
a 4096-byte buffer.

Fix by also bounding len by (PAGE_SIZE - d_off), the same check that
sev_send_update_data() already performs for its single-page guest
region.

 ==================================================================
 BUG: KASAN: slab-use-after-free in sev_dbg_crypt+0x993/0xd10 [kvm_amd]
 Write of size 4095 at addr ff110062293bb009 by task sev_dbg_test/228214

 CPU: 96 UID: 0 PID: 228214 Comm: sev_dbg_test Tainted: G     U  W           7.0.0-smp--5ce9b0c48211-dbg #156 PREEMPTLAZY
 Tainted: [U]=USER, [W]=WARN
 Hardware name: Google Astoria/astoria, BIOS 0.20250817.1-0 08/25/2025
 Call Trace:
  <TASK>
  dump_stack_lvl+0x54/0x70
  print_report+0xbc/0x260
  kasan_report+0xa2/0xd0
  kasan_check_range+0x25f/0x2c0
  __asan_memcpy+0x40/0x70
  sev_dbg_crypt+0x993/0xd10 [kvm_amd]
  sev_mem_enc_ioctl+0x33c/0x450 [kvm_amd]
  kvm_vm_ioctl+0x65d/0x6d0 [kvm]
  __se_sys_ioctl+0xb2/0x100
  do_syscall_64+0xe8/0x870
  entry_SYSCALL_64_after_hwframe+0x4b/0x53
  </TASK>

 The buggy address belongs to the physical page:
 page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x7fe72b6a0 pfn:0x62293bb
 memcg:ff11000112827d82
 flags: 0x1400000000000000(node=1|zone=1)
 raw: 1400000000000000 0000000000000000 dead000000000122 0000000000000000
 raw: 00000007fe72b6a0 0000000000000000 00000001ffffffff ff11000112827d82
 page dumped because: kasan: bad access detected

 Memory state around the buggy address:
  ff110062293bbf00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  ff110062293bbf80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 >ff110062293bc000: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
                    ^
  ff110062293bc080: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
  ff110062293bc100: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ==================================================================
 Disabling lock debugging due to kernel taint

Fixes: 24f41fb23a39 ("KVM: SVM: Add support for SEV DEBUG_DECRYPT command")
Fixes: 7d1594f5d94b ("KVM: SVM: Add support for SEV DEBUG_ENCRYPT command")
Cc: stable@vger.kernel.org
Signed-off-by: Ashutosh Desai <ashutoshdesai993@gmail.com>
[sean: add sample KASAN splat, Fixes, and stable@]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c2126b3c3072..b9d7bd868e0b 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1396,6 +1396,7 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
 		s_off = vaddr & ~PAGE_MASK;
 		d_off = dst_vaddr & ~PAGE_MASK;
 		len = min_t(size_t, (PAGE_SIZE - s_off), size);
+		len = min_t(size_t, len, PAGE_SIZE - d_off);
 
 		if (dec)
 			ret = __sev_dbg_decrypt_user(kvm,
-- 
2.54.0.rc1.513.gad8abe7a5a-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls
  2026-04-16 23:10 [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 1/6] KVM: SVM: Fix page overflow in sev_dbg_crypt() for ENCRYPT path Sean Christopherson
@ 2026-04-16 23:10 ` Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 3/6] KVM: SEV: Explicitly validate the dst buffer for debug operations Sean Christopherson
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-04-16 23:10 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Ashutosh Desai

Add a selftest to verify KVM's handling of {de,en}crypt debug ioctls,
specifically focusing on edge cases around the chunk (16 bytes) and page
(4096) sizes, where KVM had multiple bugs.  E.g. KVM would fail to handle
small sizes that aren't naturally aligned and sized, would buffer overflow
if the destination was unaligned but the source was not, etc.

Attempt to strike a balance between an exhaustive test and a reasonable
runtime.  On a system with both SEV and SEV-ES support, the current runtime
is under 45 seconds.  Which isn't great, but it's tolerable, and it's not
obvious which of the combinations are "better" than the others.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 tools/testing/selftests/kvm/include/x86/sev.h |  24 ++++
 .../testing/selftests/kvm/x86/sev_dbg_test.c  | 118 ++++++++++++++++++
 3 files changed, 143 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/sev_dbg_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 9118a5a51b89..82fa943b9503 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -140,6 +140,7 @@ TEST_GEN_PROGS_x86 += x86/tsc_msrs_test
 TEST_GEN_PROGS_x86 += x86/vmx_pmu_caps_test
 TEST_GEN_PROGS_x86 += x86/xen_shinfo_test
 TEST_GEN_PROGS_x86 += x86/xen_vmcall_test
+TEST_GEN_PROGS_x86 += x86/sev_dbg_test
 TEST_GEN_PROGS_x86 += x86/sev_init2_tests
 TEST_GEN_PROGS_x86 += x86/sev_migrate_tests
 TEST_GEN_PROGS_x86 += x86/sev_smoke_test
diff --git a/tools/testing/selftests/kvm/include/x86/sev.h b/tools/testing/selftests/kvm/include/x86/sev.h
index 008b4169f5e2..669dbec927ac 100644
--- a/tools/testing/selftests/kvm/include/x86/sev.h
+++ b/tools/testing/selftests/kvm/include/x86/sev.h
@@ -144,4 +144,28 @@ static inline void snp_launch_update_data(struct kvm_vm *vm, vm_paddr_t gpa,
 	vm_sev_ioctl(vm, KVM_SEV_SNP_LAUNCH_UPDATE, &update_data);
 }
 
+static inline void sev_dbg_crypt_memory(struct kvm_vm *vm, unsigned int cmd,
+					void *dst, void *src, unsigned int len)
+{
+	struct kvm_sev_dbg dbg = {
+		.src_uaddr = (unsigned long)src,
+		.dst_uaddr = (unsigned long)dst,
+		.len = len,
+	};
+
+	vm_sev_ioctl(vm, cmd, &dbg);
+}
+
+static inline void sev_decrypt_memory(struct kvm_vm *vm, void *dst, void *src,
+				      unsigned int len)
+{
+	sev_dbg_crypt_memory(vm, KVM_SEV_DBG_DECRYPT, dst, src, len);
+}
+
+static inline void sev_encrypt_memory(struct kvm_vm *vm, void *dst, void *src,
+				      unsigned int len)
+{
+	sev_dbg_crypt_memory(vm, KVM_SEV_DBG_ENCRYPT, dst, src, len);
+}
+
 #endif /* SELFTEST_KVM_SEV_H */
diff --git a/tools/testing/selftests/kvm/x86/sev_dbg_test.c b/tools/testing/selftests/kvm/x86/sev_dbg_test.c
new file mode 100644
index 000000000000..55530cc2be05
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/sev_dbg_test.c
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <fcntl.h>
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "sev.h"
+
+#define BUFFER_SIZE	(PAGE_SIZE * 2)
+
+static u8 *data;
+static u8 src[BUFFER_SIZE] __aligned(PAGE_SIZE);
+static u8 dst[BUFFER_SIZE] __aligned(PAGE_SIZE);
+
+static void validate_dst(int i, int nr_bytes, u8 pattern)
+{
+	for ( ; i < nr_bytes; i++)
+		TEST_ASSERT(dst[i] == pattern,
+			    "Expected 0x%x at byte %u, got 0x%x",
+			    pattern, i, dst[i]);
+}
+
+static void validate_buffers(void)
+{
+	int i;
+
+	for (i = 0; i < BUFFER_SIZE; i++)
+		TEST_ASSERT(src[i] == dst[i],
+			    "Expected src[%u] (0x%x) == dst[%u] (0x%x)",
+			    i, src[i], i, dst[i]);
+}
+
+static void ____test_sev_dbg(struct kvm_vm *vm, int i, int j, int nr_bytes)
+{
+	u8 pattern = guest_random_u32(&guest_rng);
+
+	if (i + nr_bytes > BUFFER_SIZE || j + nr_bytes > BUFFER_SIZE)
+		return;
+
+	memset(&src[i], pattern, nr_bytes);
+	sev_encrypt_memory(vm, &data[j], &src[i], nr_bytes);
+	sev_decrypt_memory(vm, &dst[i], &data[j], nr_bytes);
+	validate_buffers();
+	validate_dst(i, nr_bytes, pattern);
+}
+
+static void __test_sev_dbg(struct kvm_vm *vm, int nr_bytes)
+{
+	/*
+	 * In a perfect world, all sizes at all combinations within the buffers
+	 * would be tested.  In reality, even this much testing is quite slow.
+	 * Target sizes and offsets around the chunk (16 bytes) and page (4096
+	 * bytes) sizes.
+	 */
+	int x[] = { 1, 8, 15, 16, 23 };
+	int p = PAGE_SIZE - 24;
+	int i, j;
+
+	____test_sev_dbg(vm, 0, 0, nr_bytes);
+
+	for (i = 0; i < ARRAY_SIZE(x); i++) {
+		for (j = 0; j < ARRAY_SIZE(x); j++) {
+			____test_sev_dbg(vm, x[i], x[j], nr_bytes);
+			____test_sev_dbg(vm, x[i], p + x[j], nr_bytes);
+			____test_sev_dbg(vm, p + x[i], x[j], nr_bytes);
+			____test_sev_dbg(vm, p + x[i], p + x[j], nr_bytes);
+		}
+	}
+}
+
+static void test_sev_dbg(uint32_t type, uint64_t policy)
+{
+	int sizes[] = { 1, 8, 15, 16, 17, 32, 33 };
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	int i;
+
+	if (!(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(type)))
+		return;
+
+	vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
+
+	data = addr_gva2hva(vm, vm_vaddr_alloc(vm, BUFFER_SIZE, KVM_UTIL_MIN_VADDR));
+	memset(data, 0xaa, BUFFER_SIZE);
+
+	vm_sev_launch(vm, policy, NULL);
+
+	sev_decrypt_memory(vm, dst, data, BUFFER_SIZE);
+	validate_dst(0, BUFFER_SIZE, 0xaa);
+
+	memset(src, 0x55, BUFFER_SIZE);
+	sev_encrypt_memory(vm, data, src, BUFFER_SIZE);
+	sev_decrypt_memory(vm, dst, data, BUFFER_SIZE);
+	validate_dst(0, BUFFER_SIZE, 0x55);
+
+	__test_sev_dbg(vm, PAGE_SIZE);
+
+	for (i = 0; i < ARRAY_SIZE(sizes); i++) {
+		__test_sev_dbg(vm, sizes[i]);
+		__test_sev_dbg(vm, PAGE_SIZE - sizes[i]);
+		__test_sev_dbg(vm, PAGE_SIZE + sizes[i]);
+		__test_sev_dbg(vm, BUFFER_SIZE - sizes[i]);
+	}
+
+	kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SEV));
+
+	/* Note, KVM doesn't support {de,en}crypt commands for SNP. */
+	test_sev_dbg(KVM_X86_SEV_VM, 0);
+	test_sev_dbg(KVM_X86_SEV_ES_VM, SEV_POLICY_ES);
+	return 0;
+}
-- 
2.54.0.rc1.513.gad8abe7a5a-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/6] KVM: SEV: Explicitly validate the dst buffer for debug operations
  2026-04-16 23:10 [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 1/6] KVM: SVM: Fix page overflow in sev_dbg_crypt() for ENCRYPT path Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls Sean Christopherson
@ 2026-04-16 23:10 ` Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 4/6] KVM: SEV: Add helper function to pin/unpin a single page Sean Christopherson
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-04-16 23:10 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Ashutosh Desai

When encrypting/decrypting guest memory, explicitly check that the
destination is non-NULL and doesn't wrap instead of subtly relying on
sev_pin_memory() to perform the check.  This will allow adding and using
a more focused single-page pinning helper.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index b9d7bd868e0b..9d2044fd910d 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1357,9 +1357,11 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
 	if (copy_from_user(&debug, u64_to_user_ptr(argp->data), sizeof(debug)))
 		return -EFAULT;
 
-	if (!debug.len || debug.src_uaddr + debug.len < debug.src_uaddr)
+	if (!debug.len || !debug.src_uaddr || !debug.dst_uaddr)
 		return -EINVAL;
-	if (!debug.dst_uaddr)
+
+	if (debug.src_uaddr + debug.len < debug.src_uaddr ||
+	    debug.dst_uaddr + debug.len < debug.dst_uaddr)
 		return -EINVAL;
 
 	vaddr = debug.src_uaddr;
-- 
2.54.0.rc1.513.gad8abe7a5a-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 4/6] KVM: SEV: Add helper function to pin/unpin a single page
  2026-04-16 23:10 [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
                   ` (2 preceding siblings ...)
  2026-04-16 23:10 ` [PATCH v2 3/6] KVM: SEV: Explicitly validate the dst buffer for debug operations Sean Christopherson
@ 2026-04-16 23:10 ` Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 5/6] KVM: SEV: Rewrite logic to {de,en}crypt memory for debug Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 6/6] KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers Sean Christopherson
  5 siblings, 0 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-04-16 23:10 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Ashutosh Desai

Add helpers to pin/unpin a single page, and use it in all flows that pin
exactly one page.  None of the single-page users actually check that the
correct number of pages was pinned, which is functionally ok, but visually
jarring, especially in the decrypt/encrypt flow, which separately pins two
pages, but uses a single variable to track how pages were pinned each time.
Again, it's functionally ok since core mm guarantees exactly one page will
be pinned on success, but it's ugly.

Opportunistically use page_to_phys() instead of open coding an equivalent
via page_to_pfn().

Note, all users of the single-page helper pre-validate the address and
length, i.e. don't rely on the sanity check in sev_pin_memory().

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 145 +++++++++++++++++++++++++----------------
 1 file changed, 90 insertions(+), 55 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 9d2044fd910d..4146c91788fb 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -720,14 +720,50 @@ static int sev_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
+static int sev_check_pin_count(struct kvm *kvm, unsigned long npages)
+{
+	unsigned long total_npages, lock_limit;
+
+	total_npages = to_kvm_sev_info(kvm)->pages_locked + npages;
+	if (total_npages > totalram_pages())
+		return -EINVAL;
+
+	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+	if (total_npages > lock_limit && !capable(CAP_IPC_LOCK)) {
+		pr_err_ratelimited("SEV: %lu total pages would exceed the lock limit of %lu.\n",
+				   total_npages, lock_limit);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int sev_pin_user_pages(struct kvm *kvm, unsigned long addr, int npages,
+			      unsigned int gup_flags, struct page **pages)
+{
+	int npinned;
+
+	lockdep_assert_held(&kvm->lock);
+
+	npinned = pin_user_pages_fast(addr, npages, gup_flags, pages);
+	if (npinned != npages) {
+		if (npinned > 0)
+			unpin_user_pages(pages, npinned);
+		pr_err_ratelimited("SEV: Failure locking %u pages.\n", npages);
+		return -ENOMEM;
+	}
+
+	to_kvm_sev_info(kvm)->pages_locked += npages;
+	return 0;
+}
+
 static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr,
 				    unsigned long ulen, unsigned long *n,
 				    unsigned int flags)
 {
-	struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
-	unsigned long npages, total_npages, lock_limit;
+	unsigned long npages;
 	struct page **pages;
-	int npinned, ret;
+	int ret;
 
 	lockdep_assert_held(&kvm->lock);
 
@@ -743,16 +779,9 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr,
 	if (npages > INT_MAX)
 		return ERR_PTR(-EINVAL);
 
-	total_npages = sev->pages_locked + npages;
-	if (total_npages > totalram_pages())
-		return ERR_PTR(-EINVAL);
-
-	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
-	if (total_npages > lock_limit && !capable(CAP_IPC_LOCK)) {
-		pr_err("SEV: %lu total pages would exceed the lock limit of %lu.\n",
-		       total_npages, lock_limit);
-		return ERR_PTR(-ENOMEM);
-	}
+	ret = sev_check_pin_count(kvm, npages);
+	if (ret)
+		return ERR_PTR(ret);
 
 	/*
 	 * Don't WARN if the kernel (rightly) thinks the total size is absurd,
@@ -764,25 +793,14 @@ static struct page **sev_pin_memory(struct kvm *kvm, unsigned long uaddr,
 	if (!pages)
 		return ERR_PTR(-ENOMEM);
 
-	/* Pin the user virtual address. */
-	npinned = pin_user_pages_fast(uaddr, npages, flags, pages);
-	if (npinned != npages) {
-		pr_err("SEV: Failure locking %lu pages.\n", npages);
-		ret = -ENOMEM;
-		goto err;
+	ret = sev_pin_user_pages(kvm, uaddr, npages, flags, pages);
+	if (ret) {
+		kvfree(pages);
+		return ERR_PTR(ret);
 	}
 
 	*n = npages;
-	sev->pages_locked = total_npages;
-
 	return pages;
-
-err:
-	if (npinned > 0)
-		unpin_user_pages(pages, npinned);
-
-	kvfree(pages);
-	return ERR_PTR(ret);
 }
 
 static void sev_unpin_memory(struct kvm *kvm, struct page **pages,
@@ -793,6 +811,29 @@ static void sev_unpin_memory(struct kvm *kvm, struct page **pages,
 	to_kvm_sev_info(kvm)->pages_locked -= npages;
 }
 
+static struct page *sev_pin_page(struct kvm *kvm, unsigned long addr,
+				 unsigned int flags)
+{
+	struct page *page;
+	int r;
+
+	r = sev_check_pin_count(kvm, 1);
+	if (r)
+		return ERR_PTR(r);
+
+	r = sev_pin_user_pages(kvm, addr, 1, flags, &page);
+	if (r)
+		return ERR_PTR(r);
+
+	return page;
+}
+
+static void sev_unpin_page(struct kvm *kvm, struct page *page)
+{
+	unpin_user_pages(&page, 1);
+	to_kvm_sev_info(kvm)->pages_locked -= 1;
+}
+
 static void sev_clflush_pages(struct page *pages[], unsigned long npages)
 {
 	uint8_t *page_virtual;
@@ -1345,9 +1386,8 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
 {
 	unsigned long vaddr, vaddr_end, next_vaddr;
 	unsigned long dst_vaddr;
-	struct page **src_p, **dst_p;
+	struct page *src_p, *dst_p;
 	struct kvm_sev_dbg debug;
-	unsigned long n;
 	unsigned int size;
 	int ret;
 
@@ -1373,13 +1413,13 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
 		int len, s_off, d_off;
 
 		/* lock userspace source and destination page */
-		src_p = sev_pin_memory(kvm, vaddr & PAGE_MASK, PAGE_SIZE, &n, 0);
+		src_p = sev_pin_page(kvm, vaddr & PAGE_MASK, 0);
 		if (IS_ERR(src_p))
 			return PTR_ERR(src_p);
 
-		dst_p = sev_pin_memory(kvm, dst_vaddr & PAGE_MASK, PAGE_SIZE, &n, FOLL_WRITE);
+		dst_p = sev_pin_page(kvm, dst_vaddr & PAGE_MASK, FOLL_WRITE);
 		if (IS_ERR(dst_p)) {
-			sev_unpin_memory(kvm, src_p, n);
+			sev_unpin_page(kvm, src_p);
 			return PTR_ERR(dst_p);
 		}
 
@@ -1388,8 +1428,8 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
 		 * the pages; flush the destination too so that future accesses do not
 		 * see stale data.
 		 */
-		sev_clflush_pages(src_p, 1);
-		sev_clflush_pages(dst_p, 1);
+		sev_clflush_pages(&src_p, 1);
+		sev_clflush_pages(&dst_p, 1);
 
 		/*
 		 * Since user buffer may not be page aligned, calculate the
@@ -1402,20 +1442,20 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
 
 		if (dec)
 			ret = __sev_dbg_decrypt_user(kvm,
-						     __sme_page_pa(src_p[0]) + s_off,
+						     __sme_page_pa(src_p) + s_off,
 						     (void __user *)dst_vaddr,
-						     __sme_page_pa(dst_p[0]) + d_off,
+						     __sme_page_pa(dst_p) + d_off,
 						     len, &argp->error);
 		else
 			ret = __sev_dbg_encrypt_user(kvm,
-						     __sme_page_pa(src_p[0]) + s_off,
+						     __sme_page_pa(src_p) + s_off,
 						     (void __user *)vaddr,
-						     __sme_page_pa(dst_p[0]) + d_off,
+						     __sme_page_pa(dst_p) + d_off,
 						     (void __user *)dst_vaddr,
 						     len, &argp->error);
 
-		sev_unpin_memory(kvm, src_p, n);
-		sev_unpin_memory(kvm, dst_p, n);
+		sev_unpin_page(kvm, src_p);
+		sev_unpin_page(kvm, dst_p);
 
 		if (ret)
 			goto err;
@@ -1698,8 +1738,7 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	struct sev_data_send_update_data data;
 	struct kvm_sev_send_update_data params;
 	void *hdr, *trans_data;
-	struct page **guest_page;
-	unsigned long n;
+	struct page *guest_page;
 	int ret, offset;
 
 	if (!sev_guest(kvm))
@@ -1723,8 +1762,7 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 		return -EINVAL;
 
 	/* Pin guest memory */
-	guest_page = sev_pin_memory(kvm, params.guest_uaddr & PAGE_MASK,
-				    PAGE_SIZE, &n, 0);
+	guest_page = sev_pin_page(kvm, params.guest_uaddr & PAGE_MASK, 0);
 	if (IS_ERR(guest_page))
 		return PTR_ERR(guest_page);
 
@@ -1745,7 +1783,7 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	data.trans_len = params.trans_len;
 
 	/* The SEND_UPDATE_DATA command requires C-bit to be always set. */
-	data.guest_address = (page_to_pfn(guest_page[0]) << PAGE_SHIFT) + offset;
+	data.guest_address = page_to_phys(guest_page) + offset;
 	data.guest_address |= sev_me_mask;
 	data.guest_len = params.guest_len;
 	data.handle = to_kvm_sev_info(kvm)->handle;
@@ -1772,8 +1810,7 @@ static int sev_send_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 e_free_hdr:
 	kfree(hdr);
 e_unpin:
-	sev_unpin_memory(kvm, guest_page, n);
-
+	sev_unpin_page(kvm, guest_page);
 	return ret;
 }
 
@@ -1878,8 +1915,7 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	struct kvm_sev_receive_update_data params;
 	struct sev_data_receive_update_data data;
 	void *hdr = NULL, *trans = NULL;
-	struct page **guest_page;
-	unsigned long n;
+	struct page *guest_page;
 	int ret, offset;
 
 	if (!sev_guest(kvm))
@@ -1916,8 +1952,7 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	data.trans_len = params.trans_len;
 
 	/* Pin guest memory */
-	guest_page = sev_pin_memory(kvm, params.guest_uaddr & PAGE_MASK,
-				    PAGE_SIZE, &n, FOLL_WRITE);
+	guest_page = sev_pin_page(kvm, params.guest_uaddr & PAGE_MASK, FOLL_WRITE);
 	if (IS_ERR(guest_page)) {
 		ret = PTR_ERR(guest_page);
 		goto e_free_trans;
@@ -1928,10 +1963,10 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	 * encrypts the written data with the guest's key, and the cache may
 	 * contain dirty, unencrypted data.
 	 */
-	sev_clflush_pages(guest_page, n);
+	sev_clflush_pages(&guest_page, 1);
 
 	/* The RECEIVE_UPDATE_DATA command requires C-bit to be always set. */
-	data.guest_address = (page_to_pfn(guest_page[0]) << PAGE_SHIFT) + offset;
+	data.guest_address = page_to_phys(guest_page) + offset;
 	data.guest_address |= sev_me_mask;
 	data.guest_len = params.guest_len;
 	data.handle = to_kvm_sev_info(kvm)->handle;
@@ -1939,7 +1974,7 @@ static int sev_receive_update_data(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	ret = sev_issue_cmd(kvm, SEV_CMD_RECEIVE_UPDATE_DATA, &data,
 				&argp->error);
 
-	sev_unpin_memory(kvm, guest_page, n);
+	sev_unpin_page(kvm, guest_page);
 
 e_free_trans:
 	kfree(trans);
-- 
2.54.0.rc1.513.gad8abe7a5a-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 5/6] KVM: SEV: Rewrite logic to {de,en}crypt memory for debug
  2026-04-16 23:10 [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
                   ` (3 preceding siblings ...)
  2026-04-16 23:10 ` [PATCH v2 4/6] KVM: SEV: Add helper function to pin/unpin a single page Sean Christopherson
@ 2026-04-16 23:10 ` Sean Christopherson
  2026-04-16 23:10 ` [PATCH v2 6/6] KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers Sean Christopherson
  5 siblings, 0 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-04-16 23:10 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Ashutosh Desai

Wholesale rewrite the guts of the debug {de,en}crypt flows, as the existing
code is broken, e.g. doesn't handle cases where the access isn't naturally
sized and aligned, and is so wildly flawed that attempting to salvage the
current code in an iterative fashion would be more risky than a rewrite.

E.g. when encrypting 9 bytes at offset 8, KVM needs to _decrypt_
destination[31:0] into a temporary buffer, buffer[31:0], then copy 9 bytes
from source[8:0] to buffer[16:8], then encrypt buffer[31:0] back into
destination[31:0].  The current code only ever copies 16 bytes, and
bizarrely uses a temporary buffer for the source as well.

To keep the code easier to read and maintain, send the unaligned cases
down dedicated "slow" paths instead of trying to mix and match the possible
combinations in one helper.

For now, preserve the basic approach of the current code, e.g. allocate an
entire page for the temporary buffer, to minimize unwanted changes in
functionality.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 306 +++++++++++++++++++----------------------
 1 file changed, 139 insertions(+), 167 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 4146c91788fb..89586f821c9c 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1237,159 +1237,140 @@ static int sev_guest_status(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	return ret;
 }
 
-static int __sev_issue_dbg_cmd(struct kvm *kvm, unsigned long src,
-			       unsigned long dst, int size,
-			       int *error, bool enc)
+static int sev_issue_dbg_cmd(struct kvm *kvm, unsigned long src_pa,
+			     unsigned long dst_pa, unsigned int size,
+			     unsigned int ioctl, int *error)
 {
-	struct sev_data_dbg data;
+	int cmd = ioctl == KVM_SEV_DBG_DECRYPT ? SEV_CMD_DBG_DECRYPT :
+						 SEV_CMD_DBG_ENCRYPT;
+	struct sev_data_dbg data = {
+		.handle = to_kvm_sev_info(kvm)->handle,
+		.dst_addr = dst_pa,
+		.src_addr = src_pa,
+		.len = size,
+	};
+
+	return sev_issue_cmd(kvm, cmd, &data, error);
+}
+
+static struct page *sev_alloc_dbg_buffer(void **buf)
+{
+	struct page *buf_p;
 
-	data.reserved = 0;
-	data.handle = to_kvm_sev_info(kvm)->handle;
-	data.dst_addr = dst;
-	data.src_addr = src;
-	data.len = size;
+	buf_p = alloc_page(GFP_KERNEL);
+	if (!buf_p)
+		return NULL;
+
+	*buf = kmap_local_page(buf_p);
+	return buf_p;
+}
 
-	return sev_issue_cmd(kvm,
-			     enc ? SEV_CMD_DBG_ENCRYPT : SEV_CMD_DBG_DECRYPT,
-			     &data, error);
+static void sev_free_dbg_buffer(struct page *buf_p, void *buf)
+{
+	kunmap_local(buf);
+	__free_page(buf_p);
 }
 
-static int __sev_dbg_decrypt(struct kvm *kvm, unsigned long src_paddr,
-			     unsigned long dst_paddr, int sz, int *err)
+static unsigned int sev_dbg_crypt_slow_addr_and_size(struct page *page,
+						     unsigned long __va,
+						     unsigned int len,
+						     unsigned long *pa)
 {
-	int offset;
+	/* The number of bytes to {de,en}crypt must be 16-byte aligned. */
+	unsigned int nr_bytes = round_up(len, 16);
+	unsigned long va = ALIGN_DOWN(__va, 16);
+
+	/*
+	 * Increase the number of bytes to {de,en}crypt by one chunk (16 bytes)
+	 * if the aligned address and length doesn't cover the unaligned range,
+	 * e.g. if the address is unaligned _and_ the access will split a chunk
+	 * at the tail.
+	 */
+	if (va + nr_bytes < __va + len)
+		nr_bytes += 16;
+
+	*pa = __sme_page_pa(page) + (va & ~PAGE_MASK);
 
 	/*
-	 * Its safe to read more than we are asked, caller should ensure that
-	 * destination has enough space.
+	 * Sanity check that the new access won't split a page.  This should
+	 * never happen; just squash the access and let the firmware command
+	 * fail.
 	 */
-	offset = src_paddr & 15;
-	src_paddr = round_down(src_paddr, 16);
-	sz = round_up(sz + offset, 16);
+	if (WARN_ON_ONCE((*pa & PAGE_MASK) != ((*pa + nr_bytes - 1) & PAGE_MASK)))
+		return 0;
 
-	return __sev_issue_dbg_cmd(kvm, src_paddr, dst_paddr, sz, err, false);
+	return nr_bytes;
 }
 
-static int __sev_dbg_decrypt_user(struct kvm *kvm, unsigned long paddr,
-				  void __user *dst_uaddr,
-				  unsigned long dst_paddr,
-				  int size, int *err)
+static int sev_dbg_decrypt_slow(struct kvm *kvm, unsigned long src,
+				struct page *src_p, unsigned long dst,
+				unsigned int len, int *err)
 {
-	struct page *tpage = NULL;
-	int ret, offset;
-
-	/* if inputs are not 16-byte then use intermediate buffer */
-	if (!IS_ALIGNED(dst_paddr, 16) ||
-	    !IS_ALIGNED(paddr,     16) ||
-	    !IS_ALIGNED(size,      16)) {
-		tpage = (void *)alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO);
-		if (!tpage)
-			return -ENOMEM;
-
-		dst_paddr = __sme_page_pa(tpage);
-	}
-
-	ret = __sev_dbg_decrypt(kvm, paddr, dst_paddr, size, err);
-	if (ret)
-		goto e_free;
-
-	if (tpage) {
-		offset = paddr & 15;
-		if (copy_to_user(dst_uaddr, page_address(tpage) + offset, size))
-			ret = -EFAULT;
-	}
-
-e_free:
-	if (tpage)
-		__free_page(tpage);
-
-	return ret;
+	unsigned int nr_bytes;
+	unsigned long src_pa;
+	struct page *buf_p;
+	void *buf;
+	int r;
+
+	buf_p = sev_alloc_dbg_buffer(&buf);
+	if (!buf_p)
+		return -ENOMEM;
+
+	nr_bytes = sev_dbg_crypt_slow_addr_and_size(src_p, src, len, &src_pa);
+
+	r = sev_issue_dbg_cmd(kvm, src_pa, __sme_page_pa(buf_p),
+			      nr_bytes, KVM_SEV_DBG_DECRYPT, err);
+	if (r)
+		goto out;
+
+	if (copy_to_user((void __user *)dst, buf + (src & 15), len))
+		r = -EFAULT;
+out:
+	sev_free_dbg_buffer(buf_p, buf);
+	return r;
 }
 
-static int __sev_dbg_encrypt_user(struct kvm *kvm, unsigned long paddr,
-				  void __user *vaddr,
-				  unsigned long dst_paddr,
-				  void __user *dst_vaddr,
-				  int size, int *error)
+static int sev_dbg_encrypt_slow(struct kvm *kvm, unsigned long src,
+				unsigned long dst, struct page *dst_p,
+				unsigned int len, int *err)
 {
-	struct page *src_tpage = NULL;
-	struct page *dst_tpage = NULL;
-	int ret, len = size;
+	unsigned int nr_bytes;
+	unsigned long dst_pa;
+	struct page *buf_p;
+	void *buf;
+	int r;
 
-	/* If source buffer is not aligned then use an intermediate buffer */
-	if (!IS_ALIGNED((unsigned long)vaddr, 16)) {
-		src_tpage = alloc_page(GFP_KERNEL_ACCOUNT);
-		if (!src_tpage)
-			return -ENOMEM;
+	buf_p = sev_alloc_dbg_buffer(&buf);
+	if (!buf_p)
+		return -ENOMEM;
 
-		if (copy_from_user(page_address(src_tpage), vaddr, size)) {
-			__free_page(src_tpage);
-			return -EFAULT;
-		}
+	/* Decrypt the _destination_ to do a RMW on plaintext. */
+	nr_bytes = sev_dbg_crypt_slow_addr_and_size(dst_p, dst, len, &dst_pa);
 
-		paddr = __sme_page_pa(src_tpage);
-	}
+	r = sev_issue_dbg_cmd(kvm, dst_pa, __sme_page_pa(buf_p),
+			      nr_bytes, KVM_SEV_DBG_DECRYPT, err);
+	if (r)
+		goto out;
 
 	/*
-	 *  If destination buffer or length is not aligned then do read-modify-write:
-	 *   - decrypt destination in an intermediate buffer
-	 *   - copy the source buffer in an intermediate buffer
-	 *   - use the intermediate buffer as source buffer
+	 * Copy from the source into the intermediate buffer, and then
+	 * re-encrypt the buffer into the destination.
 	 */
-	if (!IS_ALIGNED((unsigned long)dst_vaddr, 16) || !IS_ALIGNED(size, 16)) {
-		int dst_offset;
-
-		dst_tpage = alloc_page(GFP_KERNEL_ACCOUNT);
-		if (!dst_tpage) {
-			ret = -ENOMEM;
-			goto e_free;
-		}
-
-		ret = __sev_dbg_decrypt(kvm, dst_paddr,
-					__sme_page_pa(dst_tpage), size, error);
-		if (ret)
-			goto e_free;
-
-		/*
-		 *  If source is kernel buffer then use memcpy() otherwise
-		 *  copy_from_user().
-		 */
-		dst_offset = dst_paddr & 15;
-
-		if (src_tpage)
-			memcpy(page_address(dst_tpage) + dst_offset,
-			       page_address(src_tpage), size);
-		else {
-			if (copy_from_user(page_address(dst_tpage) + dst_offset,
-					   vaddr, size)) {
-				ret = -EFAULT;
-				goto e_free;
-			}
-		}
-
-		paddr = __sme_page_pa(dst_tpage);
-		dst_paddr = round_down(dst_paddr, 16);
-		len = round_up(size, 16);
-	}
-
-	ret = __sev_issue_dbg_cmd(kvm, paddr, dst_paddr, len, error, true);
-
-e_free:
-	if (src_tpage)
-		__free_page(src_tpage);
-	if (dst_tpage)
-		__free_page(dst_tpage);
-	return ret;
+	if (copy_from_user(buf + (dst & 15), (void __user *)src, len))
+		r = -EFAULT;
+	else
+		r = sev_issue_dbg_cmd(kvm, __sme_page_pa(buf_p), dst_pa,
+				      nr_bytes, KVM_SEV_DBG_ENCRYPT, err);
+out:
+	sev_free_dbg_buffer(buf_p, buf);
+	return r;
 }
 
-static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
+static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp,
+			 unsigned int cmd)
 {
-	unsigned long vaddr, vaddr_end, next_vaddr;
-	unsigned long dst_vaddr;
-	struct page *src_p, *dst_p;
 	struct kvm_sev_dbg debug;
-	unsigned int size;
-	int ret;
+	unsigned int i, len;
 
 	if (!sev_guest(kvm))
 		return -ENOTTY;
@@ -1404,20 +1385,29 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
 	    debug.dst_uaddr + debug.len < debug.dst_uaddr)
 		return -EINVAL;
 
-	vaddr = debug.src_uaddr;
-	size = debug.len;
-	vaddr_end = vaddr + size;
-	dst_vaddr = debug.dst_uaddr;
+	for (i = 0; i < debug.len; i += len) {
+		unsigned long src = debug.src_uaddr + i;
+		unsigned long dst = debug.dst_uaddr + i;
+		unsigned long s_off = src & ~PAGE_MASK;
+		unsigned long d_off = dst & ~PAGE_MASK;
+		struct page *src_p, *dst_p;
+		int ret;
 
-	for (; vaddr < vaddr_end; vaddr = next_vaddr) {
-		int len, s_off, d_off;
+		/*
+		 * Copy as many remaining bytes as possible while staying in a
+		 * single page for both the source and destination.
+		 */
+		len = min3(debug.len - i, PAGE_SIZE - s_off, PAGE_SIZE - d_off);
 
-		/* lock userspace source and destination page */
-		src_p = sev_pin_page(kvm, vaddr & PAGE_MASK, 0);
+		/*
+		 * Pin the source and destination pages; firmware operates on
+		 * physical addresses.
+		 */
+		src_p = sev_pin_page(kvm, src & PAGE_MASK, 0);
 		if (IS_ERR(src_p))
 			return PTR_ERR(src_p);
 
-		dst_p = sev_pin_page(kvm, dst_vaddr & PAGE_MASK, FOLL_WRITE);
+		dst_p = sev_pin_page(kvm, dst & PAGE_MASK, FOLL_WRITE);
 		if (IS_ERR(dst_p)) {
 			sev_unpin_page(kvm, src_p);
 			return PTR_ERR(dst_p);
@@ -1431,41 +1421,25 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp, bool dec)
 		sev_clflush_pages(&src_p, 1);
 		sev_clflush_pages(&dst_p, 1);
 
-		/*
-		 * Since user buffer may not be page aligned, calculate the
-		 * offset within the page.
-		 */
-		s_off = vaddr & ~PAGE_MASK;
-		d_off = dst_vaddr & ~PAGE_MASK;
-		len = min_t(size_t, (PAGE_SIZE - s_off), size);
-		len = min_t(size_t, len, PAGE_SIZE - d_off);
-
-		if (dec)
-			ret = __sev_dbg_decrypt_user(kvm,
-						     __sme_page_pa(src_p) + s_off,
-						     (void __user *)dst_vaddr,
-						     __sme_page_pa(dst_p) + d_off,
-						     len, &argp->error);
+		if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16))
+			ret = sev_issue_dbg_cmd(kvm,
+						__sme_page_pa(src_p) + s_off,
+						__sme_page_pa(dst_p) + d_off,
+						len, cmd, &argp->error);
+		else if (cmd == KVM_SEV_DBG_DECRYPT)
+			ret = sev_dbg_decrypt_slow(kvm, src, src_p, dst,
+						   len, &argp->error);
 		else
-			ret = __sev_dbg_encrypt_user(kvm,
-						     __sme_page_pa(src_p) + s_off,
-						     (void __user *)vaddr,
-						     __sme_page_pa(dst_p) + d_off,
-						     (void __user *)dst_vaddr,
-						     len, &argp->error);
+			ret = sev_dbg_encrypt_slow(kvm, src, dst, dst_p,
+						   len, &argp->error);
 
 		sev_unpin_page(kvm, src_p);
 		sev_unpin_page(kvm, dst_p);
 
 		if (ret)
-			goto err;
-
-		next_vaddr = vaddr + len;
-		dst_vaddr = dst_vaddr + len;
-		size -= len;
+			return ret;
 	}
-err:
-	return ret;
+	return 0;
 }
 
 static int sev_launch_secret(struct kvm *kvm, struct kvm_sev_cmd *argp)
@@ -2727,10 +2701,8 @@ int sev_mem_enc_ioctl(struct kvm *kvm, void __user *argp)
 		r = sev_guest_status(kvm, &sev_cmd);
 		break;
 	case KVM_SEV_DBG_DECRYPT:
-		r = sev_dbg_crypt(kvm, &sev_cmd, true);
-		break;
 	case KVM_SEV_DBG_ENCRYPT:
-		r = sev_dbg_crypt(kvm, &sev_cmd, false);
+		r = sev_dbg_crypt(kvm, &sev_cmd, sev_cmd.id);
 		break;
 	case KVM_SEV_LAUNCH_SECRET:
 		r = sev_launch_secret(kvm, &sev_cmd);
-- 
2.54.0.rc1.513.gad8abe7a5a-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 6/6] KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers
  2026-04-16 23:10 [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
                   ` (4 preceding siblings ...)
  2026-04-16 23:10 ` [PATCH v2 5/6] KVM: SEV: Rewrite logic to {de,en}crypt memory for debug Sean Christopherson
@ 2026-04-16 23:10 ` Sean Christopherson
  5 siblings, 0 replies; 7+ messages in thread
From: Sean Christopherson @ 2026-04-16 23:10 UTC (permalink / raw)
  To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel, Ashutosh Desai

When using a temporary buffer to {de,en}crypt unaligned memory for debug,
allocate only the number of bytes that are needed instead of allocating an
entire page.  The most common case for unaligned accesses will be reading
or writing less than 16 bytes.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/sev.c | 69 ++++++++++++++----------------------------
 1 file changed, 22 insertions(+), 47 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 89586f821c9c..0865ce4bcecb 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1253,53 +1253,34 @@ static int sev_issue_dbg_cmd(struct kvm *kvm, unsigned long src_pa,
 	return sev_issue_cmd(kvm, cmd, &data, error);
 }
 
-static struct page *sev_alloc_dbg_buffer(void **buf)
+static void *sev_dbg_crypt_slow_alloc(struct page *page, unsigned long __va,
+				      unsigned int len, unsigned long *pa,
+				      unsigned int *nr_bytes)
 {
-	struct page *buf_p;
-
-	buf_p = alloc_page(GFP_KERNEL);
-	if (!buf_p)
-		return NULL;
-
-	*buf = kmap_local_page(buf_p);
-	return buf_p;
-}
-
-static void sev_free_dbg_buffer(struct page *buf_p, void *buf)
-{
-	kunmap_local(buf);
-	__free_page(buf_p);
-}
-
-static unsigned int sev_dbg_crypt_slow_addr_and_size(struct page *page,
-						     unsigned long __va,
-						     unsigned int len,
-						     unsigned long *pa)
-{
-	/* The number of bytes to {de,en}crypt must be 16-byte aligned. */
-	unsigned int nr_bytes = round_up(len, 16);
 	unsigned long va = ALIGN_DOWN(__va, 16);
 
+	/* The number of bytes to {de,en}crypt must be 16-byte aligned. */
+	*nr_bytes = round_up(len, 16);
+
 	/*
 	 * Increase the number of bytes to {de,en}crypt by one chunk (16 bytes)
 	 * if the aligned address and length doesn't cover the unaligned range,
 	 * e.g. if the address is unaligned _and_ the access will split a chunk
 	 * at the tail.
 	 */
-	if (va + nr_bytes < __va + len)
-		nr_bytes += 16;
+	if (va + *nr_bytes < __va + len)
+		*nr_bytes += 16;
 
 	*pa = __sme_page_pa(page) + (va & ~PAGE_MASK);
 
 	/*
 	 * Sanity check that the new access won't split a page.  This should
-	 * never happen; just squash the access and let the firmware command
-	 * fail.
+	 * never happen; just pretend the allocation failed.
 	 */
-	if (WARN_ON_ONCE((*pa & PAGE_MASK) != ((*pa + nr_bytes - 1) & PAGE_MASK)))
-		return 0;
+	if (WARN_ON_ONCE((*pa & PAGE_MASK) != ((*pa + *nr_bytes - 1) & PAGE_MASK)))
+		return NULL;
 
-	return nr_bytes;
+	return kmalloc(*nr_bytes, GFP_KERNEL);
 }
 
 static int sev_dbg_decrypt_slow(struct kvm *kvm, unsigned long src,
@@ -1308,17 +1289,14 @@ static int sev_dbg_decrypt_slow(struct kvm *kvm, unsigned long src,
 {
 	unsigned int nr_bytes;
 	unsigned long src_pa;
-	struct page *buf_p;
 	void *buf;
 	int r;
 
-	buf_p = sev_alloc_dbg_buffer(&buf);
-	if (!buf_p)
+	buf = sev_dbg_crypt_slow_alloc(src_p, src, len, &src_pa, &nr_bytes);
+	if (!buf)
 		return -ENOMEM;
 
-	nr_bytes = sev_dbg_crypt_slow_addr_and_size(src_p, src, len, &src_pa);
-
-	r = sev_issue_dbg_cmd(kvm, src_pa, __sme_page_pa(buf_p),
+	r = sev_issue_dbg_cmd(kvm, src_pa, __sme_set(__pa(buf)),
 			      nr_bytes, KVM_SEV_DBG_DECRYPT, err);
 	if (r)
 		goto out;
@@ -1326,7 +1304,7 @@ static int sev_dbg_decrypt_slow(struct kvm *kvm, unsigned long src,
 	if (copy_to_user((void __user *)dst, buf + (src & 15), len))
 		r = -EFAULT;
 out:
-	sev_free_dbg_buffer(buf_p, buf);
+	kfree(buf);
 	return r;
 }
 
@@ -1336,18 +1314,15 @@ static int sev_dbg_encrypt_slow(struct kvm *kvm, unsigned long src,
 {
 	unsigned int nr_bytes;
 	unsigned long dst_pa;
-	struct page *buf_p;
 	void *buf;
 	int r;
 
-	buf_p = sev_alloc_dbg_buffer(&buf);
-	if (!buf_p)
-		return -ENOMEM;
-
 	/* Decrypt the _destination_ to do a RMW on plaintext. */
-	nr_bytes = sev_dbg_crypt_slow_addr_and_size(dst_p, dst, len, &dst_pa);
+	buf = sev_dbg_crypt_slow_alloc(dst_p, dst, len, &dst_pa, &nr_bytes);
+	if (!buf)
+		return -ENOMEM;
 
-	r = sev_issue_dbg_cmd(kvm, dst_pa, __sme_page_pa(buf_p),
+	r = sev_issue_dbg_cmd(kvm, dst_pa, __sme_set(__pa(buf)),
 			      nr_bytes, KVM_SEV_DBG_DECRYPT, err);
 	if (r)
 		goto out;
@@ -1359,10 +1334,10 @@ static int sev_dbg_encrypt_slow(struct kvm *kvm, unsigned long src,
 	if (copy_from_user(buf + (dst & 15), (void __user *)src, len))
 		r = -EFAULT;
 	else
-		r = sev_issue_dbg_cmd(kvm, __sme_page_pa(buf_p), dst_pa,
+		r = sev_issue_dbg_cmd(kvm, __sme_set(__pa(buf)), dst_pa,
 				      nr_bytes, KVM_SEV_DBG_ENCRYPT, err);
 out:
-	sev_free_dbg_buffer(buf_p, buf);
+	kfree(buf);
 	return r;
 }
 
-- 
2.54.0.rc1.513.gad8abe7a5a-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-16 23:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-16 23:10 [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
2026-04-16 23:10 ` [PATCH v2 1/6] KVM: SVM: Fix page overflow in sev_dbg_crypt() for ENCRYPT path Sean Christopherson
2026-04-16 23:10 ` [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls Sean Christopherson
2026-04-16 23:10 ` [PATCH v2 3/6] KVM: SEV: Explicitly validate the dst buffer for debug operations Sean Christopherson
2026-04-16 23:10 ` [PATCH v2 4/6] KVM: SEV: Add helper function to pin/unpin a single page Sean Christopherson
2026-04-16 23:10 ` [PATCH v2 5/6] KVM: SEV: Rewrite logic to {de,en}crypt memory for debug Sean Christopherson
2026-04-16 23:10 ` [PATCH v2 6/6] KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers Sean Christopherson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox