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

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

[Jacky Li]

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.

Fixes: 88caf544c930 ("KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event")
Fixes: 74458e4859d8 ("KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event")
Signed-off-by: Jacky Li <jackyli@google.com>
---
 arch/x86/kvm/svm/sev.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 5705723f1f41..d07562310519 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4228,8 +4228,10 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
 
 	guard(mutex)(&sev->guest_req_mutex);
 
-	if (kvm_read_guest(kvm, req_gpa, sev->guest_req_buf, PAGE_SIZE))
-		return -EIO;
+	if (kvm_read_guest(kvm, req_gpa, sev->guest_req_buf, PAGE_SIZE)) {
+		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
+		return 1;
+	}
 
 	data.gctx_paddr = __psp_pa(sev->snp_context);
 	data.req_paddr = __psp_pa(sev->guest_req_buf);
@@ -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;
+	}
 
 	/* No action is requested *from KVM* if there was a firmware error. */
 	svm_vmgexit_no_action(svm, SNP_GUEST_ERR(0, fw_err));
@@ -4296,7 +4300,7 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r
 
 	if (kvm_read_guest(kvm, req_gpa + offsetof(struct snp_guest_msg_hdr, msg_type),
 			   &msg_type, 1))
-		return -EIO;
+		goto request_invalid;
 
 	/*
 	 * As per GHCB spec, requests of type MSG_REPORT_REQ also allow for
@@ -4336,7 +4340,7 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r
 		 * certificate table is terminated by 24-bytes of zeroes.
 		 */
 		if (data_npages && kvm_clear_guest(kvm, data_gpa, 24))
-			return -EIO;
+			goto request_invalid;
 	}
 
 	return snp_handle_guest_req(svm, req_gpa, resp_gpa);

---
base-commit: 50d05c7c76c96b90462f24debacca971d2e86713
change-id: 20260910-snp-invalid-input-5dbb6a408e7f

Best regards,
-- 
Jacky Li <jackyli@google.com>

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

[Sean Christopherson]

On Thu, Sep 10, 2026, 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.

When stating what a patch does (or doesn't) do, phrase everything as commands.
Passively describing the patch, as done above, is problematic as it's not clear
if the changelog is talking about what the patch itself is (not) doing, or if
it's talking about the side effects of the changes.  Whereas this:

  Don't try to validate guest-provide ahead of time, as such checks are prone
  to TOCTOU races, e.g. with Page State Changes, memslot updates, etc.

is more obviously talking about the patch.

> Fixes: 88caf544c930 ("KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event")
> Fixes: 74458e4859d8 ("KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event")
> Signed-off-by: Jacky Li <jackyli@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 5705723f1f41..d07562310519 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4228,8 +4228,10 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
>  
>  	guard(mutex)(&sev->guest_req_mutex);
>  
> -	if (kvm_read_guest(kvm, req_gpa, sev->guest_req_buf, PAGE_SIZE))
> -		return -EIO;
> +	if (kvm_read_guest(kvm, req_gpa, sev->guest_req_buf, PAGE_SIZE)) {

I don't love that userspace VMM goofs will bleed into the guest, but on the other
hand, KVM already uses this pattern for Hyper-V hypercalls (and worse patterns
for KVM-defined PV features), and practically speaking this is better behavior
than returning -EIO.  So I'm good with this.

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

[Jacky Li]

On Thu, Sep 10, 2026, Sean Christopherson wrote:
> When stating what a patch does (or doesn't) do, phrase everything as commands.
> Passively describing the patch, as done above, is problematic as it's not clear
> if the changelog is talking about what the patch itself is (not) doing, or if
> it's talking about the side effects of the changes.  Whereas this:
>
>   Don't try to validate guest-provide ahead of time, as such checks are prone
>   to TOCTOU races, e.g. with Page State Changes, memslot updates, etc.
>
> is more obviously talking about the patch.

Will do, thanks!  I'll use your wording in v2, with s/guest-provide/
guest-provided buffers/ and reflowed to fit two lines:

  Don't try to validate guest-provided buffers ahead of time, as such checks
  are prone to TOCTOU races (e.g. with Page State Changes, memslot updates).

> I don't love that userspace VMM goofs will bleed into the guest, but on the other
> hand, KVM already uses this pattern for Hyper-V hypercalls (and worse patterns
> for KVM-defined PV features), and practically speaking this is better behavior
> than returning -EIO.  So I'm good with this.

Agreed, it's not ideal, but KVM can't distinguish a bogus guest GPA from a
VMM goof for now, and killing the VM is worse for the former.

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

[Tom Lendacky]

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.

Thanks,
Tom

> 
> Fixes: 88caf544c930 ("KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event")
> Fixes: 74458e4859d8 ("KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event")
> Signed-off-by: Jacky Li <jackyli@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 5705723f1f41..d07562310519 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4228,8 +4228,10 @@ static int snp_handle_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t resp_
>  
>  	guard(mutex)(&sev->guest_req_mutex);
>  
> -	if (kvm_read_guest(kvm, req_gpa, sev->guest_req_buf, PAGE_SIZE))
> -		return -EIO;
> +	if (kvm_read_guest(kvm, req_gpa, sev->guest_req_buf, PAGE_SIZE)) {
> +		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_INPUT);
> +		return 1;
> +	}
>  
>  	data.gctx_paddr = __psp_pa(sev->snp_context);
>  	data.req_paddr = __psp_pa(sev->guest_req_buf);
> @@ -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;
> +	}
>  
>  	/* No action is requested *from KVM* if there was a firmware error. */
>  	svm_vmgexit_no_action(svm, SNP_GUEST_ERR(0, fw_err));
> @@ -4296,7 +4300,7 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r
>  
>  	if (kvm_read_guest(kvm, req_gpa + offsetof(struct snp_guest_msg_hdr, msg_type),
>  			   &msg_type, 1))
> -		return -EIO;
> +		goto request_invalid;
>  
>  	/*
>  	 * As per GHCB spec, requests of type MSG_REPORT_REQ also allow for
> @@ -4336,7 +4340,7 @@ static int snp_handle_ext_guest_req(struct vcpu_svm *svm, gpa_t req_gpa, gpa_t r
>  		 * certificate table is terminated by 24-bytes of zeroes.
>  		 */
>  		if (data_npages && kvm_clear_guest(kvm, data_gpa, 24))
> -			return -EIO;
> +			goto request_invalid;
>  	}
>  
>  	return snp_handle_guest_req(svm, req_gpa, resp_gpa);
> 
> ---
> base-commit: 50d05c7c76c96b90462f24debacca971d2e86713
> change-id: 20260910-snp-invalid-input-5dbb6a408e7f
> 
> Best regards,

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

[Sean Christopherson]

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

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

[Tom Lendacky]

On 9/15/26 09:31, Sean Christopherson wrote:
> 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?).

That sounds good to me.

Thanks,
Tom

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

[Jacky Li]

On Tue, Sep 15, 2026 at 8:21 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 9/15/26 09:31, Sean Christopherson wrote:
> > 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?).
>
> That sounds good to me.

Thinking through the sequence number and VMM bug concerns, I'd still argue
that returning INVALID_INPUT on kvm_write_guest() failure is preferable to
exiting to userspace:

1. req_gpa and resp_gpa are two independent guest inputs, so a guest can pass
   a valid req_gpa and a resp_gpa that isn't backed by any memslot to sail
   through the initial read and fail only on the write.  Exiting to userspace
   on write failure therefore still lets a guest kill the VM at will.

2. Comparing the two outcomes after a kvm_write_guest() failure:

   - Case 1 (exit to userspace with -EFAULT/-EIO): unrecoverable.  The PSP's
     response is lost, and userspace can't rewind the PSP's sequence number,
     so retrying KVM_RUN would just fail that check.  There is nothing
     userspace can do other than kill the VM.

   - Case 2 (return INVALID_INPUT to the guest): the guest disables the VMPCK
     and moves on to the next one, as Tom mentioned.  The worst case, once all
     four VMPCKs are exhausted, is that the guest can no longer issue guest
     requests, but the VM keeps running.

   Neither is ideal, but Case 2 seems to be better: a localized loss of
   functionality instead of the entire VM dying over a single bad request.
   It also leaves the decision to the guest, which can carry on with whatever
   doesn't depend on attestation, wind down cleanly, and shut itself down on
   its own terms instead of being killed abruptly.

3. More generally, when KVM can't tell a guest error apart from a VMM bug at a
   hypercall boundary, reporting the error to the guest seems to be the safer
   default: failing the guest request degrades one service while keeping the
   VM alive and debuggable, whereas exiting to userspace turns a single failed
   request into a dead VM, and hands an untrusted guest a way to terminate it.

   And in the context of SNP the guest doesn't trust the VMM anyway, so a VMM
   that fails to deliver a response is indistinguishable from one that refuses
   to, and the guest already deals with that by disabling the VMPCK on a
   VMGEXIT error.  Letting a host problem bleed into the guest is arguably
   "fine" here, since the guest is designed to cope with it regardless.