syzkaller (or is it syzbot?) found a race [1] in vmm(4)'s VMM_IOC_TERM
ioctl path. In short, two threads can race and result in both calling
SLIST_REMOVE with the same vm pointer. (Things would go sideways after
this as well, but that's the source of the page fault trap.)

The diff uses an atomic cas on a new struct member to make sure only the
first caller proceeds through the teardown path. If another caller is
late, it vm_terminate returns EBUSY.

If curious, you can reliably reproduce this by throwing a tsleep_nsec(9)
of a second or two before the lock acquisition for the SLIST and adapt
the regress code to fork and have two callers of the VMM_IOC_TERM
ioctl. Kaboom.

ok?

[1] https://syzkaller.appspot.com/bug?extid=b3be6264d2f1d5c22b0b

Reported-by: syzbot+b3be6264d2f1d5c22b0b@syzkaller.appspotmail.com


diff refs/heads/master refs/heads/vm-terminate-race
commit - 9c2b8e445a0bdfafdd6148b1760f00aa5429627b
commit + 72e835035b5ce1be1ec1c3cdb9e0dbbe62716530
blob - 73d8cad333b2549841340fce8969982fe6702595
blob + 6850cc8134cf29216c57deacbf686184ad01d1d9
--- sys/dev/vmm/vmm.c
+++ sys/dev/vmm/vmm.c
@@ -483,6 +483,7 @@ vm_create(struct vm_create_params *vcp, struct proc *p
 	vcp->vcp_id = vm->vm_id;

 	/* Publish the vm into the list and update counts. */
+	vm->vm_dying = 0;
 	refcnt_init(&vm->vm_refcnt);
 	SLIST_INSERT_HEAD(&vmm_softc->vm_list, vm, vm_link);
 	vmm_softc->vm_ct++;
@@ -723,6 +724,12 @@ vm_terminate(struct vm_terminate_params *vtp)
 	if (error)
 		return (error);

+	/* Only proceed through remove and teardown once. */
+	if (atomic_cas_uint(&vm->vm_dying, 0, 1) == 1) {
+		refcnt_rele_wake(&vm->vm_refcnt);
+		return (EBUSY);
+	}
+
 	/* Pop the vm out of the global vm list. */
 	rw_enter_write(&vmm_softc->vm_lock);
 	SLIST_REMOVE(&vmm_softc->vm_list, vm, vm, vm_link);
blob - 169fc82da9d43e12a11371b8ad4258439661dbde
blob + 187a9afb1eb45c6219713ee96f0845fd5df36980
--- sys/dev/vmm/vmm.h
+++ sys/dev/vmm/vmm.h
@@ -187,6 +187,7 @@ struct vm {

 	char			 vm_name[VMM_MAX_NAME_LEN];
 	struct refcnt		 vm_refcnt;		/* [a] */
+	unsigned int		 vm_dying;		/* [a] */

 	struct vcpu_head	 vm_vcpu_list;		/* [v] */
 	uint32_t		 vm_vcpu_ct;		/* [v] */