Re: vmd crashes when multiple VMs are rebooted simultaneously

On Thu, May 28, 2026 at 01:37:13PM +0200, hshoexer wrote:
> Hi,
>
> too bad! With that diff I introduced a regression:
>
> My original fix breaks using images supplied with "vmctl start -b":
> In that case, kernfd remained to be set to -1, thus passing that
> filedescriptor to the child process failed.
>
> To resolve this, I now dup(2) vm->vm_kernel right before passing
> the descriptor with proc_compose().  This fixes -b and keeps the
> original fix working.
>
> In the error path I now rely on vm_stop()/vm_remove() closing
> vm->vm_kernel.  Therefore, I remove the redundant close(2).
>
> ok?
>

ok mlarkin

> -------------------------------------------------------------------------
> diff --git a/usr.sbin/vmd/config.c b/usr.sbin/vmd/config.c
> index d7cdf2360ae..8741a58685f 100644
> --- a/usr.sbin/vmd/config.c
> +++ b/usr.sbin/vmd/config.c
> @@ -296,13 +296,11 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid)
>  			ret = EPERM;
>  			goto fail;
>  		}
>
> -		if ((vm->vm_kernel = dup(kernfd)) == -1) {
> -			ret = errno;
> -			goto fail;
> -		}
> -		vmc->vmc_kernel = vm->vm_kernel;
> +		vm->vm_kernel = kernfd;
> +		vmc->vmc_kernel = kernfd;
> +		kernfd = -1;
>  	}
>
>  	/* Open CDROM image for child */
>  	if (strlen(vmc->vmc_cdrom)) {
> @@ -483,8 +481,12 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid)
>  	}
>
>  	/* Send VM information */
>  	/* XXX check proc_compose_imsg return values */
> +	if ((kernfd = dup(vm->vm_kernel)) == -1) {
> +		ret = errno;
> +		goto fail;
> +	}
>  	proc_compose_imsg(ps, PROC_VMM, IMSG_VMDOP_START_VM_REQUEST,
>  	    vm->vm_vmid, kernfd, vmc, sizeof(*vmc));
>
>  	if (strlen(vmc->vmc_cdrom))
> @@ -529,10 +531,8 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid)
>
>   fail:
>  	log_warnx("failed to start vm %s", vmc->vmc_name);
>
> -	if (vm->vm_kernel != -1)
> -		close(vm->vm_kernel);
>  	if (kernfd != -1)
>  		close(kernfd);
>  	if (cdromfd != -1)
>  		close(cdromfd);
> @@ -545,8 +545,9 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid)
>  			close(tapfds[i]);
>  		free(tapfds);
>  	}
>
> +	/* Both vm_stop() and vm_remove() will close vm->vm_kernel. */
>  	if (vm->vm_from_config) {
>  		vm_stop(vm, 0, __func__);
>  	} else {
>  		vm_remove(vm, __func__);
>