From: Mike Larkin <mlarkin@nested.page>
Subject: Re: fix vm_terminate race in vmm found by syzkaller
To: Dave Voutila <dv@sisu.io>
Cc: tech@openbsd.org, gnezdo@openbsd.org
Date: Wed, 17 Dec 2025 09:21:17 -0800

On Wed, Dec 17, 2025 at 12:07:27PM -0500, Dave Voutila wrote:
> 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?
>

ok mlarkin

> [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] */