From: Sean Christopherson <seanjc@google.com>
To: Michael Roth <michael.roth@amd.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>,
Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls
Date: Wed, 24 Jun 2026 17:25:07 -0700 [thread overview]
Message-ID: <ajx1Y93iX2oV-DLa@google.com> (raw)
In-Reply-To: <45ixyum4upln4b2yvasovm2bmsfxyfezsi23vsr75r4fklelef@utxv4mymvfcd>
On Wed, Jun 24, 2026, Michael Roth wrote:
> On Wed, Jun 24, 2026 at 02:24:24PM -0700, Sean Christopherson wrote:
> > 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.
>
> 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,
Yeah, DECRYPT worked fine, it's all the others that break.
> > 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?
The big difference is that guest_memfd owns the folio. For SEV/SEV-ES, KVM has
no clue what sits behind the destination of the ENCRYPT operation (or the other
guest_owned commands). KVM knows it's a refcounted struct page due to pinning
the memory, but that's about it. We'd basically have to audit every filesystem
to see if a "fix" would actually work.
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. Ha! Now that I think about it, I bet the pages mapped via
kvm_vcpu_fault() are problematic, because KVM owns the pages and will access them
at will.
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.
> > 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.
next prev parent reply other threads:[~2026-06-25 0:25 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-01 20:35 [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
2026-05-01 20:35 ` [PATCH v2 1/6] KVM: SVM: Fix page overflow in sev_dbg_crypt() for ENCRYPT path Sean Christopherson
2026-05-01 20:35 ` [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls Sean Christopherson
2026-05-21 7:04 ` [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt kernel test robot
2026-05-21 14:28 ` Sean Christopherson
[not found] ` <ajNQ_Ix5e0p6FuFT@google.com>
[not found] ` <tx3hcnrhfbcz2p7e7umc5o2zrw74pluzuzgtlqt4dz5cc5nhxx@6tzzorc25nqw>
[not found] ` <s7jfbw3tnfgmp6blfqlfoi7yhq6f5n6xl62zzty5up45cqtsty@ljpjlvqw2gyy>
2026-06-23 19:55 ` [PATCH v2 2/6] KVM: selftests: Add a test to verify SEV {en,de}crypt debug ioctls Sean Christopherson
2026-06-23 23:22 ` Michael Roth
2026-06-24 0:22 ` Sean Christopherson
2026-06-24 21:24 ` Sean Christopherson
2026-06-24 23:11 ` Michael Roth
2026-06-25 0:25 ` Sean Christopherson [this message]
2026-05-01 20:35 ` [PATCH v2 3/6] KVM: SEV: Explicitly validate the dst buffer for debug operations Sean Christopherson
2026-05-01 20:35 ` [PATCH v2 4/6] KVM: SEV: Add helper function to pin/unpin a single page Sean Christopherson
2026-05-01 20:35 ` [PATCH v2 5/6] KVM: SEV: Rewrite logic to {de,en}crypt memory for debug Sean Christopherson
2026-05-01 20:35 ` [PATCH v2 6/6] KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers Sean Christopherson
2026-05-19 0:41 ` [PATCH v2 0/6] KVM: SEV: sev_dbg_crypt() fix and overhaul Sean Christopherson
-- strict thread matches above, loose matches on Subject: below --
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ajx1Y93iX2oV-DLa@google.com \
--to=seanjc@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michael.roth@amd.com \
--cc=pbonzini@redhat.com \
--cc=thomas.lendacky@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox