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

[Sean Christopherson <seanjc@google.com>]

On Mon, Sep 14, 2026, Tom Lendacky wrote:
> On 9/10/26 16:06, Jacky Li wrote:
> > Currently, snp_handle_(ext_)guest_req() returns -EIO when
> > kvm_{read/write/clear}_guest() fails while accessing guest-provided
> > buffers. Returning -EIO causes KVM_RUN to exit to userspace, likely
> > killing the VM.
> > 
> > Fix this by returning GHCB_HV_RESP_MALFORMED_INPUT with sub-error code
> > GHCB_ERR_INVALID_INPUT to the guest and resuming the vCPU.
> > 
> > Per the GHCB specification, guest-provided GPA buffers that cannot
> > be accessed by the hypervisor (e.g. private pages) should be treated
> > as guest input errors. Because kvm_{read/write/clear}_guest() only
> > returns -EFAULT on failure, treating this failure as an invalid input
> > aligns with the definition of -EFAULT ("Bad address").
> > 
> > Returning GHCB_ERR_INVALID_INPUT also matches existing SNP handling
> > in KVM, which already returns this error code for unaligned or
> > overlapping buffers. It also aligns with other hypercall implementations
> > in KVM (e.g. Hyper-V returning INVALID_HYPERCALL_INPUT on
> > kvm_read_guest() failures in kvm_hv_flush_tlb()).
> > 
> > Performing upfront validation (e.g. via kvm_mem_is_private()) is
> > avoided because it is prone to TOCTOU races with concurrent Page State
> > Changes.
> 
> I'm ok with this approach overall, but will we run into a sequence
> number problem now?
> 
> If the kvm_write_guest() in snp_handle_guest_req() fails, invalid input
> is going to be returned, but we will have successfully called
> SEV_CMD_SNP_GUEST_REQUEST. The sequence number will have advanced in the
> firmware, but I think the guest will not think that it has and not
> increment the sequence number causing subsequent requests to fail. At
> that point the guest will need to zero out the associated VMPCK used and
> move to the next one (there are a max of 4). If this continues happening
> the guest will eventually not be able to make guest requests anymore.
> But, I guess, if the kvm_write_guest() is failing, we're probably
> already in a bad situation, so maybe it is fine.

Oof, "fine" is definitely not ideal though.  

> > @@ -4244,8 +4246,10 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
> >  	if (ret && !fw_err)
> >  		return ret;
> >  
> > -	if (kvm_write_guest(kvm, resp_gpa, sev->guest_resp_buf, PAGE_SIZE))
> > -		return -EIO;
> > +	if (kvm_write_guest(kvm, resp_gpa, sev->guest_resp_buf, PAGE_SIZE)) {
> > +		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
> > +		return 1;
> > +	}

What if we keep this one as -EIO (or better, change it to -EFAULT in a separate
patch?), but add a comment explaning why KVM needs to exit to userspace in this
particular case?  That would be a good compromise; if the guest is attempting to
access non-existent memory, the initial READ will fail, i.e. we still get most
of the behavior Jacky wants.  The only fatal case would be where either userspace
really did screw up, or the guest managed to find read-only memory (though I
would probably argue that's likely also a userspace bug?).