From: dv <dv@openbsd.org>
Date: Sun, 5 Sep 2021 13:13:31 +0000 (+0000)
Subject: vmm(4): fix vcpu locking issues reported by syzbot
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=5f3d69798ad5a04e95493d31d8f03b163ea94bd4;p=openbsd

vmm(4): fix vcpu locking issues reported by syzbot

Syzbot found 3 issues related to the new vcpu lock. This diff adds
a write lock to vm_rwregs (needed on VMX as vmread instructions
require taking ownership of the vcpu to load the VMCS) and prevents
locking the vcpu in vm_run if we fail the cas operation for toggling
vcpu state.

In the future, we can push the locking in vm_rwregs on AMD SVM
systems.

The panics in question:

 panic: rw_enter: vcpulock locking against myself
 panic: lock (rwlock) vcpulock not locked
 panic: vcpulock: lock not held

Reported-by: syzbot+1dab11e14aa7a159cadf@syzkaller.appspotmail.com
Reported-by: syzbot+36244e105daffa1a81b6@syzkaller.appspotmail.com
Reported-by: syzbot+c78b5644c7dc3d9b689a@syzkaller.appspotmail.com

ok mlarkin@
---

diff --git a/sys/arch/amd64/amd64/vmm.c b/sys/arch/amd64/amd64/vmm.c
index c04a1f05bf4..51305111cf2 100644
--- a/sys/arch/amd64/amd64/vmm.c
+++ b/sys/arch/amd64/amd64/vmm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmm.c,v 1.290 2021/09/03 11:47:05 dv Exp $	*/
+/*	$OpenBSD: vmm.c,v 1.291 2021/09/05 13:13:31 dv Exp $	*/
 /*
  * Copyright (c) 2014 Mike Larkin <mlarkin@openbsd.org>
  *
@@ -786,7 +786,7 @@ vm_rwregs(struct vm_rwregs_params *vrwp, int dir)
 	struct vm *vm;
 	struct vcpu *vcpu;
 	struct vcpu_reg_state *vrs = &vrwp->vrwp_regs;
-	int error;
+	int error, ret;
 
 	/* Find the desired VM */
 	rw_enter_read(&vmm_softc->vm_lock);
@@ -804,20 +804,24 @@ vm_rwregs(struct vm_rwregs_params *vrwp, int dir)
 	if (vcpu == NULL)
 		return (ENOENT);
 
+	rw_enter_write(&vcpu->vc_lock);
 	if (vmm_softc->mode == VMM_MODE_VMX ||
 	    vmm_softc->mode == VMM_MODE_EPT)
-		return (dir == 0) ?
+		ret = (dir == 0) ?
 		    vcpu_readregs_vmx(vcpu, vrwp->vrwp_mask, vrs) :
 		    vcpu_writeregs_vmx(vcpu, vrwp->vrwp_mask, 1, vrs);
 	else if (vmm_softc->mode == VMM_MODE_SVM ||
 	    vmm_softc->mode == VMM_MODE_RVI)
-		return (dir == 0) ?
+		ret = (dir == 0) ?
 		    vcpu_readregs_svm(vcpu, vrwp->vrwp_mask, vrs) :
 		    vcpu_writeregs_svm(vcpu, vrwp->vrwp_mask, vrs);
 	else {
 		DPRINTF("%s: unknown vmm mode", __func__);
-		return (EINVAL);
+		ret = EINVAL;
 	}
+	rw_exit_write(&vcpu->vc_lock);
+
+	return (ret);
 }
 
 /*
@@ -4229,9 +4233,10 @@ vm_run(struct vm_run_params *vrp)
 
 			if (atomic_cas_uint(&vcpu->vc_state, old, next) != old)
 				ret = EBUSY;
-			else
+			else {
 				atomic_inc_int(&vm->vm_vcpus_running);
-			rw_enter_write(&vcpu->vc_lock);
+				rw_enter_write(&vcpu->vc_lock);
+			}
 		} else
 			ret = ENOENT;