Re: [PATCH] KVM: SEV: Return INVALID_INPUT on SNP req/resp buffer access failure

[Sean Christopherson <seanjc@google.com>]

On Mon, Sep 21, 2026, Jacky Li wrote:
> On Wed, Sep 16, 2026 at 11:34 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > The problem is, I don't see how the guest can know when it needs to discard the
> > VMPCK (post-request failures) versus when the VMPCK is still "fine" (pre-request
> > errors).
> >
> > FWIW, I'm not terribly concerned about bleeding host issues into the guest, I'm
> > more concerned about ending up with deferred fatalities and a mess of an "ABI"
> > between the guest and KVM with respect to handling failures.
> >
> > What if we "tickle" the resp_gpa before doing the request?  Similar to how CPUs
> > probe bytes early in XSAVE/XRSTOR to avoid having to unwind later on.  In this
> > case, the response is restricted to a single page, so we only need to tickle a
> > single byte.  E.g.
> >
> 
> Agree. I see your points and why this tickle idea is a great solution.
> Actually, building on that idea, what do you think about using
> kvm_vcpu_map() in the exact place that you did
> kvm_write_guest(kvm, resp_gpa, &tickle, sizeof(tickle)) instead (and
> unmapping afterwards)? A few reasons:
> 
> 1. Like kvm_write_guest(), kvm_vcpu_map() also checks the existence and
>    writability of the memslot via __gfn_to_hva_many(..., write=true) and
>    faults in the host page as writable, plus it pins the page across the
>    firmware command.
> 
> 2. kvm_vcpu_map() has cleaner semantics of acquiring and holding the
>    guest response page in KVM until the response is delivered, rather
>    than doing a dummy write to test the waters. This is somewhat
>    consistent with the GHCB page itself, which stays mapped until the
>    response is filled in.

No, kvm_vcpu_map() actually has far worse semantics.  GUP pins the *page*, it does
not pin the userspace mapping.  I.e. kvm_vcpu_map() guarantees that KVM won't write
to a freed page, but that's literally it.  E.g. if userspace mprotect()'s the page
to make it read-only, KVM will write to what is supposed to be a RO page.  Or if
userspace completely unmaps the page, KVM will write to soon-to-be-freed memory.

We're actively trying to remove kvm_vcpu_map() to replace all usage with
"struct gfn_to_pfn_cache" APIs, as the gpc functionally is tied into mmu_notifiers,
i.e. guards against things like mprotect(), munmap(), etc.

So if we wanted to do something other than a uaccess, a gpc would be the way to
go.  But I'm not convinced that's the right choice here.  There is zero danger to
the host beyond what already exists with private memory in general.  I.e. if
guest_memfd leaves a dangling userspace mapping, then the host is hosed regardless
of what this code does.  Using a gpc could guarantee the write goes through or
fails early, but IMO that's not worth the extra complexity involved with using a
gpc, because I'm not convinced that continuing on is the best outcome if the
response GPA mapping is being concurrently modified.

> 3. Pinning the page prevents a malicious guest from converting resp_gpa
>    to private between the pre-check and kvm_write_guest() after the
>    guest_memfd in-place conversion patch series [1] is merged. 

Who cares?  The guest can only hurt itself by converting resp_gpa to private.

>    (This assumes a future guest_memfd implementation, so it's less convincing
>    than the first two points. This was also my original TOCTOU concern.  And
>    once we have either pre-check, on upstream today the guest has no way to
>    change the memslot state before kvm_write_guest(), so I'll also drop the
>    original TOCTOU commit message.)
> 
> [1] https://lore.kernel.org/lkml/20260830-gmem-inplace-conversion-v12-15-85e5fd25252a@google.com/