Skip to content

Commit 9a1dfef

Browse files
committed
KVM: x86: clear vcpu->run->hypercall.ret before exiting for KVM_EXIT_HYPERCALL
QEMU up to 9.2.0 is assuming that vcpu->run->hypercall.ret is 0 on exit and it never modifies it when processing KVM_EXIT_HYPERCALL. Make this explicit in the code, to avoid breakage when KVM starts modifying that field. This in principle is not a good idea... It would have been much better if KVM had set the field to -KVM_ENOSYS from the beginning, so that a dumb userspace that does nothing on KVM_EXIT_HYPERCALL would tell the guest it does not support KVM_HC_MAP_GPA_RANGE. However, breaking userspace is a Very Bad Thing, as everybody should know. Reported-by: Binbin Wu <binbin.wu@linux.intel.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
1 parent 10b2c8a commit 9a1dfef

File tree

2 files changed

+21
-0
lines changed

2 files changed

+21
-0
lines changed

arch/x86/kvm/svm/sev.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3634,6 +3634,13 @@ static int snp_begin_psc_msr(struct vcpu_svm *svm, u64 ghcb_msr)
36343634

36353635
vcpu->run->exit_reason = KVM_EXIT_HYPERCALL;
36363636
vcpu->run->hypercall.nr = KVM_HC_MAP_GPA_RANGE;
3637+
/*
3638+
* In principle this should have been -KVM_ENOSYS, but userspace (QEMU <=9.2)
3639+
* assumed that vcpu->run->hypercall.ret is never changed by KVM and thus that
3640+
* it was always zero on KVM_EXIT_HYPERCALL. Since KVM is now overwriting
3641+
* vcpu->run->hypercall.ret, ensuring that it is zero to not break QEMU.
3642+
*/
3643+
vcpu->run->hypercall.ret = 0;
36373644
vcpu->run->hypercall.args[0] = gpa;
36383645
vcpu->run->hypercall.args[1] = 1;
36393646
vcpu->run->hypercall.args[2] = (op == SNP_PAGE_STATE_PRIVATE)
@@ -3797,6 +3804,13 @@ static int snp_begin_psc(struct vcpu_svm *svm, struct psc_buffer *psc)
37973804
case VMGEXIT_PSC_OP_SHARED:
37983805
vcpu->run->exit_reason = KVM_EXIT_HYPERCALL;
37993806
vcpu->run->hypercall.nr = KVM_HC_MAP_GPA_RANGE;
3807+
/*
3808+
* In principle this should have been -KVM_ENOSYS, but userspace (QEMU <=9.2)
3809+
* assumed that vcpu->run->hypercall.ret is never changed by KVM and thus that
3810+
* it was always zero on KVM_EXIT_HYPERCALL. Since KVM is now overwriting
3811+
* vcpu->run->hypercall.ret, ensuring that it is zero to not break QEMU.
3812+
*/
3813+
vcpu->run->hypercall.ret = 0;
38003814
vcpu->run->hypercall.args[0] = gfn_to_gpa(gfn);
38013815
vcpu->run->hypercall.args[1] = npages;
38023816
vcpu->run->hypercall.args[2] = entry_start.operation == VMGEXIT_PSC_OP_PRIVATE

arch/x86/kvm/x86.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10052,6 +10052,13 @@ unsigned long __kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr,
1005210052

1005310053
vcpu->run->exit_reason = KVM_EXIT_HYPERCALL;
1005410054
vcpu->run->hypercall.nr = KVM_HC_MAP_GPA_RANGE;
10055+
/*
10056+
* In principle this should have been -KVM_ENOSYS, but userspace (QEMU <=9.2)
10057+
* assumed that vcpu->run->hypercall.ret is never changed by KVM and thus that
10058+
* it was always zero on KVM_EXIT_HYPERCALL. Since KVM is now overwriting
10059+
* vcpu->run->hypercall.ret, ensuring that it is zero to not break QEMU.
10060+
*/
10061+
vcpu->run->hypercall.ret = 0;
1005510062
vcpu->run->hypercall.args[0] = gpa;
1005610063
vcpu->run->hypercall.args[1] = npages;
1005710064
vcpu->run->hypercall.args[2] = attrs;

0 commit comments

Comments
 (0)