Skip to content

Commit

Permalink
KVM: make processes waiting on vcpu mutex killable
Browse files Browse the repository at this point in the history
vcpu mutex can be held for unlimited time so
taking it with mutex_lock on an ioctl is wrong:
one process could be passed a vcpu fd and
call this ioctl on the vcpu used by another process,
it will then be unkillable until the owner exits.

Call mutex_lock_killable instead and return status.
Note: mutex_lock_interruptible would be even nicer,
but I am not sure all users are prepared to handle EINTR
from these ioctls. They might misinterpret it as an error.

Cleanup paths expect a vcpu that can't be used by
any userspace so this will always succeed - catch bugs
by calling BUG_ON.

Catch callers that don't check return state by adding
__must_check.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
  • Loading branch information
Michael S. Tsirkin authored and Marcelo Tosatti committed Sep 17, 2012
1 parent 7454766 commit 9fc7744
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 7 deletions.
12 changes: 9 additions & 3 deletions arch/x86/kvm/x86.c
Original file line number Diff line number Diff line change
Expand Up @@ -6016,7 +6016,9 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
int r;

vcpu->arch.mtrr_state.have_fixed = 1;
vcpu_load(vcpu);
r = vcpu_load(vcpu);
if (r)
return r;
r = kvm_arch_vcpu_reset(vcpu);
if (r == 0)
r = kvm_mmu_setup(vcpu);
Expand All @@ -6027,9 +6029,11 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)

void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
{
int r;
vcpu->arch.apf.msr_val = 0;

vcpu_load(vcpu);
r = vcpu_load(vcpu);
BUG_ON(r);
kvm_mmu_unload(vcpu);
vcpu_put(vcpu);

Expand Down Expand Up @@ -6275,7 +6279,9 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)

static void kvm_unload_vcpu_mmu(struct kvm_vcpu *vcpu)
{
vcpu_load(vcpu);
int r;
r = vcpu_load(vcpu);
BUG_ON(r);
kvm_mmu_unload(vcpu);
vcpu_put(vcpu);
}
Expand Down
2 changes: 1 addition & 1 deletion include/linux/kvm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id);
void kvm_vcpu_uninit(struct kvm_vcpu *vcpu);

void vcpu_load(struct kvm_vcpu *vcpu);
int __must_check vcpu_load(struct kvm_vcpu *vcpu);
void vcpu_put(struct kvm_vcpu *vcpu);

int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
Expand Down
10 changes: 7 additions & 3 deletions virt/kvm/kvm_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,12 @@ bool kvm_is_mmio_pfn(pfn_t pfn)
/*
* Switches to specified vcpu, until a matching vcpu_put()
*/
void vcpu_load(struct kvm_vcpu *vcpu)
int vcpu_load(struct kvm_vcpu *vcpu)
{
int cpu;

mutex_lock(&vcpu->mutex);
if (mutex_lock_killable(&vcpu->mutex))
return -EINTR;
if (unlikely(vcpu->pid != current->pids[PIDTYPE_PID].pid)) {
/* The thread running this VCPU changed. */
struct pid *oldpid = vcpu->pid;
Expand All @@ -148,6 +149,7 @@ void vcpu_load(struct kvm_vcpu *vcpu)
preempt_notifier_register(&vcpu->preempt_notifier);
kvm_arch_vcpu_load(vcpu, cpu);
put_cpu();
return 0;
}

void vcpu_put(struct kvm_vcpu *vcpu)
Expand Down Expand Up @@ -1891,7 +1893,9 @@ static long kvm_vcpu_ioctl(struct file *filp,
#endif


vcpu_load(vcpu);
r = vcpu_load(vcpu);
if (r)
return r;
switch (ioctl) {
case KVM_RUN:
r = -EINVAL;
Expand Down

0 comments on commit 9fc7744

Please sign in to comment.