Skip to contentSkip to conversation

KVM: SEV: Allocate full pages for {DE,EN}CRYPT ops on SNP-enabled hosts

PATCH Applied Sean Christopherson 2 participants 3 messages Jul 23 – Jul 31

Subsystems (3)
lore
Share & tools
lore

Sean Christopherson

author maintainer
#0 lore raw

PATCH v2 1/1 KVM: SEV: Allocate full pages for {DE,EN}CRYPT ops on SNP-enabled hosts

When {de,en}crypting memory of an SEV or SEV-ES guest on an SNP-enabled host via a temporary buffer, allocate a full 4KiB page for the buffer to ensure the page containing the buffer is wholly owned by KVM, i.e. won't be concurrently allocated and accessed by other kernel code while KVM is using the buffer to {de,en}crypt memory. On SNP-enabled platforms, when sending SEV/SEV-ES commands that trigger firmware writes to memory, the to-be-written page(s) must be (temporarily) assigned to Firmware (as required by the SNP architecture, to guard against using such commands as gadgets to attack SNP guests). See snp_map_cmd_buf_desc() and friends.

Unfortunately, transferring ownership of a page to Firmware makes the page inaccessible to software, and thus writes generate RMP #PF violations. If KVM uses a sub-page allocation for its temporary buffer, some other actor in the kernel can allocate and use the other portions of the page, and thus trigger unexpected (and seemingly spurious) RMP #PF violations due to software attempting to access a Firmware-owned page.

  BUG: unable to handle page fault for address: ffff906ae30f0300
  #PF: supervisor write access in kernel mode
  #PF: error_code(0x80000003) - RMP violation
  PGD 6b1b80d067 P4D 6b1b80d067 PUD 100231e2063 PMD 10055a88063 PTE 80000100630f0163
  SEV-SNP: PFN 0x100630f0 unassigned, dumping non-zero entries in 2M PFN region: [0x10063000 - 0x10063200]
  Oops: Oops: 0003 [#1] SMP
  CPU: 70 UID: 0 PID: 10658 Comm: svw_WaiterThrea Tainted: G     U  W  O        7.1.0-smp--c22293789940-seanjc-next #1 PREEMPTLAZY
  Tainted: [U]=USER, [W]=WARN, [O]=OOT_MODULE
  Hardware name: Google, Inc.                                                       Arcadia_IT_80/Arcadia_IT_80, BIOS 34.86.0-102 01/25/2026
  RIP: 0010:memset+0xf/0x20
  Call Trace:
   <TASK>
   __kvmalloc_node_noprof+0x2a4/0x710
   do_getxattr+0x4e/0x130
   path_getxattrat+0x125/0x1b0
   do_syscall_64+0x10a/0x480
   entry_SYSCALL_64_after_hwframe+0x4b/0x53
  RIP: 0033:0x7f3a22cb6daa
   </TASK>
  Modules linked in: kvm_amd kvm irqbypass vfat fat ccp k10temp sha3 libsha3 i2c_piix4 gq(O) cdc_acm xhci_pci xhci_hcd
  gsmi: Log Shutdown Reason 0x03
  CR2: ffff906ae30f0300
  ---[ end trace 0000000000000000 ]---
  RIP: 0010:memset+0xf/0x20
  Kernel panic - not syncing: Fatal exception
  Kernel Offset: 0x39e00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
  gsmi: Log Shutdown Reason 0x02
Fixes 4c735bf1bc22 ("KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers")
Cc Michael Roth <[email protected]>

---

v2: Use __get_free_page() to ensure it really is a full page. [Sashiko]

v1: https://lore.kernel.org/all/[email protected]

 arch/x86/kvm/svm/sev.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)
arch/x86/kvm/svm/sev.c +21 -2
index 74fb15551e83..803bd39695fd 100644
@@ -1280,9 +1280,28 @@ static void *sev_dbg_crypt_slow_alloc(struct page *page, unsigned long __va,
 	if (WARN_ON_ONCE((*pa & PAGE_MASK) != ((*pa + *nr_bytes - 1) & PAGE_MASK)))
 		return NULL;
 
+	/*
+	 * If SNP is enabled, i.e. the RMP is active, allocate a full page to
+	 * prevent concurrent accesses to the page.  As required by firmware,
+	 * the PSP driver updates the RMP to temporarily transfer ownership of
+	 * the page to Firmware while the {DE,EN}CRYPT operation is in-progress,
+	 * and so concurrent software accesses to the page will encounter
+	 * seemingly spurious RMP #PF violations
+	 */
+	if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+		return (void *)__get_free_page(GFP_KERNEL);
+
 	return kmalloc(*nr_bytes, GFP_KERNEL);
 }
 
+static void sev_dbg_crypt_slow_free(void *buf)
+{
+	if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+		free_page((unsigned long)buf);
+	else
+		kfree(buf);
+}
+
 static int sev_dbg_decrypt_slow(struct kvm *kvm, unsigned long src,
 				struct page *src_p, unsigned long dst,
 				unsigned int len, int *err)
@@ -1304,7 +1323,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:
-	kfree(buf);
+	sev_dbg_crypt_slow_free(buf);
 	return r;
 }
 
@@ -1337,7 +1356,7 @@ static int sev_dbg_encrypt_slow(struct kvm *kvm, unsigned long src,
 		r = sev_issue_dbg_cmd(kvm, __sme_set(__pa(buf)), dst_pa,
 				      nr_bytes, KVM_SEV_DBG_ENCRYPT, err);
 out:
-	kfree(buf);
+	sev_dbg_crypt_slow_free(buf);
 	return r;
 }
 

base-commit: a204badd8432f93b7e862e7dac6db0fe3d65f370

Signature · 1 line
2.55.0.229.g6434b31f56-goog
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The fast path in sev_dbg_crypt directly passes a pinned user page to the PSP firmware without a bounce buffer, allowing an unprivileged userspace process to trigger a host kernel panic via an RMP #PF. --

--- Patch [1]: [PATCH v2] KVM: SEV: Allocate full pages for {DE,EN}CRYPT ops on SNP-enabled hosts --- commit b2c2b9ead19801c1266e2d6c5a4d9c103c90e11c
Author: Sean Christopherson <[email protected]>

KVM: SEV: Allocate full pages for {DE,EN}CRYPT ops on SNP-enabled hosts

This commit updates SEV debug crypt operations on SNP-enabled hosts to use full page allocations for temporary bounce buffers. This prevents RMP #PF host panics caused by other kernel actors sharing and accessing the same page while firmware temporarily owns it.

Quoted 5 lines
arch/x86/kvm/svm/sev.c +0 -0
index 74fb15551e83f..803bd39695fdd 100644
@@ -1280,9 +1280,28 @@ static void *sev_dbg_crypt_slow_alloc(struct page *page, unsigned long __va,

[ ... ]

arch/x86/kvm/svm/sev.c static void *sev_dbg_crypt_slow_alloc(struct page *page, unsigned long __va, · line 1292

Quoted 10 lines
arch/x86/kvm/svm/sev.c +10 -0
+	/*
+	 * If SNP is enabled, i.e. the RMP is active, allocate a full page to
+	 * prevent concurrent accesses to the page.  As required by firmware,
+	 * the PSP driver updates the RMP to temporarily transfer ownership of
+	 * the page to Firmware while the {DE,EN}CRYPT operation is in-progress,
+	 * and so concurrent software accesses to the page will encounter
+	 * seemingly spurious RMP #PF violations
+	 */
+	if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+		return (void *)__get_free_page(GFP_KERNEL);

[Severity: High]
This is a pre-existing issue, but does this full-page allocation fix in the slow path still leave the fast path vulnerable?

Looking at sev_dbg_crypt(), if the addresses and length are 16-byte aligned, the code bypasses the bounce buffer and directly passes pinned user pages to the firmware:

arch/x86/kvm/svm/sev.c:sev_dbg_crypt() {
    ...
		src_p = sev_pin_page(kvm, src & PAGE_MASK, 0);
    ...
		dst_p = sev_pin_page(kvm, dst & PAGE_MASK, FOLL_WRITE);
    ...
		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);
    ...
}

Because these are user-controlled pages, couldn't an unprivileged userspace process issue KVM_SEV_DBG_DECRYPT on aligned addresses, and concurrently execute a normal syscall (like read() or getxattr()) on the same user page?

Since the firmware temporarily takes ownership of the page, wouldn't the concurrent kernel access in supervisor mode still generate an RMP #PF and panic the host, bypassing the protection added here for the slow path?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
author maintainer Applied
#2 lore raw
In reply to Sean Christopherson
Quoted 11 lines · Sean Christopherson

When {de,en}crypting memory of an SEV or SEV-ES guest on an SNP-enabled host via a temporary buffer, allocate a full 4KiB page for the buffer to ensure the page containing the buffer is wholly owned by KVM, i.e. won't be concurrently allocated and accessed by other kernel code while KVM is using the buffer to {de,en}crypt memory. On SNP-enabled platforms, when sending SEV/SEV-ES commands that trigger firmware writes to memory, the to-be-written page(s) must be (temporarily) assigned to Firmware (as required by the SNP architecture, to guard against using such commands as gadgets to attack SNP guests). See snp_map_cmd_buf_desc() and friends.

[...]

Applied to kvm-x86 svm, thanks!

[1/1] KVM: SEV: Allocate full pages for {DE,EN}CRYPT ops on SNP-enabled hosts

      https://github.com/kvm-x86/linux/commit/a33c40b93ccf

--
https://github.com/kvm-x86/linux/tree/next