Re: [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls

[Sean Christopherson <seanjc@google.com>]

On Tue, Jun 23, 2026, Sean Christopherson wrote:
> On Tue, Jun 23, 2026, Michael Roth wrote:
> > 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.

> 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...

> > > diff --git arch/x86/kvm/svm/sev.c arch/x86/kvm/svm/sev.c
> > > index 87025d0d2f91..7e3334c90c57 100644
> > > --- arch/x86/kvm/svm/sev.c
> > > +++ arch/x86/kvm/svm/sev.c
> > > @@ -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.
> > 
> > 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.

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


> 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).

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.