From 3fdb3676ba938c22fd21e8a70b07dfc7f5f8cbe4 Mon Sep 17 00:00:00 2001
From: Ariff Abdullah <ariff@FreeBSD.org>
Date: Tue, 21 Mar 2006 06:35:48 +0000
Subject: [PATCH] Apply more thorough fixes while dealing with device opening
 and closing:

- Determine open direction using 'flags', not 'mode'. This bug exist since
  past 4 years.
- Don't allow opening the same device twice, be it in a same or different
  direction.
- O_RDWR is allowed, provided that it is done by a single open (for example
  by mixer(8)) and the underlying hardware support true full-duplex operation.
- Do various paranoid checking in case other process/thread trying to hijack
  the same device twice (or more).

MFC after:	5 days
---
 sys/dev/sound/pcm/dsp.c   | 164 ++++++++++------------
 sys/dev/sound/pcm/mixer.c |  14 +-
 sys/dev/sound/pcm/sound.c | 280 +++++++++++++++++++++++++-------------
 sys/dev/sound/pcm/sound.h |   2 +-
 4 files changed, 275 insertions(+), 185 deletions(-)

diff --git a/sys/dev/sound/pcm/dsp.c b/sys/dev/sound/pcm/dsp.c
index aa8d966a627..23f20386e31 100644
--- a/sys/dev/sound/pcm/dsp.c
+++ b/sys/dev/sound/pcm/dsp.c
@@ -170,11 +170,18 @@ dsp_open(struct cdev *i_dev, int flags, int mode, struct thread *td)
 	struct snddev_info *d;
 	u_int32_t fmt;
 	int devtype;
-	int rdref;
 	int error;
+	int chnum;
+
+	if (i_dev == NULL || td == NULL)
+		return ENODEV;
+
+	if ((flags & (FREAD | FWRITE)) == 0)
+		return EINVAL;
 
 	d = dsp_get_info(i_dev);
 	devtype = PCMDEV(i_dev);
+	chnum = -1;
 
 	/* decide default format */
 	switch (devtype) {
@@ -196,34 +203,24 @@ dsp_open(struct cdev *i_dev, int flags, int mode, struct thread *td)
 
 	case SND_DEV_DSPREC:
 		fmt = AFMT_U8;
-		if (mode & FWRITE) {
+		if (flags & FWRITE)
 			return EINVAL;
-		}
+		chnum = PCMCHAN(i_dev);
 		break;
 
 	default:
 		panic("impossible devtype %d", devtype);
 	}
 
-	rdref = 0;
-
 	/* lock snddev so nobody else can monkey with it */
 	pcm_lock(d);
 
 	rdch = i_dev->si_drv1;
 	wrch = i_dev->si_drv2;
 
-	if ((dsp_get_flags(i_dev) & SD_F_SIMPLEX) && (rdch || wrch)) {
-		/* we're a simplex device and already open, no go */
-		pcm_unlock(d);
-		return EBUSY;
-	}
-
-	if (((flags & FREAD) && rdch) || ((flags & FWRITE) && wrch)) {
-		/*
-		 * device already open in one or both directions that
-		 * the opener wants; we can't handle this.
-		 */
+	if (rdch || wrch || ((dsp_get_flags(i_dev) & SD_F_SIMPLEX) &&
+		    (flags & (FREAD | FWRITE)) == (FREAD | FWRITE))) {
+		/* simplex or not, better safe than sorry. */
 		pcm_unlock(d);
 		return EBUSY;
 	}
@@ -237,67 +234,48 @@ dsp_open(struct cdev *i_dev, int flags, int mode, struct thread *td)
 	if (flags & FREAD) {
 		/* open for read */
 		pcm_unlock(d);
-		if (devtype == SND_DEV_DSPREC)
-			rdch = pcm_chnalloc(d, PCMDIR_REC, td->td_proc->p_pid, PCMCHAN(i_dev));
-		else
-			rdch = pcm_chnalloc(d, PCMDIR_REC, td->td_proc->p_pid, -1);
-		if (!rdch) {
-			/* no channel available, exit */
-			return EBUSY;
-		}
-		/* got a channel, already locked for us */
-		if (chn_reset(rdch, fmt) ||
-				(fmt && chn_setspeed(rdch, DSP_DEFAULT_SPEED))) {
-			pcm_chnrelease(rdch);
-			pcm_lock(d);
-			i_dev->si_drv1 = NULL;
-			pcm_unlock(d);
-			return ENODEV;
+		error = pcm_chnalloc(d, &rdch, PCMDIR_REC, td->td_proc->p_pid, chnum);
+		if (error != 0 && error != EBUSY && chnum != -1 && (flags & FWRITE))
+			error = pcm_chnalloc(d, &rdch, PCMDIR_REC, td->td_proc->p_pid, -1);
+
+		if (error == 0 && (chn_reset(rdch, fmt) ||
+				(fmt && chn_setspeed(rdch, DSP_DEFAULT_SPEED))))
+			error = ENODEV;
+
+		if (error != 0) {
+			if (rdch)
+				pcm_chnrelease(rdch);
+			return error;
 		}
 
 		if (flags & O_NONBLOCK)
 			rdch->flags |= CHN_F_NBIO;
 		pcm_chnref(rdch, 1);
 	 	CHN_UNLOCK(rdch);
-		rdref = 1;
-		/*
-		 * Record channel created, ref'ed and unlocked
-		 */
-		 pcm_lock(d);
+		pcm_lock(d);
 	}
 
 	if (flags & FWRITE) {
 	    /* open for write */
 	    pcm_unlock(d);
-	    wrch = pcm_chnalloc(d, PCMDIR_PLAY, td->td_proc->p_pid, -1);
-	    error = 0;
+	    error = pcm_chnalloc(d, &wrch, PCMDIR_PLAY, td->td_proc->p_pid, chnum);
+	    if (error != 0 && error != EBUSY && chnum != -1 && (flags & FREAD))
+	    	error = pcm_chnalloc(d, &wrch, PCMDIR_PLAY, td->td_proc->p_pid, -1);
 
-	    if (!wrch)
-		error = EBUSY; /* XXX Right return code? */
-	    else if (chn_reset(wrch, fmt) ||
-	    		(fmt && chn_setspeed(wrch, DSP_DEFAULT_SPEED)))
+	    if (error == 0 && (chn_reset(wrch, fmt) ||
+	    		(fmt && chn_setspeed(wrch, DSP_DEFAULT_SPEED))))
 		error = ENODEV;
 
 	    if (error != 0) {
-		if (wrch) {
-		    /*
-		     * Free play channel
-		     */
+		if (wrch)
 		    pcm_chnrelease(wrch);
-		    pcm_lock(d);
-		    i_dev->si_drv2 = NULL;
-		    pcm_unlock(d);
-		}
-		if (rdref) {
+		if (rdch) {
 		    /*
 		     * Lock, deref and release previously created record channel
 		     */
 		    CHN_LOCK(rdch);
 		    pcm_chnref(rdch, -1);
 		    pcm_chnrelease(rdch);
-		    pcm_lock(d);
-		    i_dev->si_drv1 = NULL;
-		    pcm_unlock(d);
 		}
 
 		return error;
@@ -328,40 +306,17 @@ dsp_close(struct cdev *i_dev, int flags, int mode, struct thread *td)
 	pcm_lock(d);
 	rdch = i_dev->si_drv1;
 	wrch = i_dev->si_drv2;
+	if (rdch && td->td_proc->p_pid != rdch->pid)
+		rdch = NULL;
+	if (wrch && td->td_proc->p_pid != wrch->pid)
+		wrch = NULL;
 	pcm_unlock(d);
 
-	refs = 0;
-
-	if (rdch) {
-		CHN_LOCK(rdch);
-		refs += pcm_chnref(rdch, -1);
-		CHN_UNLOCK(rdch);
-	}
-	if (wrch) {
-		CHN_LOCK(wrch);
-		refs += pcm_chnref(wrch, -1);
-		CHN_UNLOCK(wrch);
-	}
-
-	/*
-	 * If there are no more references, release the channels.
-	 */
-	if ((rdch || wrch) && refs == 0) {
-
-		pcm_lock(d);
-
-		if (pcm_getfakechan(d))
-			pcm_getfakechan(d)->flags = 0;
-
-		i_dev->si_drv1 = NULL;
-		i_dev->si_drv2 = NULL;
-
-		dsp_set_flags(i_dev, dsp_get_flags(i_dev) & ~SD_F_TRANSIENT);
-
-		pcm_unlock(d);
-
+	if (rdch || wrch) {
+		refs = 0;
 		if (rdch) {
 			CHN_LOCK(rdch);
+			refs += pcm_chnref(rdch, -1);
 			chn_abort(rdch); /* won't sleep */
 			rdch->flags &= ~(CHN_F_RUNNING | CHN_F_MAPPED | CHN_F_DEAD);
 			chn_reset(rdch, 0);
@@ -369,6 +324,7 @@ dsp_close(struct cdev *i_dev, int flags, int mode, struct thread *td)
 		}
 		if (wrch) {
 			CHN_LOCK(wrch);
+			refs += pcm_chnref(wrch, -1);
 			/*
 			 * XXX: Maybe the right behaviour is to abort on non_block.
 			 * It seems that mplayer flushes the audio queue by quickly
@@ -381,6 +337,23 @@ dsp_close(struct cdev *i_dev, int flags, int mode, struct thread *td)
 			chn_reset(wrch, 0);
 			pcm_chnrelease(wrch);
 		}
+
+		pcm_lock(d);
+		if (rdch)
+			i_dev->si_drv1 = NULL;
+		if (wrch)
+			i_dev->si_drv2 = NULL;
+		/*
+		 * If there are no more references, release the channels.
+		 */
+		if (refs == 0 && i_dev->si_drv1 == NULL &&
+			    i_dev->si_drv2 == NULL) {
+			if (pcm_getfakechan(d))
+				pcm_getfakechan(d)->flags = 0;
+			/* What is this?!? */
+			dsp_set_flags(i_dev, dsp_get_flags(i_dev) & ~SD_F_TRANSIENT);
+		}
+		pcm_unlock(d);
 	}
 	return 0;
 }
@@ -445,15 +418,24 @@ dsp_ioctl(struct cdev *i_dev, u_long cmd, caddr_t arg, int mode, struct thread *
 	 */
 
 	d = dsp_get_info(i_dev);
-	if (IOCGROUP(cmd) == 'M')
-		return mixer_ioctl(d->mixer_dev, cmd, arg, mode, td);
+	if (IOCGROUP(cmd) == 'M') {
+		/*
+		 * This is at least, a bug to bug compatible with OSS.
+		 */
+		if (d->mixer_dev != NULL)
+			return mixer_ioctl(d->mixer_dev, cmd, arg, -1, td);
+		else
+			return EBADF;
+	}
 
 	getchns(i_dev, &rdch, &wrch, 0);
 
 	kill = 0;
-	if (wrch && (wrch->flags & CHN_F_DEAD))
+	if (wrch && ((wrch->flags & CHN_F_DEAD) ||
+		    td->td_proc->p_pid != wrch->pid))
 		kill |= 1;
-	if (rdch && (rdch->flags & CHN_F_DEAD))
+	if (rdch && ((rdch->flags & CHN_F_DEAD) ||
+		    td->td_proc->p_pid != rdch->pid))
 		kill |= 2;
 	if (kill == 3) {
 		relchns(i_dev, rdch, wrch, 0);
@@ -1162,8 +1144,8 @@ dsp_clone(void *arg, struct ucred *cred, char *name, int namelen,
 	struct snddev_info *pcm_dev;
 	struct snddev_channel *pcm_chan;
 	int i, unit, devtype;
-	int devtypes[3] = {SND_DEV_DSP, SND_DEV_DSP16, SND_DEV_AUDIO};
-	char *devnames[3] = {"dsp", "dspW", "audio"};
+	static int devtypes[3] = {SND_DEV_DSP, SND_DEV_DSP16, SND_DEV_AUDIO};
+	static char *devnames[3] = {"dsp", "dspW", "audio"};
 
 	if (*dev != NULL)
 		return;
diff --git a/sys/dev/sound/pcm/mixer.c b/sys/dev/sound/pcm/mixer.c
index cf8403f8a6b..8e4ab84691d 100644
--- a/sys/dev/sound/pcm/mixer.c
+++ b/sys/dev/sound/pcm/mixer.c
@@ -269,10 +269,14 @@ int
 mixer_uninit(device_t dev)
 {
 	int i;
+	struct snddev_info *d;
 	struct snd_mixer *m;
 	struct cdev *pdev;
 
+	d = device_get_softc(dev);
 	pdev = mixer_get_devt(dev);
+	if (d == NULL || pdev == NULL || pdev->si_drv1 == NULL)
+		return EBADF;
 	m = pdev->si_drv1;
 	snd_mtxlock(m->lock);
 
@@ -294,6 +298,8 @@ mixer_uninit(device_t dev)
 	snd_mtxfree(m->lock);
 	kobj_delete((kobj_t)m, M_MIXER);
 
+	d->mixer_dev = NULL;
+
 	return 0;
 }
 
@@ -465,10 +471,16 @@ mixer_ioctl(struct cdev *i_dev, u_long cmd, caddr_t arg, int mode, struct thread
 	int v = -1, j = cmd & 0xff;
 
 	m = i_dev->si_drv1;
-	if (mode != -1 && !m->busy)
+
+	if (m == NULL)
 		return EBADF;
 
 	snd_mtxlock(m->lock);
+	if (mode != -1 && !m->busy) {
+		snd_mtxunlock(m->lock);
+		return EBADF;
+	}
+
 	if ((cmd & MIXER_WRITE(0)) == MIXER_WRITE(0)) {
 		if (j == SOUND_MIXER_RECSRC)
 			ret = mixer_setrecsrc(m, *arg_i);
diff --git a/sys/dev/sound/pcm/sound.c b/sys/dev/sound/pcm/sound.c
index b334d0a8c8d..8bebf2ac38a 100644
--- a/sys/dev/sound/pcm/sound.c
+++ b/sys/dev/sound/pcm/sound.c
@@ -158,30 +158,45 @@ pcm_getfakechan(struct snddev_info *d)
 	return d->fakechan;
 }
 
-/* return a locked channel */
-struct pcm_channel *
-pcm_chnalloc(struct snddev_info *d, int direction, pid_t pid, int chnum)
+/* return error status and a locked channel */
+int
+pcm_chnalloc(struct snddev_info *d, struct pcm_channel **ch, int direction,
+		pid_t pid, int chnum)
 {
 	struct pcm_channel *c;
     	struct snddev_channel *sce;
-	int err;
+	int err, ret;
 
+retry_chnalloc:
+	ret = ENODEV;
 	/* scan for a free channel */
 	SLIST_FOREACH(sce, &d->channels, link) {
 		c = sce->channel;
 		CHN_LOCK(c);
 		if ((c->direction == direction) && !(c->flags & CHN_F_BUSY)) {
-			if (chnum == -1 || c->num == chnum) {
+			if (chnum < 0 || sce->chan_num == chnum) {
 				c->flags |= CHN_F_BUSY;
 				c->pid = pid;
-				return c;
+				*ch = c;
+				return 0;
 			}
 		}
+		if (sce->chan_num == chnum) {
+			if (c->direction != direction)
+				err = EOPNOTSUPP;
+			else if (c->flags & CHN_F_BUSY)
+				err = EBUSY;
+			else
+				err = EINVAL;
+			CHN_UNLOCK(c);
+			return err;
+		} else if (c->direction == direction && (c->flags & CHN_F_BUSY))
+			ret = EBUSY;
 		CHN_UNLOCK(c);
 	}
 
 	/* no channel available */
-	if (direction == PCMDIR_PLAY && d->vchancount > 0 &&
+	if (chnum == -1 && direction == PCMDIR_PLAY && d->vchancount > 0 &&
 			d->vchancount < snd_maxautovchans &&
 			d->devcount <= PCMMAXCHAN) {
 		/* try to create a vchan */
@@ -192,16 +207,17 @@ pcm_chnalloc(struct snddev_info *d, int direction, pid_t pid, int chnum)
 					!SLIST_EMPTY(&c->children)) {
 				err = vchan_create(c);
 				CHN_UNLOCK(c);
-				if (!err)
-					return pcm_chnalloc(d, direction, pid, -1);
-				else
-					device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err);
+				if (!err) {
+					chnum = -2;
+					goto retry_chnalloc;
+				} else
+					device_printf(d->dev, "%s: vchan_create(%s) == %d\n", __func__, c->name, err);
 			} else
 				CHN_UNLOCK(c);
 		}
 	}
 
-	return NULL;
+	return ret;
 }
 
 /* release a locked channel and unlock it */
@@ -259,16 +275,17 @@ pcm_setmaxautovchans(struct snddev_info *d, int num)
 			c = sce->channel;
 			CHN_LOCK(c);
 			if ((c->direction == PCMDIR_PLAY) &&
-					!(c->flags & CHN_F_BUSY) &&
+					(c->flags & (CHN_F_BUSY | CHN_F_VIRTUAL)) == 0 &&
 					SLIST_EMPTY(&c->children)) {
 				c->flags |= CHN_F_BUSY;
 				err = vchan_create(c);
 				if (err) {
 					c->flags &= ~CHN_F_BUSY;
-					device_printf(d->dev, "vchan_create(%s) == %d\n", c->name, err);
+					device_printf(d->dev, "%s: vchan_create(%s) == %d\n", __func__, c->name, err);
+				} else {
+					CHN_UNLOCK(c);
+					return;
 				}
-				CHN_UNLOCK(c);
-				return;
 			}
 			CHN_UNLOCK(c);
 		}
@@ -338,9 +355,9 @@ sysctl_hw_snd_maxautovchans(SYSCTL_HANDLER_ARGS)
 	v = snd_maxautovchans;
 	error = sysctl_handle_int(oidp, &v, sizeof(v), req);
 	if (error == 0 && req->newptr != NULL) {
-		if (v < 0 || v > (PCMMAXCHAN + 1) || pcm_devclass == NULL)
+		if (v < 0 || v > (PCMMAXCHAN + 1))
 			return EINVAL;
-		if (v != snd_maxautovchans) {
+		if (pcm_devclass != NULL && v != snd_maxautovchans) {
 			for (i = 0; i < devclass_get_maxunit(pcm_devclass); i++) {
 				d = devclass_get_softc(pcm_devclass, i);
 				if (!d)
@@ -362,9 +379,11 @@ SYSCTL_PROC(_hw_snd, OID_AUTO, maxautovchans, CTLTYPE_INT | CTLFLAG_RW,
 struct pcm_channel *
 pcm_chn_create(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t cls, int dir, void *devinfo)
 {
-	struct pcm_channel *ch;
+	struct snddev_channel *sce;
+	struct pcm_channel *ch, *tmpch;
 	char *dirs;
-    	int direction, err, *pnum;
+	u_int32_t flsearch = 0;
+	int direction, err, rpnum, *pnum;
 
 	switch(dir) {
 	case PCMDIR_PLAY:
@@ -383,6 +402,7 @@ pcm_chn_create(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t c
 		dirs = "virtual";
 		direction = PCMDIR_PLAY;
 		pnum = &d->vchancount;
+		flsearch = CHN_F_VIRTUAL;
 		break;
 
 	default:
@@ -401,14 +421,61 @@ pcm_chn_create(struct snddev_info *d, struct pcm_channel *parent, kobj_class_t c
 	}
 
 	snd_mtxlock(d->lock);
-	ch->num = (*pnum)++;
+	ch->num = 0;
+	rpnum = 0;
+	SLIST_FOREACH(sce, &d->channels, link) {
+		if (sce == NULL || sce->channel == NULL)
+			continue;
+		tmpch = sce->channel;
+		if (direction != tmpch->direction ||
+				(tmpch->flags & CHN_F_VIRTUAL) != flsearch)
+			continue;
+		if (ch->num == tmpch->num)
+			ch->num++;
+		else {
+			/*
+			 * Channel numbering screwed. Bail out, and do the
+			 * hard way lookup.
+			 */
+			device_printf(d->dev,
+				"%s: channel numbering dirs=%s screwed.\n",
+				__func__, dirs);
+			ch->num = 0;
+			goto retry_num_search;
+		}
+		rpnum++;
+	}
+	goto retry_num_search_out;
+retry_num_search:
+	rpnum = 0;
+	SLIST_FOREACH(sce, &d->channels, link) {
+		if (sce == NULL || sce->channel == NULL)
+			continue;
+		tmpch = sce->channel;
+		if (direction != tmpch->direction ||
+				(tmpch->flags & CHN_F_VIRTUAL) != flsearch)
+			continue;
+		if (ch->num == tmpch->num) {
+			ch->num++;
+			goto retry_num_search;
+		}
+		rpnum++;
+	}
+retry_num_search_out:
+	if (*pnum != rpnum) {
+		device_printf(d->dev,
+			"%s: pnum screwed : dirs=%s, pnum=%d, rpnum=%d\n",
+			__func__, dirs, *pnum, rpnum);
+		*pnum = rpnum;
+	}
+	(*pnum)++;
 	snd_mtxunlock(d->lock);
 
 	ch->pid = -1;
 	ch->parentsnddev = d;
 	ch->parentchannel = parent;
 	ch->dev = d->dev;
-	snprintf(ch->name, 32, "%s:%s:%d", device_get_nameunit(ch->dev), dirs, ch->num);
+	snprintf(ch->name, CHN_NAMELEN, "%s:%s:%d", device_get_nameunit(ch->dev), dirs, ch->num);
 
 	err = chn_init(ch, devinfo, dir, direction);
 	if (err) {
@@ -448,7 +515,10 @@ int
 pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch)
 {
     	struct snddev_channel *sce, *tmp, *after;
+	unsigned rdevcount;
     	int device = device_get_unit(d->dev);
+	int stop;
+	size_t namelen;
 
 	/*
 	 * Note it's confusing nomenclature.
@@ -467,76 +537,91 @@ pcm_chn_add(struct snddev_info *d, struct pcm_channel *ch)
 
 	snd_mtxlock(d->lock);
 	sce->channel = ch;
-	if (SLIST_EMPTY(&d->channels)) {
-		SLIST_INSERT_HEAD(&d->channels, sce, link);
-		sce->chan_num = 0;
-	} else {
-		sce->chan_num = 0;
-retry_search:
-		SLIST_FOREACH(tmp, &d->channels, link) {
-			if (tmp == NULL)
-				continue;
-			if (sce->chan_num == tmp->chan_num) {
-				sce->chan_num++;
-				goto retry_search;
-			}
-		}
-		/*
-		 * Don't overflow PCMMKMINOR / PCMMAXCHAN.
-		 */
-		if (sce->chan_num > PCMMAXCHAN) {
-			snd_mtxunlock(d->lock);
-			device_printf(d->dev,
-				"%s: WARNING: sce->chan_num overflow! (%d)\n",
-				__func__, sce->chan_num);
-			free(sce, M_DEVBUF);
-			return E2BIG;
+	sce->chan_num = 0;
+	after = NULL;
+	stop = 0;
+retry_chan_num_search:
+	/*
+	 * Look for possible channel numbering collision. This may not
+	 * be optimized, but it will ensure that no collision occured.
+	 * Creating maximum possible channels (256 channels) will cost
+	 * us at most 32895 cycles, but this can be considered cheap
+	 * since none of the locking/unlocking operations involved.
+	 *
+	 * Micro optimization, channel ordering:
+	 * hw,hw,hw,vch,vch,vch,rec
+	 */
+	rdevcount = 0;
+	SLIST_FOREACH(tmp, &d->channels, link) {
+		if (tmp == NULL || tmp->channel == NULL)
+			continue;
+		if (sce->chan_num == tmp->chan_num) {
+			sce->chan_num++;
+			after = NULL;
+			stop = 0;
+			goto retry_chan_num_search;
 		}
-		/*
-		 * Micro optimization, channel ordering:
-		 * hw,hw,hw,vch,vch,vch,rec
-		 */
-		after = NULL;
-		if (ch->flags & CHN_F_VIRTUAL) {
-			/* virtual channel to the end */
-			SLIST_FOREACH(tmp, &d->channels, link) {
+		if (stop == 0) {
+			if (ch->flags & CHN_F_VIRTUAL) {
 				if (tmp->channel->direction == PCMDIR_REC)
-					break;
-				after = tmp;
-			}
-		} else {
-			if (ch->direction == PCMDIR_REC) {
-				SLIST_FOREACH(tmp, &d->channels, link) {
+					stop = 1;
+				else
 					after = tmp;
-				}
+			} else if (ch->direction == PCMDIR_REC) {
+				after = tmp;
 			} else {
-				SLIST_FOREACH(tmp, &d->channels, link) {
-					if (tmp->channel->direction == PCMDIR_REC)
-						break;
-					if (!(tmp->channel->flags & CHN_F_VIRTUAL))
-						after = tmp;
+				if (tmp->channel->direction != PCMDIR_PLAY ||
+						(tmp->channel->flags & CHN_F_VIRTUAL)) {
+					stop = 1;
+				} else {
+					after = tmp;
 				}
 			}
 		}
-		if (after == NULL) {
-			SLIST_INSERT_HEAD(&d->channels, sce, link);
-		} else {
-			SLIST_INSERT_AFTER(after, sce, link);
-		}
+		rdevcount++;
+	}
+	/*
+	 * Don't overflow PCMMKMINOR / PCMMAXCHAN.
+	 */
+	if (sce->chan_num > PCMMAXCHAN) {
+		snd_mtxunlock(d->lock);
+		device_printf(d->dev,
+			"%s: WARNING: sce->chan_num overflow! (%d)\n",
+			__func__, sce->chan_num);
+		free(sce, M_DEVBUF);
+		return E2BIG;
+	}
+	if (d->devcount != rdevcount) {
+		device_printf(d->dev,
+			"%s: WARNING: devcount screwed! d->devcount=%u, rdevcount=%u\n",
+			__func__, d->devcount, rdevcount);
+		d->devcount = rdevcount;
 	}
 	d->devcount++;
+	if (after == NULL) {
+		SLIST_INSERT_HEAD(&d->channels, sce, link);
+	} else {
+		SLIST_INSERT_AFTER(after, sce, link);
+	}
+
+	namelen = strlen(ch->name);
+	if ((CHN_NAMELEN - namelen) > 10) {	/* ":dspXX.YYY" */
+		snprintf(ch->name + namelen,
+			CHN_NAMELEN - namelen, ":dsp%d.%d",
+			device, sce->chan_num);
+	}
 	snd_mtxunlock(d->lock);
-	sce->dsp_devt= make_dev(&dsp_cdevsw,
+	sce->dsp_devt = make_dev(&dsp_cdevsw,
 			PCMMKMINOR(device, SND_DEV_DSP, sce->chan_num),
 			UID_ROOT, GID_WHEEL, 0666, "dsp%d.%d",
 			device, sce->chan_num);
 
-	sce->dspW_devt= make_dev(&dsp_cdevsw,
+	sce->dspW_devt = make_dev(&dsp_cdevsw,
 			PCMMKMINOR(device, SND_DEV_DSP16, sce->chan_num),
 			UID_ROOT, GID_WHEEL, 0666, "dspW%d.%d",
 			device, sce->chan_num);
 
-	sce->audio_devt= make_dev(&dsp_cdevsw,
+	sce->audio_devt = make_dev(&dsp_cdevsw,
 			PCMMKMINOR(device, SND_DEV_AUDIO, sce->chan_num),
 			UID_ROOT, GID_WHEEL, 0666, "audio%d.%d",
 			device, sce->chan_num);
@@ -612,20 +697,6 @@ pcm_addchan(device_t dev, int dir, kobj_class_t cls, void *devinfo)
 		return err;
 	}
 
-	CHN_LOCK(ch);
-	if (snd_maxautovchans > 0 && (d->flags & SD_F_AUTOVCHAN) &&
-	    ch->direction == PCMDIR_PLAY && d->vchancount == 0) {
-		ch->flags |= CHN_F_BUSY;
-		err = vchan_create(ch);
-		if (err) {
-			ch->flags &= ~CHN_F_BUSY;
-			CHN_UNLOCK(ch);
-			device_printf(d->dev, "vchan_create(%s) == %d\n", ch->name, err);
-			return err;
-		}
-	}
-	CHN_UNLOCK(ch);
-
 	return err;
 }
 
@@ -650,10 +721,37 @@ int
 pcm_setstatus(device_t dev, char *str)
 {
     	struct snddev_info *d = device_get_softc(dev);
+	struct snddev_channel *sce;
+	struct pcm_channel *ch;
+	int err;
 
 	snd_mtxlock(d->lock);
 	strncpy(d->status, str, SND_STATUSLEN);
 	snd_mtxunlock(d->lock);
+	if (snd_maxautovchans > 0 && (d->flags & SD_F_AUTOVCHAN) &&
+			d->vchancount == 0) {
+		SLIST_FOREACH(sce, &d->channels, link) {
+			if (sce == NULL || sce->channel == NULL)
+				continue;
+			ch = sce->channel;
+			CHN_LOCK(ch);
+			if (ch->direction == PCMDIR_PLAY &&
+					(ch->flags & (CHN_F_BUSY | CHN_F_VIRTUAL)) == 0 &&
+					SLIST_EMPTY(&ch->children)) {
+				ch->flags |= CHN_F_BUSY;
+				err = vchan_create(ch);
+				if (err) {
+					ch->flags &= ~CHN_F_BUSY;
+					device_printf(d->dev, "%s: vchan_create(%s) == %d\n",
+						__func__, ch->name, err);
+				} else {
+					CHN_UNLOCK(ch);
+					return 0;
+				}
+			}
+			CHN_UNLOCK(ch);
+		}
+	}
 	return 0;
 }
 
@@ -761,8 +859,8 @@ pcm_register(device_t dev, void *devinfo, int numplay, int numrec)
             OID_AUTO, "buffersize", CTLFLAG_RD, &d->bufsz, 0, "");
 #endif
 	if (numplay > 0) {
-		vchan_initsys(dev);
 		d->flags |= SD_F_AUTOVCHAN;
+		vchan_initsys(dev);
 	}
 
 	sndstat_register(dev, d->status, sndstat_prepare_pcm);
@@ -973,10 +1071,8 @@ sysctl_hw_snd_vchans(SYSCTL_HANDLER_ARGS)
 	 */
 	d = oidp->oid_arg1;
 
-	if (!(d->flags & SD_F_AUTOVCHAN)) {
-		pcm_inprog(d, -1);
+	if (!(d->flags & SD_F_AUTOVCHAN))
 		return EINVAL;
-	}
 
 	cnt = 0;
 	SLIST_FOREACH(sce, &d->channels, link) {
diff --git a/sys/dev/sound/pcm/sound.h b/sys/dev/sound/pcm/sound.h
index 2bf490c3675..7613b300c64 100644
--- a/sys/dev/sound/pcm/sound.h
+++ b/sys/dev/sound/pcm/sound.h
@@ -212,7 +212,7 @@ struct sysctl_ctx_list *snd_sysctl_tree(device_t dev);
 struct sysctl_oid *snd_sysctl_tree_top(device_t dev);
 
 struct pcm_channel *pcm_getfakechan(struct snddev_info *d);
-struct pcm_channel *pcm_chnalloc(struct snddev_info *d, int direction, pid_t pid, int chnum);
+int pcm_chnalloc(struct snddev_info *d, struct pcm_channel **ch, int direction, pid_t pid, int chnum);
 int pcm_chnrelease(struct pcm_channel *c);
 int pcm_chnref(struct pcm_channel *c, int ref);
 int pcm_inprog(struct snddev_info *d, int delta);
-- 
2.45.2