Re: [PATCH 20/24] KVM: SEV: Return INVALID_EVENT for SNP-only #VMGEXIT from non-SNP guest

[Sean Christopherson <seanjc@google.com>]

On Sat, May 30, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 880a2acd77bff..b59adddfdbccb 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> [ ... ]
> > @@ -4527,6 +4537,13 @@ int sev_handle_vmgexit(struct kvm_vcpu *vcpu)
> >  		return 1;
> >  	}
> >  
> > +	if (is_snp_only_vmgexit(control->exit_code) && !is_sev_snp_guest(vcpu)) {
> > +		vcpu_unimpl(vcpu, "vmgexit: exit code %#llx is SNP-only\n",
> > +			    control->exit_code);
> > +		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_EVENT);
> > +		return 1;
> > +	}
> > +
> >  	ret = sev_es_validate_vmgexit(svm);
> >  	if (ret)
> >  		return ret;
> 
> [Severity: Low]
> Does this consume the GHCB sw_exit_code field before verifying that the
> guest actually marked it as valid?
> 
> By moving the is_snp_only_vmgexit(control->exit_code) check before the
> call to sev_es_validate_vmgexit(), KVM evaluates the exit code before
> kvm_ghcb_sw_exit_code_is_valid() is checked.
> 
> If a non-SNP guest provides a GHCB without setting the valid bit for the
> exit code, but the uninitialized memory coincidentally matches an SNP-only
> exit code, will KVM incorrectly return GHCB_ERR_INVALID_EVENT instead of
> the required GHCB_ERR_MISSING_INPUT?

Super duper technically, yes.  We can't simply swap the order, because exits
like SVM_VMGEXIT_PSC shouldn't exist for non-SNP guests.  But I guess it would
be easy and safe enough to do:

	if (kvm_ghcb_sw_exit_code_is_valid(svm) &&
	    is_snp_only_vmgexit(control->exit_code) && !is_sev_snp_guest(vcpu)) {
		vcpu_unimpl(vcpu, "vmgexit: exit code %#llx is SNP-only\n",
			    control->exit_code);
		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_EVENT);
		return 1;
	}

Hmm, I guess the other option would be to split out the "always mandatory" checks


	if (!kvm_ghcb_sw_exit_code_is_valid(svm) ||
	    !kvm_ghcb_sw_exit_info_1_is_valid(svm) ||
	    !kvm_ghcb_sw_exit_info_2_is_valid(svm))
		return false;

e.g. end up with:

	if (!kvm_ghcb_sw_exit_code_is_valid(svm) ||
	    !kvm_ghcb_sw_exit_info_1_is_valid(svm) ||
	    !kvm_ghcb_sw_exit_info_2_is_valid(svm)) {
		/*
		 * Print the exit code even though it may not be marked valid
		 * as it could help with debugging.
		 */
		vcpu_unimpl(vcpu, "vmgexit: exit code %#llx input is not valid\n",
			    control->exit_code);
		dump_ghcb(svm);
		svm_vmgexit_bad_input(svm, GHCB_ERR_MISSING_INPUT);
		return 1;
	}

	if (is_snp_only_vmgexit(control->exit_code) && !is_sev_snp_guest(vcpu)) {
		vcpu_unimpl(vcpu, "vmgexit: exit code %#llx is SNP-only\n",
			    control->exit_code);
		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_EVENT);
		return 1;
	}

	if (!sev_es_are_required_ghcb_fields_valid(svm)) {
		vcpu_unimpl(vcpu, "vmgexit: exit code %#llx input is not valid\n",
			    control->exit_code);
		dump_ghcb(svm);
		svm_vmgexit_bad_input(svm, GHCB_ERR_MISSING_INPUT);
		return 1;
	}

Hmm, and if we go with the second option, maybe rename sev_es_are_required_ghcb_fields_valid()
to sev_es_are_ancillary_ghcb_fields_valid()?

I don't particularly love either option, but I think I have a slight (very slight)
preference for this?

	if (!kvm_ghcb_sw_exit_code_is_valid(svm) ||
	    !kvm_ghcb_sw_exit_info_1_is_valid(svm) ||
	    !kvm_ghcb_sw_exit_info_2_is_valid(svm)) {
		/*
		 * Print the exit code even though it may not be marked valid
		 * as it could help with debugging.
		 */
		vcpu_unimpl(vcpu, "vmgexit: exit code %#llx input is not valid\n",
			    control->exit_code);
		dump_ghcb(svm);
		svm_vmgexit_bad_input(svm, GHCB_ERR_MISSING_INPUT);
		return 1;
	}

	if (is_snp_only_vmgexit(control->exit_code) && !is_sev_snp_guest(vcpu)) {
		vcpu_unimpl(vcpu, "vmgexit: exit code %#llx is SNP-only\n",
			    control->exit_code);
		svm_vmgexit_bad_input(svm, GHCB_ERR_INVALID_EVENT);
		return 1;
	}

	if (!sev_es_are_ancillary_ghcb_fields_valid(svm)) {
		vcpu_unimpl(vcpu, "vmgexit: exit code %#llx input is not valid\n",
			    control->exit_code);
		dump_ghcb(svm);
		svm_vmgexit_bad_input(svm, GHCB_ERR_MISSING_INPUT);
		return 1;
	}