KVM: SEV: sev_dbg_crypt() fix and overhaul

Hello,

kernel test robot noticed "kernel-selftests.kvm.make.fail" on:

commit: f6ed3165a8ced57973a0919d0ead11eaff393477 ("[PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt")

url: https://github.com/intel-lab-lkp/linux/commits/Sean-Christopherson/KVM-SVM-Fix-page-overflow-in-sev_dbg_crypt-for-ENCRYPT-path/20260502-190513
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt

in testcase: kernel-selftests
version: kernel-selftests-x86_64-917719c412c4-1_20260508 with following parameters:

group: kvm

config: x86_64-rhel-9.4-kselftests
compiler: gcc-14
test machine: 16 threads Intel(R) Core(TM) i7-13620H (Raptor Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

gcc -D_GNU_SOURCE= -I/usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/tools/testing/selftests/../../../tools/testing/selftests -I/usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/tools/testing/selftests/cgroup/lib/include -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 -Wno-gnu-variable-sized-type-not-at-end -MD -MP -DCONFIG_64BIT -U_FORTIFY_SOURCE -fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset -fno-builtin-strnlen -fno-stack-protector -fno-PIE -fno-strict-aliasing -I/usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/tools/testing/selftests/../../../tools/include -I/usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/tools/testing/selftests/../../../tools/arch/x86/include -I/usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/tools/testing/selftests/../../../usr/include/ -Iinclude -I. -Iinclude/x86 -I ../rseq -I.. -isystem /usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/usr/include -march=x86-64-v2 -c dirty_log_perf_test.c -o /usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/tools/testing/selftests/kvm/dirty_log_perf_test.o
x86/sev_dbg_test.c: In function ‘test_sev_dbg’:
x86/sev_dbg_test.c:85:33: error: implicit declaration of function ‘vm_vaddr_alloc’ [-Wimplicit-function-declaration]

   85 |         data = addr_gva2hva(vm, vm_vaddr_alloc(vm, BUFFER_SIZE, KVM_UTIL_MIN_VADDR));
      |                                 ^~~~~~~~~~~~~~

At top level:
cc1: note: unrecognized command-line option ‘-Wno-gnu-variable-sized-type-not-at-end’ may have been intended to silence earlier diagnostics
make[1]: *** [Makefile.kvm:329: /usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/tools/testing/selftests/kvm/x86/sev_dbg_test.o] Error 1 make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory '/usr/src/perf_selftests-x86_64-rhel-9.4-kselftests-f6ed3165a8ced57973a0919d0ead11eaff393477/tools/testing/selftests/kvm'
make: *** [Makefile:217: all] Error 2

The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20260521/[email protected]

Signature · 2 lines

0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Quoted 27 lines · Michael Roth

Quoted 12 lines · Sean Christopherson

off-list in case this is double-ungood.

I ran this on an SNP host today (I think this is the first time I've run the full test on an SNP host), and it explodes with RMP violation #PFs in weird ways. I don't _think_ it's a KVM bug? Because I get the RMP violations even without ever running an SNP guest. I'm using kvm/next, at commit ef057cbf825e ("KVM: x86/mmu:
Ensure hugepage is in by slot before checking max mapping level")

It might be related to edge cases around pages, or maybe large sizes, as the test passes if I comment out the testcases that focus on the larger sizes. I haven't narrowed it down further than this (reboots aren't exactly fast).

Quoted 8 lines

What's especially odd is that on most of the crashes, there's no apparent reason for the RMP violation, as the RMP says the page is unassigned. I did get one crash (#2 below) where the RMP entry was non-zero. That one was after booting an SNP VM, but it was quite some time after shutting down that VM. The RMP dump is equally confusing to me, because AFAICT the RMP entry is corrupted.

Mike/Tom, can you try and repro and debug? I don't have the bandwidth to dig deep on this, and even if I did, I suspect it's beyond my abilities to debug.

Hi Sean,

I was able to reproduce this after running the test in a loop for a few minutes after a clean boot (trace below).

Quoted 2 lines

Not making much sense to be ATM either, but might be related to RMP table memory itself. Will keep investigating.

It looks like this was a recent regression caused by changes to how the temp buffers are allocated. I tested the below revert patch ~14 hours and the issue seems to be fixed (I generally was reproducing it within an couple hours or so max so it seems good).

Planning to submit upstream today if there's no objections/concerns.

From: Michael Roth <[email protected]>
Date: Mon, 22 Jun 2026 22:47:03 -0500
Subject: [PATCH] Revert "KVM: SEV: Allocate only as many bytes as needed for
temp crypt buffers"

When SNP is enabled, the data passed to firmware must be contained within pages that have been transferred to firmware ownership via the corresponding RMP table updates.

This is not compatible with uses kmalloc() allocations since kernel accesses

to other allocations within that page will trigger an RMP fault and crash the

host. Fix this by moving back to page-based allocations.o

+lists

Quoted 28 lines · Michael Roth

Quoted 27 lines · Michael Roth

Quoted 12 lines · Sean Christopherson

off-list in case this is double-ungood.

I ran this on an SNP host today (I think this is the first time I've run the full test on an SNP host), and it explodes with RMP violation #PFs in weird ways. I don't _think_ it's a KVM bug? Because I get the RMP violations even without ever running an SNP guest. I'm using kvm/next, at commit ef057cbf825e ("KVM: x86/mmu:
Ensure hugepage is in by slot before checking max mapping level")

It might be related to edge cases around pages, or maybe large sizes, as the test passes if I comment out the testcases that focus on the larger sizes. I haven't narrowed it down further than this (reboots aren't exactly fast).

Quoted 8 lines

What's especially odd is that on most of the crashes, there's no apparent reason for the RMP violation, as the RMP says the page is unassigned. I did get one crash (#2 below) where the RMP entry was non-zero. That one was after booting an SNP VM, but it was quite some time after shutting down that VM. The RMP dump is equally confusing to me, because AFAICT the RMP entry is corrupted.

Mike/Tom, can you try and repro and debug? I don't have the bandwidth to dig deep on this, and even if I did, I suspect it's beyond my abilities to debug.

Hi Sean,

I was able to reproduce this after running the test in a loop for a few minutes after a clean boot (trace below).

Quoted 7 lines

Quoted 2 lines

Not making much sense to be ATM either, but might be related to RMP table memory itself. Will keep investigating.

It looks like this was a recent regression caused by changes to how the temp buffers are allocated. I tested the below revert patch ~14 hours and the issue seems to be fixed (I generally was reproducing it within an couple hours or so max so it seems good).

Thanks for root causing!

Quoted 1 line

Planning to submit upstream today if there's no objections/concerns.

I object :-)

Quoted 6 lines

From: Michael Roth <[email protected]>
Date: Mon, 22 Jun 2026 22:47:03 -0500
Subject: [PATCH] Revert "KVM: SEV: Allocate only as many bytes as needed for
temp crypt buffers"

Quoted 3 lines

When SNP is enabled, the data passed to firmware must be contained within pages that have been transferred to firmware ownership via the corresponding RMP table updates.

IIUC, and I'm pretty sure I do at this point, this is wrong, or at least *very* misleading. Nothing in KVM performs RMP table updates, and so saying "the data passed to firmware must be contained within pages that have been transferred to firmware ownership" is completely inaccurate.

Piecing together the changelog and diff, and my own observations, my understanding is that **firmware** modifies the RMP to *temporarily* take ownership of the page

while performing the {DE,EN}CRYPT operation. So there's no requirement that the pages be in any specific state, the only "requirement" is that software needs to either prevent concurrent accesses or be prepared to handle spurious RMP #PFs due to the temporarily RMP modifications.

SNP_DBG_DECRYPT does require software to convert the destination to be a firmware
page: "The firmware also checks that the destination page is a Firmware page." But this is about SEV_DBG_{DE,EN}CRYPT, not SNP_DBG_ENCRYPT.

Quoted 2 lines

This is not compatible with uses kmalloc() allocations since kernel accesses

As is the statement about kmalloc(). kmalloc() of a PAGE_SIZE is a-ok, because the kernel will always hand out a full page.

Quoted 1 line

to other allocations within that page will trigger an RMP fault and crash the

And it's specificaly about other *accesses*, not simply other allocations.

Quoted 1 line

host. Fix this by moving back to page-based allocations.o

Always forcing full page allocations is overkill for SEV and SEV-ES, and robs us of the opportunity to document that SNP+ is special, which is especially important because AFAICT, none of the specs are so kind as to document this "minor" behavior.

Rather than fully revert, just force PAGE_SIZE allocations if the host supports SNP. I haven't run anywhere near 14 hours, but I was also getting failures on every single run of the test, and I ran the test 100 times without problem.

However, this only addresses the case where KVM is using temporary buffers. For small, nicely aligned operations, a misbehaving userspace could induce a crash by coercing the kernel into accessing the to-be-{de,en}crypted page via a kernel mapping while the crypo op is in-progess. I don't see a less awful option than forcing KVM to use the "slow" path, e.g. if the problem is limited to the dest for a decrypt operation:

x86/kvm/svm/sev.c +2 -1

index 87025d0d2f91..7e3334c90c57 100644
@@ -1406,7 +1406,8 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp,
                sev_clflush_pages(&src_p, 1);
                sev_clflush_pages(&dst_p, 1);
 
-               if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16))
+               if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16) &&
+                   (!cc_platform_has(CC_ATTR_HOST_SEV_SNP) || cmd != KVM_SEV_DBG_DECRYPT))
                        ret = sev_issue_dbg_cmd(kvm,
                                                __sme_page_pa(src_p) + s_off,
                                                __sme_page_pa(dst_p) + d_off,

But if firmware temporarily converts *both* pages, then we're "stuck" because at some point KVM needs to actually target the correct guest-owned page. The only option I can think of is to require capable(CAP_SYS_BOOT), *if* the above doesn't suffice.

So what exactly is the behavior here? Because I can't find anything in the specs, I can only make educated guesses based on the SNP_DBG_DECRYPT documentation.

---
From: Sean Christopherson <[email protected]>
Date: Tue, 23 Jun 2026 12:21:56 -0700
Subject: [PATCH] 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 kernel won't concurrently access the page that's being {de,en}crypted. On SNP-enabled platforms, firmware modifies the RMP to temporarily take ownership of the to-be-{de,en}crypted page, and so using a sub-allocation results in unexpected (and seemingly spurious) RMP #PF violations due to software attempting to access a firmware-owned page.

Note, it is unclear whether the RMP updates are considered architectural, or an implementation quirk, as none of the documentation for the non-SNP DBG_{DE,EN}CRYPT commands say anything about RMP updates, nor does the SNP specific firmware ABI spec.

Fixes 4c735bf1bc22 ("KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers")

Debugged-by Michael Roth <[email protected]>

Signed-off-by Sean Christopherson <[email protected]>

---
 arch/x86/kvm/svm/sev.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

arch/x86/kvm/svm/sev.c +10 -0

index 74fb15551e83..87025d0d2f91 100644
@@ -1280,6 +1280,16 @@ 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.  Firmware modifies the RMP
+	 * to temporarily take ownership of the page while the {DE,EN}CRYPT
+	 * operation is in-progress, and so concurrent software accesses will
+	 * encounter seemingly spurious RMP #PF violations
+	 */
+	if (cc_platform_has(CC_ATTR_HOST_SEV_SNP))
+		return kmalloc(PAGE_SIZE, GFP_KERNEL);
+
 	return kmalloc(*nr_bytes, GFP_KERNEL);
 }
 

base-commit: 9d4853b044beefa21c4ee3e18c40653601a64ced

Quoted 5 lines · Sean Christopherson

Quoted 5 lines

From: Michael Roth <[email protected]>
Date: Mon, 22 Jun 2026 22:47:03 -0500
Subject: [PATCH] Revert "KVM: SEV: Allocate only as many bytes as needed for
temp crypt buffers"

Quoted 8 lines

Quoted 3 lines

When SNP is enabled, the data passed to firmware must be contained within pages that have been transferred to firmware ownership via the corresponding RMP table updates.

IIUC, and I'm pretty sure I do at this point, this is wrong, or at least *very* misleading. Nothing in KVM performs RMP table updates, and so saying "the data passed to firmware must be contained within pages that have been transferred to firmware ownership" is completely inaccurate.

The driver will place it into the 'firmware' state ("5.3 Page States" in SEV-SNP Firmware ABI) prior to issuing the command to firmware, so from a kernel perspective I don't think it is inaccurate. It's true that this is handled transparently to KVM, but I don't think the wording is really in conflict with that.

Just trying to clarify and understand if I'm missing something more subtle here,

since your proposed fix seems nicer and more complete anyway.

Quoted 2 lines

Piecing together the changelog and diff, and my own observations, my understanding is that **firmware** modifies the RMP to *temporarily* take ownership of the page

rmp_mark_pages_firmware() in drivers/crypto/ccp/sev-dev.c does the RMP table update as part of preparing the command buffers prior to issuing the firmware call, placing the page in the 'firmware' state, and once the command completes, firmware modifies the RMP table state to be in the 'reclaim' state. The page will still be 'private' at that point, in the sense it will have the 'assigned' bit set.

The hypervisor can then take it back via additional firmware call SEV_CMD_SNP_PAGE_RECLAIM and subsequent RMPUPDATE to normal/'hypervisor' page. So the transition out of 'firmware' does sort of happen automatically, but the overall reclaim process requires explicit work by the hypervisor.

Quoted 8 lines

while performing the {DE,EN}CRYPT operation. So there's no requirement that the pages be in any specific state, the only "requirement" is that software needs to either prevent concurrent accesses or be prepared to handle spurious RMP #PFs due to the temporarily RMP modifications.

SNP_DBG_DECRYPT does require software to convert the destination to be a firmware
page: "The firmware also checks that the destination page is a Firmware page." But this is about SEV_DBG_{DE,EN}CRYPT, not SNP_DBG_ENCRYPT.

The above behavior isn't really specific to DECRYPT/ENCRYPT, but one example of a general requirement that when issuing legacy SEV commands on an SNP-enabled host, the memory that firmware writes to must undergo the above transitions before/after the actual firmware call:

5.3.9 SEV Legacy Commands
The behavior of the SEV-legacy commands is altered when the SNP firmware is in the INIT state. In this case, the SEV-legacy commands require any page that the SEV-legacy command writes to be a Firmware or Default page.

('Default' state is for ranges that have no corresponding RMP table entries at all, which isn't really applicable for how things are implemented in the kernel currently. The reasoning there though is that those pages can't be used for SNP guests, so legacy firmware commands can't be used as a widget to try to mess with private guest memory).

SNP_DBG_DECRYPT calls it out explicitly since that's how it as always worked, whereas for legacy commands these are additional, SNP-specific operations that happen before/after the legacy commands, so they are documented in a broader fashion.

Quoted 41 lines

Quoted 1 line

This is not compatible with uses kmalloc() allocations since kernel accesses

As is the statement about kmalloc(). kmalloc() of a PAGE_SIZE is a-ok, because the kernel will always hand out a full page.

Quoted 1 line

to other allocations within that page will trigger an RMP fault and crash the

And it's specificaly about other *accesses*, not simply other allocations.

Quoted 1 line

host. Fix this by moving back to page-based allocations.o

Always forcing full page allocations is overkill for SEV and SEV-ES, and robs us of the opportunity to document that SNP+ is special, which is especially important because AFAICT, none of the specs are so kind as to document this "minor" behavior.

Rather than fully revert, just force PAGE_SIZE allocations if the host supports SNP. I haven't run anywhere near 14 hours, but I was also getting failures on every single run of the test, and I ran the test 100 times without problem.

However, this only addresses the case where KVM is using temporary buffers. For small, nicely aligned operations, a misbehaving userspace could induce a crash by coercing the kernel into accessing the to-be-{de,en}crypted page via a kernel mapping while the crypo op is in-progess. I don't see a less awful option than forcing KVM to use the "slow" path, e.g. if the problem is limited to the dest for a decrypt operation:

x86/kvm/svm/sev.c +2 -1

index 87025d0d2f91..7e3334c90c57 100644
@@ -1406,7 +1406,8 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp,
                sev_clflush_pages(&src_p, 1);
                sev_clflush_pages(&dst_p, 1);
 
-               if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16))
+               if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16) &&
+                   (!cc_platform_has(CC_ATTR_HOST_SEV_SNP) || cmd != KVM_SEV_DBG_DECRYPT))
                        ret = sev_issue_dbg_cmd(kvm,
                                                __sme_page_pa(src_p) + s_off,
                                                __sme_page_pa(dst_p) + d_off,

Quoted 4 lines

But if firmware temporarily converts *both* pages, then we're "stuck" because at some point KVM needs to actually target the correct guest-owned page. The only option I can think of is to require capable(CAP_SYS_BOOT), *if* the above doesn't suffice.

Not sure I understand fully, but snp_populate_cmd_buf_desc_list() in the firmware driver requires the transiton for the destination page that we write to both for 'encrypt' as well as 'decrypt'. What the issue with using a temp buffer as the destination for the 'encrypt' case?

Quoted 2 lines · Michael Roth

On Tue, Jun 23, 2026 at 07:55:27PM +0000, Sean Christopherson wrote:
since your proposed fix seems nicer and more complete anyway.

It's missing a case though :-(

The final write for ENCRYPT writes to the destination directly:

		r = sev_issue_dbg_cmd(kvm, __sme_set(__pa(buf)), dst_pa,
				      nr_bytes, KVM_SEV_DBG_ENCRYPT, err);

More at the bottom.

Quoted 15 lines

Quoted 15 lines

x86/kvm/svm/sev.c +2 -1

index 87025d0d2f91..7e3334c90c57 100644
@@ -1406,7 +1406,8 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp,
                sev_clflush_pages(&src_p, 1);
                sev_clflush_pages(&dst_p, 1);
 
-               if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16))
+               if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16) &&
+                   (!cc_platform_has(CC_ATTR_HOST_SEV_SNP) || cmd != KVM_SEV_DBG_DECRYPT))
                        ret = sev_issue_dbg_cmd(kvm,
                                                __sme_page_pa(src_p) + s_off,
                                                __sme_page_pa(dst_p) + d_off,

Quoted 9 lines

Quoted 4 lines

But if firmware temporarily converts *both* pages, then we're "stuck" because at some point KVM needs to actually target the correct guest-owned page. The only option I can think of is to require capable(CAP_SYS_BOOT), *if* the above doesn't suffice.

Not sure I understand fully, but snp_populate_cmd_buf_desc_list() in the firmware driver requires the transiton for the destination page that we write to both for 'encrypt' as well as 'decrypt'. What the issue with using a temp buffer as the destination for the 'encrypt' case?

The problem is if KVM does NOT use a temporary buffer, which is the "fast" path where src, dst, and len are all 16-byte aligned. In that case, KVM does inline {de,en}cryption between src and dst. But those pages are also mapped into userspace (they have to be, since KVM pins them via gup()), which means that userspace could feed the same pages to a different chunk of the kernel that reads/write userspace memory via gup() (or gup-like equivalent).

E.g. pread()/read() on shmem-backed memory.

Looking at the full picture, isn't this a bug in the PSP driver that affects all commands? With pre-SNP VMs, "guest owned" isn't really a thing, since nothing guarantees that the destination page is accessible *only* by the guest.

If we rip that out and always bounce-buffer, then KVM won't have to force its own bounce buffering of sub-page lengths for SNP, and that would eliminate the race I described above.

Completely untested (I'll give this a whirl tomorrow).

Quoted 5 lines · Sean Christopherson

Quoted 2 lines · Michael Roth

On Tue, Jun 23, 2026 at 07:55:27PM +0000, Sean Christopherson wrote:
since your proposed fix seems nicer and more complete anyway.

It's missing a case though :-(

Ah, but so is the revert, and it's not a coincidence that the old code didn't bounce buffer the destination for KVM_SEV_DBG_ENCRYPT.

Quoted 7 lines

The final write for ENCRYPT writes to the destination directly:

		r = sev_issue_dbg_cmd(kvm, __sme_set(__pa(buf)), dst_pa,
				      nr_bytes, KVM_SEV_DBG_ENCRYPT, err);

More at the bottom.

Once more, with feeling...

Quoted 32 lines

Quoted 15 lines

Quoted 15 lines

x86/kvm/svm/sev.c +2 -1

index 87025d0d2f91..7e3334c90c57 100644
@@ -1406,7 +1406,8 @@ static int sev_dbg_crypt(struct kvm *kvm, struct kvm_sev_cmd *argp,
                sev_clflush_pages(&src_p, 1);
                sev_clflush_pages(&dst_p, 1);
 
-               if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16))
+               if (IS_ALIGNED(src, 16) && IS_ALIGNED(dst, 16) && IS_ALIGNED(len, 16) &&
+                   (!cc_platform_has(CC_ATTR_HOST_SEV_SNP) || cmd != KVM_SEV_DBG_DECRYPT))
                        ret = sev_issue_dbg_cmd(kvm,
                                                __sme_page_pa(src_p) + s_off,
                                                __sme_page_pa(dst_p) + d_off,

Quoted 9 lines

Quoted 4 lines

But if firmware temporarily converts *both* pages, then we're "stuck" because at some point KVM needs to actually target the correct guest-owned page. The only option I can think of is to require capable(CAP_SYS_BOOT), *if* the above doesn't suffice.

Not sure I understand fully, but snp_populate_cmd_buf_desc_list() in the firmware driver requires the transiton for the destination page that we write to both for 'encrypt' as well as 'decrypt'. What the issue with using a temp buffer as the destination for the 'encrypt' case?

The problem is if KVM does NOT use a temporary buffer, which is the "fast" path where src, dst, and len are all 16-byte aligned. In that case, KVM does inline {de,en}cryption between src and dst. But those pages are also mapped into userspace (they have to be, since KVM pins them via gup()), which means that userspace could feed the same pages to a different chunk of the kernel that reads/write userspace memory via gup() (or gup-like equivalent).

Quoted 1 line

E.g. pread()/read() on shmem-backed memory.

pread() doesn't work, because the source or destination needs to be a userspace address, and so any RMP #PFs will get eaten. But copy_file_range() makes the world go kablooie.

Weirdly, I haven't been able to get a splat of any kind, the host just goes AWOL and reboots. But I'm very confident RMP #PFs are the issue, because doing the same operation via writes in userspace yields:

    sev_dbg_test-24385   [049] d....  1892.853197: page_fault_user: address=0x7fc63f8fe000 ip=0x41cb44 error_code=0x80000007
    sev_dbg_test-24385   [049] d....  1892.853202: page_fault_user: address=0x7fc63f8fe000 ip=0x41cb44 error_code=0x80000007
    sev_dbg_test-24385   [049] d....  1892.853204: page_fault_user: address=0x7fc63f8fe000 ip=0x41cb44 error_code=0x80000007
    sev_dbg_test-24385   [049] d....  1892.853206: page_fault_user: address=0x7fc63f8fe000 ip=0x41cb44 error_code=0x80000007
    sev_dbg_test-24385   [049] d....  1892.853207: page_fault_user: address=0x7fc63f8fe000 ip=0x41cb44 error_code=0x80000007
    sev_dbg_test-24385   [049] d....  1892.853209: page_fault_user: address=0x7fc63f8fe000 ip=0x41cb44 error_code=0x80000007
    sev_dbg_test-24385   [049] d....  1892.853211: page_fault_user: address=0x7fc63f8fe000 ip=0x41cb44 error_code=0x80000007
    sev_dbg_test-24385   [049] d....  1892.853212: page_fault_user: address=0x7fc63f8fe000 ip=0x41cb44 error_code=0x80000007

Quoted 8 lines

Looking at the full picture, isn't this a bug in the PSP driver that affects all commands? With pre-SNP VMs, "guest owned" isn't really a thing, since nothing guarantees that the destination page is accessible *only* by the guest.

If we rip that out and always bounce-buffer, then KVM won't have to force its own bounce buffering of sub-page lengths for SNP, and that would eliminate the race I described above.

Quoted 1 line

Completely untested (I'll give this a whirl tomorrow).

So this doesn't work. Dredging up some knowledge of how this all works, I'm fairly certain the issue is that the encryption is salted with the physical address, and so encrypting at X instead of Y will yield different ciphertext. I.e. any command that encrypts the contents in the destination (or in theory, decrypts, though AFAICT none of the commands do so) *can't* be bounce buffered.

I don't see a way to salvage SEV/SEV-ES on SNP systems, short of requiring CAP_SYS_BOOT or some other elevated permission to do *anything*. Or redo SEV/SEV-ES support to require guest_memfd for operations that require putting pages into Firmware state.

Quoted 9 lines · Sean Christopherson

Quoted 5 lines · Sean Christopherson

Quoted 2 lines · Michael Roth

On Tue, Jun 23, 2026 at 07:55:27PM +0000, Sean Christopherson wrote:
since your proposed fix seems nicer and more complete anyway.

It's missing a case though :-(

Ah, but so is the revert, and it's not a coincidence that the old code didn't bounce buffer the destination for KVM_SEV_DBG_ENCRYPT.

Yah I was realizing this too :( Though I'm not sure why we set 'guest_owned' for DBG_DECRYPT... I'm not even sure it makes sense to write decrypted data into guest memory since it would only be usable if the range was decrypted to begin with... in most cases this likely wouldn't be guest memory.

So maybe the bounce buffer approach would work for DBG_DECRYPT at least,

Quoted 3 lines

pread() doesn't work, because the source or destination needs to be a userspace address, and so any RMP #PFs will get eaten. But copy_file_range() makes the world go kablooie.

I was hoping we could at least rely on kernel accesses going through GUP and maybe doing something like unmapping and freezing folio refcount to block concurrent access to the page, or read-only protections that would force the GUP to fail and just fail the concurrent userspace-requested operation...

copy_file_range() bypasses the GUP, but freezing the refcount or locking the folio would cause it to spin until the folio was released. So maybe that's an option?

This is similar to what's been proposed for gmem hugetlb support when splitting/rebuilding hugepages as part of the conversion path, where we need to guard against concurrent access by both kernel/userspace, so maybe it's not too hacky?

Quoted 4 lines

I don't see a way to salvage SEV/SEV-ES on SNP systems, short of requiring CAP_SYS_BOOT or some other elevated permission to do *anything*. Or redo SEV/SEV-ES support to require guest_memfd for operations that require putting pages into Firmware state.

Yah, short of maybe the above approach, I don't see any way around it atm :( If you think it's worth pursuing though I can give that a shot on my end.

Quoted 2 lines · Michael Roth

So maybe the bounce buffer approach would work for DBG_DECRYPT at least,

Yeah, DECRYPT worked fine, it's all the others that break.

Quoted 3 lines

Quoted 3 lines

pread() doesn't work, because the source or destination needs to be a userspace address, and so any RMP #PFs will get eaten. But copy_file_range() makes the world go kablooie.

And the risk isn't limited to copy_file_range(), that just happens to be the first syscall I found that did what I want. E.g. any memory that is kernel allocated and mapped into userspace would be problematic, because that memory could be fed back into KVM.

So I don't think we can fix this by playing games in KVM. Which is pretty much why we ended up with guest_memfd: the only way to lock down memory to the point where we're 100% certain it's safe to "poison" the pages is if KVM sequesters them away in a dedicated file system.

Quoted 4 lines

Quoted 4 lines

I'm fairly certain the issue is that the encryption is salted with the physical address, and so encrypting at X instead of Y will yield different ciphertext. I.e. any command that encrypts the contents in the destination (or in theory, decrypts, though AFAICT none of the commands do so) *can't* be bounce buffered.

Quoted 8 lines

Quoted 4 lines

I don't see a way to salvage SEV/SEV-ES on SNP systems, short of requiring CAP_SYS_BOOT or some other elevated permission to do *anything*. Or redo SEV/SEV-ES support to require guest_memfd for operations that require putting pages into Firmware state.

Yah, short of maybe the above approach, I don't see any way around it atm :( If you think it's worth pursuing though I can give that a shot on my end.

I say we wait for Paolo to get back from holiday (in a few weeks) before doing anything drastic. I'll send a patch to fix the existing selftest though, no reason to leave that hole open.

Quoted 15 lines · Sean Christopherson

Quoted 2 lines · Michael Roth

So maybe the bounce buffer approach would work for DBG_DECRYPT at least,

Yeah, DECRYPT worked fine, it's all the others that break.

Quoted 3 lines

Quoted 3 lines

pread() doesn't work, because the source or destination needs to be a userspace address, and so any RMP #PFs will get eaten. But copy_file_range() makes the world go kablooie.

And the risk isn't limited to copy_file_range(), that just happens to be the first syscall I found that did what I want. E.g. any memory that is kernel allocated and mapped into userspace would be problematic, because that memory could be fed back into KVM.

Quoted 4 lines

So I don't think we can fix this by playing games in KVM. Which is pretty much why we ended up with guest_memfd: the only way to lock down memory to the point where we're 100% certain it's safe to "poison" the pages is if KVM sequesters them away in a dedicated file system.

Yes, I agree that it's basically the same issue that led to using guest_memfd for SNP. Another one would be io_submit().

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

I'm fairly certain the issue is that the encryption is salted with the physical address, and so encrypting at X instead of Y will yield different ciphertext. I.e. any command that encrypts the contents in the destination (or in theory, decrypts, though AFAICT none of the commands do so) *can't* be bounce buffered.

Yep. :/

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

I don't see a way to salvage SEV/SEV-ES on SNP systems, short of requiring CAP_SYS_BOOT or some other elevated permission to do *anything*. Or redo SEV/SEV-ES support to require guest_memfd for operations that require putting pages into Firmware state.

Quoted 3 lines

Quoted 3 lines

Yah, short of maybe the above approach, I don't see any way around it atm :( If you think it's worth pursuing though I can give that a shot on my end.

The unavoidable ones include, if I understand correctly, not just DBG_ENCRYPT but also LAUNCH_UPDATE_DATA?

For encrypt the destination is the guest memory, so how would adding guest_memfd support for SEV/SEV-ES work? The guest doesn't have a page-state-change call and neither does it have RMP nested page faults, so how would you communicate the private<->shared switch to userspace?

Quoted 4 lines

I say we wait for Paolo to get back from holiday (in a few weeks) before doing anything drastic. I'll send a patch to fix the existing selftest though, no reason to leave that hole open.

Well, everything is drastic. "Fortunately" AMD helped us with the firmware update that already hides SEV-ES on SNP machines (CVE-2025-48514), so at least there's a precedent.

Could we say CAP_SYS_BOOT is only be required to create the VM, after which it's up to userspace to not screw up? Probably not, because userspace can then drop privileges and operate on the file descriptor; which is the actual dangerous part.

The only not-horrible alternative would be a default-disabled module parameter that allows SEV/SEV-ES on SNP systems but adds TAINT_USER when you create one. And downstreams that don't like the idea can just rip out the taint.

Paolo

Quoted 11 lines · Paolo Bonzini

Quoted 8 lines

Quoted 8 lines

Quoted 4 lines

I don't see a way to salvage SEV/SEV-ES on SNP systems, short of requiring CAP_SYS_BOOT or some other elevated permission to do *anything*. Or redo SEV/SEV-ES support to require guest_memfd for operations that require putting pages into Firmware state.

Yah, short of maybe the above approach, I don't see any way around it atm :( If you think it's worth pursuing though I can give that a shot on my end.

The unavoidable ones include, if I understand correctly, not just DBG_ENCRYPT but also LAUNCH_UPDATE_DATA?

That was my understanding, e.g. a user could try to use the HVA to trigger the copy_file_range() path right after snp_map_cmd_buf_desc()->rmp_mark_pages_firmware() is performed as part of servicing the KVM_SEV_LAUNCH_UPDATE_DATA request, and it seems like that would trigger the same issue. And bounce buffers wouldn't work for that one either.

and it's kinda important =/

Quoted 4 lines

For encrypt the destination is the guest memory, so how would adding guest_memfd support for SEV/SEV-ES work? The guest doesn't have a page-state-change call and neither does it have RMP nested page faults, so how would you communicate the private<->shared switch to userspace?

During early SNP hypervisor development (when guest_memfd was called "restricted_mem"), the patches were actually based on top of some patches from Nikunj that added guest_memfd/restricted_mem support for SEV-ES. One nice thing about that is it brought about support for lazy page allocation instead of relying on KVM_MEMORY_ENCRYPT_REG_REGION.

The support piggybacks off the KVM_HC_MAP_GPA RANGE hypercall that was added on the guest side to enable SEV live migration so the VMM could distinguish between shared/private (and punt private page handling to something else). By coincedence, that's also what SNP/TDX ended up using to forward conversion requests to userspace.

However, since SEV live migration never became a thing upstream there's potential that the KVM_HC_MAP_GPA_RANGE handling might miss some of the newer cases, which would lead to silent corruption (though with SNP enabled we might still get an indicator of whether or not the guest had the C-bit set, so we could detect a mismatch that way and maybe even be able to handle implicit conversion requests).

The big issue with this approach though is hugepages: we'd be dropping existing SEV support for both hugetlb/THP unless we implemented in-place conversion support for SEV-ES so it could eventually benefit from the hugetlb patches at least, but if in the meantime it's 4K-only I'm not sure anyone is going to get much use out of that, or still care about this support when gmem+hugepages is eventually enabled.

Quoted 13 lines

Quoted 4 lines

I say we wait for Paolo to get back from holiday (in a few weeks) before doing anything drastic. I'll send a patch to fix the existing selftest though, no reason to leave that hole open.

Well, everything is drastic. "Fortunately" AMD helped us with the firmware update that already hides SEV-ES on SNP machines (CVE-2025-48514), so at least there's a precedent.

Could we say CAP_SYS_BOOT is only be required to create the VM, after which it's up to userspace to not screw up? Probably not, because userspace can then drop privileges and operate on the file descriptor; which is the actual dangerous part.

Quoted 4 lines

The only not-horrible alternative would be a default-disabled module parameter that allows SEV/SEV-ES on SNP systems but adds TAINT_USER when you create one. And downstreams that don't like the idea can just rip out the taint.

It might not be as elegant, but either of these seems more useful as a means to allow users to continue to be able to run existing workloads if they trust their userspace/VMMs. I'm worried the guest_memfd support would end up being mostly-wasted effort at this point since it's not a straightforward/drop-in replacement.

Disclaimer: I could have sworn I sent this last week, but obviously did not, and I can't remember _why_ I didn't send it. So don't be too surprised if there's some glaring flaw or boneheaded mistake.

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.

[...]